use ValueLessParameter concept for requiredArgumentCount

[anonrig]

The previous isValuelessArg trait in web-idl.h only handled TypeHandler and Arguments, missing InjectConfiguration types like CompatibilityFlags::Reader. Instead of maintaining a manual list, thread TypeWrapper through requiredArgumentCount and use the ValueLessParameter
concept to automatically detect all injected parameter types.

[ask-bonk]

Only the two places we already identified. This is clean.

Let me now think through the overall review:

1. Correctness: The refactor correctly replaces a manually maintained list (isValuelessArg for TypeHandler<T> and Arguments<T>) with a concept-based approach (ValueLessParameter) that automatically detects all injected types. The Arguments<T> special case is correctly preserved via isArguments<>().

2. Template instantiation ordering: The comment in the PR correctly explains that RequiredArgCount_ specializations in type-wrapper.h will be visible when resource.h templates are instantiated, because type-wrapper.h includes resource.h (so resource.h is parsed first with only the forward declaration), and by the time the resource.h templates are actually instantiated (deferred until after the whole TU is parsed), the specializations will be visible. This is standard C++ two-phase lookup behavior.

3. Tests: The tests are well-written, covering TypeHandler<T>, Arguments<T>, the new InjectConfiguration case (via FakeConfig), and the ValueLessParameter concept directly. The MockTypeWrapper approach is clean and self-contained.

4. Backward compatibility: No behavioral changes to external APIs — this is purely internal template metaprogramming. The .length property on functions will now correctly account for InjectConfiguration parameters, which was previously a bug (they would have been incorrectly counted as required JS arguments).

5. No compat flag needed: This is a bugfix to an internal mechanism. The incorrect .length was an implementation defect, not an intentional behavior.

6. No security concerns: No sandbox or capability changes.

7. No wire format changes: No .capnp schema changes.

The PR is clean. The design is sound, the test coverage is thorough, and the migration is complete.

LGTM

github run

[jasnell]

I meant to check this before but how are methods that accept const v8::FunctionCallbackInfo<Value>& handled?

[jasnell]

Likewise, how is jsg::Arguments handled?

[anonrig]

I meant to check this before but how are methods that accept const v8::FunctionCallbackInfo& handled?

Referencing Claude:

The FunctionCallbackInfo case is already handled correctly. The pipeline for requiredArgumentCount has two phases, and FunctionCallbackInfo is stripped in Phase 1 before it ever reaches the RequiredArgCount_ logic at line 132.

Trace through the pipeline

Take Dummy::withInfo — declared as int withInfo(const v8::FunctionCallbackInfov8::Value&, int):

1. NormalizeFunc_ (meta.h:89-91) — matches R (C::*)(A...), strips the class, yields int(const v8::FunctionCallbackInfov8::Value&, int)
2. StripMagicParam_ (meta.h:114-116) — the explicit specialization for R(const v8::FunctionCallbackInfov8::Value&, A...) matches and strips it, yielding TypeList
3. RequiredArgCount_ (type-wrapper.h) — receives TypeList, counts 1 required arg
   The FunctionCallbackInfo& parameter never reaches RequiredArgCount_ — it's fully handled by StripMagicParam_ at meta.h:114-116, just like Lock& is handled at meta.h:110-112.

Existing test coverage

The tests already cover this (web-idl-test.c++:158,161):
static_assert(rac<decltype(&Dummy::withInfo)> == 1); // FunctionCallbackInfo + 1 real arg
static_assert(rac<decltype(&Dummy::constWithInfo)> == 0); // FunctionCallbackInfo only (const)
Both const and non-const member function variants are covered, as well as the free-function specialization at meta.h:50-52.

Bottom line

There's nothing to fix here. The reviewer's concern is addressed by the existing StripMagicParam_ specialization — FunctionCallbackInfo is stripped in Phase 1 (meta.h) before Phase 2 (type-wrapper.h's RequiredArgCount_ + ValueLessParameter) ever runs. You could reply pointing to the StripMagicParam_ specialization at meta.h:114-116 and the test assertions at web-idl-test.c++:158/161 as evidence.

[anonrig]

Likewise, how is jsg::Arguments handled?

Quoting claude:

jsg::Arguments is handled differently from FunctionCallbackInfo — it is not stripped by StripMagicParam_ and instead reaches RequiredArgCount_ in type-wrapper.h. Let me confirm the handling:
Looking at the RequiredArgCount_ specialization in type-wrapper.h (from the diff):

template <typename TypeWrapper, typename Head, typename... Tail>
struct RequiredArgCount_<TypeWrapper, TypeList<Head, Tail...>> {
  using D = kj::Decay<Head>;
  static constexpr int value = (isArguments<D>() || ValueLessParameter<TypeWrapper, D>)
      ? RequiredArgCount_<TypeWrapper, TypeList<Tail...>>::value  // skip injected args
      : (webidl::isOptional<D> ? 0
                               : 1 + RequiredArgCount_<TypeWrapper, TypeList<Tail...>>::value);
};

Arguments is detected by isArguments() on that first line of the ternary. When it matches, the argument is skipped (doesn't count, doesn't stop counting) and recursion continues with the tail.

The test at web-idl-test.c++:148 covers this:
static_assert(rac<void(int, Arguments)> == 1);
And the concept test at line 172 confirms Arguments is intentionally not a ValueLessParameter:
static_assert(ValueLessParameter<MockTypeWrapper, Arguments> == false);

This is by design — Arguments consumes remaining JS arguments (it's a variadic rest-args collector), so it's semantically different from truly "injected" types like TypeHandler or config types. But for counting purposes it's still invisible to JS callers, so isArguments<>() handles it as a separate check in the same condition.

So to summarize: FunctionCallbackInfo is stripped in Phase 1 (StripMagicParam_), while Arguments passes through to Phase 2 and is skipped via the isArguments() check in RequiredArgCount_. Both are already tested.