Interop: a params overload is chosen by the array's element type, and a failing element declines instead of throwing (#3764)

[lahma]

Backports #3764 (main) to the 4.x maintenance branch. Both defects were confirmed present on 4.x by running the ported tests against unfixed code first.

The two defects

A. A JavaScript array short-circuited overload scoring above the converter probe. A params call bundles its trailing arguments into one JsArray before overload resolution runs, so InteropHelper.CalculateMethodParameterScore rated three candidates differing only in their element type — CDecimal[], CInteger[], CLong[] — with one rule, "is an array, wants an array", that answered a flat 2 for every one of them. The element type was never consulted, the converter probe below that rule (the #3407 fix, which 4.x already has) was never reached, and declaration order decided. Where the first-declared candidate happened to be convertible it answered silently and wrongly; where it was not, the conversion's InvalidCastException left through the embedder's Evaluate — invisible to a script catch and to a host catch (JavaScriptException) alike.

B. The composite branches of DefaultTypeConverter.TryConvert converted their parts through the public, throwing Convert. A List<T> or Collection<T> item, a T[] element, a target dictionary's value and a member of a POCO built from a dictionary all ignored the propagateException flag their own frame had been called with, so a method documented as returning false threw. That is not something the candidate loop in MethodInfoFunction.Call can move on from: it tries candidates in score order and declines its way to the next, and an exception ends the call rather than the candidate.

They are fixed together, because fixing A by asking the converter per element throws until B is fixed.

• TryConvertPart routes all five composite sites through the flag their frame was given. It mirrors Convert — the virtual TryConvert first, so a subclass override still answers for the parts — so behaviour under propagateException: true is unchanged.
• CalculateParameterTypeScore is the type-level core of the parameter score, so an element with no ParameterInfo of its own can be scored by the same rules. An array parameter now scores its base 2 plus its worst element, up to eight of them, and returns -1 on the first element that cannot bind at all. Element types any JsValue is assignable to — object, JsValue — are carved out and keep the flat base, or params object[] and params JsValue[] would rank below the scalar overload beside them.

Deliberately out of scope, exactly as on main: MethodDescriptor.Call's TypeReference constructor lane keeps its throwing Convert and gets no retry loop (correct selection is what repairs it), and an empty JS array against several params T[] overloads stays ambiguous and keeps today's answer.

Failing first

Both files were ported and run against unfixed 4.x before Jint/ was touched. Identical counts on both legs (net472, which exercises Jint's net462-compiled asset, and net10.0):

Suite	Unfixed 4.x	After
Jint.Tests.PublicInterface/HostArrayElementOverloadTests	8 failed / 3 passed of 11	11 passed
Jint.Tests/Runtime/CompositeConversionDeclineTests	5 failed / 6 passed of 11	11 passed

The three that passed unfixed in the first file are exactly the ones meant to: the empty-array pin (AnEmptyArrayKeepsTodaysAnswer, deliberately unchanged behaviour) and the two carve-out guards (params object[], params JsValue[]). The six that passed unfixed in the second are the five Convert-still-throws cases and the convertible-composite control — the half that pins what must not change.

The two representative unfixed failures:

• math.Add(a, b) with two CIntegers → System.InvalidCastException: Invalid cast from '…' to '…+CDecimal' out of Evaluate, through MethodInfoFunction.TryCall → DefaultTypeConverter.TryConvert → Convert.
• converter.TryConvert(new object[] { new Widget() }, typeof(Gadget[]), …) threw instead of returning false.

Divergences from main actually found

1. parameterValue.IsSpecArray() on main is parameterValue.IsArray() on 4.x — confirmed. JsProxy overrides it on 4.x too, so the Proxy carve-out (is not ArrayInstance keeps the base score, no traps run during scoring) still holds and the comment was reworded from "spec array" to "array".
2. engine._typeConverter on main is engine.TypeConverter on 4.x — confirmed, but it did not affect this change: the only site reading it is the existing Interop: a plain type mismatch scores 100 instead of being discarded, so 'text' + an overloaded CLR value throws #3407 probe, which the port leaves untouched.
3. ClrTypeConverter on main is ITypeConverter on 4.x — confirmed as the base type of DefaultTypeConverter. It did not affect TryConvertPart, whose doc comment references only Convert, TryConvert and Options.InteropOptions.ExceptionHandler, all of which exist on 4.x with the same names.
4. The fifth site does differ structurally, as expected: 4.x has the POCO-member copy inline in a foreach (var member in type.GetMembers()) loop with a MemberType != Property && != Field filter, not main's extracted static void CopyDictionaryEntryToMember(…) local function over separate GetProperties()/GetFields() passes. main's refactor was not imported; TryConvertPart is called inline and the loop return falses on a decline.
5. 4.x's catch in the Convert.ChangeType fallback has no Throw.MustPropagateHostException(e) call — confirmed, and left alone.
6. ArrayInstance.GetLength() (internal override uint) and TryGetValue(uint, out JsValue) (public bool) exist on 4.x with the same signatures — no adaptation needed.

Two more that turned up during the port:

7. The constructor-resolution message differs. 4.x throws Could not resolve a constructor for type {type} for given arguments; main reworded it to Could not resolve a constructor for the specified arguments.. AConstructorArgumentNoOverloadAcceptsIsACatchableResolutionFailure asserts 4.x's wording (with a * wildcard for the type name). The behaviour being pinned — a catchable JavaScriptException / script-visible TypeError instead of a raw InvalidCastException — is identical, and that test is red on unfixed 4.x either way.
8. The tests are xUnit v3 on 4.x, and the folder layout differs. [Test] → [Fact] throughout (no [TestCase] in either file). Invoking(…) and the AwesomeAssertions global usings are already present on 4.x in both projects, so the assertion style is unchanged. main created Jint.Tests/Runtime/Interop/ for CompositeConversionDeclineTests.cs; 4.x has no such folder and keeps DefaultTypeConverterTests.cs directly in Jint.Tests/Runtime/, so the ported file sits there with namespace Jint.Tests.Runtime.

No AGENTS.md change. 4.x has no Jint/Runtime/Interop/AGENTS.md — the co-located split is main-only, and 4.x's single root AGENTS.md carries no "Overload scoring's last rule" bullet (and no mention of #3407) to extend. No docs/v5-migration.md row either; that file is main-only.

Verification

All in Release, no --no-build, TreatWarningsAsErrors on, both TFMs where the project multi-targets:

Suite	Result
Jint.Tests	7017 passed / 0 failed / 4 skipped (net472) · 7102 / 0 / 4 (net10.0)
Jint.Tests.PublicInterface	1828 / 0 / 9 (net472) · 1836 / 0 / 9 (net10.0)
Jint.Tests.CommonScripts	28 / 0 / 0 on both
Jint.Tests.Test262	102,499 passed / 0 failed / 185 skipped of 102,684

test262 matches the branch control exactly, with neither of the two known load flakes (intl402/supportedLocalesOf-unicode-extensions-ignored.js, staging/sm/Array/toSpliced-dense.js) firing this run. Full solution build: 0 errors, and the only warning is the pre-existing MSB3277 reference conflict in Jint.Tests.CommonScripts on net472, untouched by this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LfyCDDqtkhyWVEFHKWhZfu