Add NTF004 (ref/out/in duck near-miss warning) + async coverage + diagnostics docs#42
Add NTF004 (ref/out/in duck near-miss warning) + async coverage + diagnostics docs#42timonkrebs wants to merge 12 commits into
Conversation
Async-returning members had no coverage despite being ordinary return types to the generator. Add a runtime test that ducks a structurally matching type to an interface exposing Task, Task<T> and ValueTask<T> members - via both Duck<T>() and implicit argument ducking - and awaits each through the proxy, plus a codegen-validity case asserting the forwarding members emitted for those return types parse and bind. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
When an implicit (non-Duck<T>) call can't be bridged, NTypeForge leaves it alone with no NTF diagnostic, so the user is left with the compiler's own overload-resolution error. Add SilentDuckFailureTests pinning that contract for the previously-untested case where the ducked argument itself is passed by ref/out/in (IsDuckableArgument rejects RefKind != None), plus user-facing 'compiler error still stands' assertions for the ambiguous and partial multi-argument cases (whose 'no proxy' view DiagnosticTests already covers). Document the three silent cases in a new README 'When implicit ducking is silently skipped' subsection under Limitations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
When an implicit call's argument structurally matches an interface parameter but that parameter is ref/out/in, ducking can't help (a generated proxy can't be passed by reference) and the call previously failed silently. Detect that clean near-miss in CandidateAnalyzer - every other argument binds, the ref kind is the sole blocker, exactly one interpretation - and surface NTF004, a Warning mirroring NTF003 so it explains the failure without masking the compiler's own error and never fires on ordinary type errors. - DuckedArgModel carries the ref-kind blocker + parameter name (folded into the cache key); near-miss candidates route to a new diagnostic branch and are excluded from the emit branch. - Move the ref/out/in characterization tests from SilentDuckFailureTests to DiagnosticTests as NTF004 assertions (+ a guard that a non-matching by-ref argument stays silent). Ambiguity and partial matches remain silent. - Register NTF004 and document it (README Limitations + Diagnostics table). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
timonkrebs
changed the title
|
@codex review this pr |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67e764fc99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var parameter = candidate.Parameters[paramIndex]; | ||
| var expression = arguments[syntaxIndex].Expression; | ||
|
|
||
| if (IsRefKindInterfaceParameter(parameter)) |
There was a problem hiding this comment.
When a call fails only because the caller omitted or mismatched the by-reference keyword, this branch still treats every ref/out/in interface parameter as a ducking near-miss. For example, ICalc c; m.H(c) against void H(ref ICalc c) reaches TryBuildRefKindBlocker and reports NTF004 even though the argument is already the interface and no generated proxy is involved; the actionable compiler error is just the missing ref. Please verify the argument's ref kind matches the parameter, and that the argument is not already implicitly convertible, before reporting this warning.
Useful? React with 👍 / 👎.
| else if (!BindsImplicitly(semanticModel, expression, parameter.Type)) | ||
| { | ||
| return null; |
There was a problem hiding this comment.
For clean near-misses that also pass expanded params arguments, this check compares each expanded element to the array parameter type, so a valid call like H(ref ICalc c, params int[] values) invoked as H(ref adder, 1, 2) is rejected because 1 is not implicitly convertible to int[]. That suppresses NTF004 even though the ref duck is the only real blocker and regular implicit ducking already supports this params call shape; handle params expansion here by checking elements against the parameter element type.
Useful? React with 👍 / 👎.
| if (candidate.ContainingType == null) continue; | ||
| var blocked = CollectRefKindNearMiss(semanticModel, candidate, arguments, argFacts, cancellationToken); | ||
| if (blocked == null) continue; | ||
| if (only != null) return null; // more than one near-miss interpretation: ambiguous, stay silent |
There was a problem hiding this comment.
This loop treats a second qualifying raw candidate as ambiguity before applying the equivalent-candidate deduplication used by normal implicit ducking. In contexts where Roslyn reports both an override/base method or repeats a candidate (the cases handled by DistinctInterpretations below), a single ref/out/in near-miss is dropped and NTF004 is never reported. Please key and deduplicate the blocked candidate interpretations before returning null for ambiguity.
Useful? React with 👍 / 👎.
Address three review findings on the NTF004 near-miss detection: - Fire only when the argument was passed with the parameter's matching ref/out/in keyword and is not already implicitly convertible to the interface. A missing/mismatched keyword or an already-convertible value (e.g. the arg is already the interface, or ref-invariance on a nominal implementer) is a plain compiler error, not a ducking near-miss, so those no longer warn. - Treat expanded params elements as bound against the array's element type, so a real ref near-miss alongside a params call (H(ref adder, 1, 2)) is no longer suppressed. - Route near-miss candidates through DistinctInterpretations so an override and the base it hides (or a repeated candidate) collapse to one interpretation instead of reading as false ambiguity. Refactors the block to mirror the CollectDuckableInterpretations -> DistinctInterpretations -> Resolve...Sites structure of the normal duck path. Adds tests for the omitted-keyword, already-implements, params, and overridden-method cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Thanks @codex — all three were valid. Addressed in
The block was refactored to mirror the normal duck path's Generated by Claude Code |
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe8ccd6545
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (BindsImplicitly(semanticModel, expression, parameter.Type)) return true; | ||
| return parameter.IsParams && parameter.Type is IArrayTypeSymbol array && | ||
| BindsImplicitly(semanticModel, expression, array.ElementType); |
There was a problem hiding this comment.
When another by-reference parameter is present but is not the ducked near-miss, this helper accepts it based only on type conversion. For example, H(ref ICalc c, ref int n) called as H(ref adder, n) treats n as already bound because int converts to int, so NTF004 is emitted even though the missing ref on the second argument is an additional blocker; similarly, ref invariance can be missed for derived-to-base arguments. Please check the argument's ref kind (and exact by-ref binding semantics) before considering non-near-miss by-reference parameters already bound.
Useful? React with 👍 / 👎.
| if (parameter.RefKind == RefKind.None || parameter.Type.TypeKind != TypeKind.Interface) return false; | ||
| if (!ArgumentRefKindMatches(argument, parameter.RefKind)) return false; |
There was a problem hiding this comment.
This only verifies that the caller wrote the matching ref/out/in keyword, but a by-reference argument also has to be an assignable variable. A call like m.H(ref new Adder()) or m.H(ref some.Property) against ref ICalc still reaches NTF004 because the expression has the structural type, even though the compiler error is that the argument is not a valid ref/out variable rather than a ducking-only blocker. Please include the ref/out variable requirement in this near-miss gate so NTF004 stays limited to clean by-reference blockers.
Useful? React with 👍 / 👎.
| if (BindsImplicitly(semanticModel, expression, parameter.Type)) return true; | ||
| return parameter.IsParams && parameter.Type is IArrayTypeSymbol array && | ||
| BindsImplicitly(semanticModel, expression, array.ElementType); |
There was a problem hiding this comment.
This treats every argument mapped to a params parameter as bound if it converts to the element type, but a named params argument is not expanded element-by-element. For example, H(ref ICalc c, params int[] values) called as H(ref adder, values: 1) is still invalid because values: must supply an int[], yet this check accepts 1 as an int element and reports NTF004 even though the bad named params argument is an additional blocker. Only apply the element-type fallback to positional expanded params arguments.
Useful? React with 👍 / 👎.
Address three more review findings where NTF004 could fire even though the ref duck was not the only blocker: - A by-reference passthrough argument (a ref/out/in parameter that is not the ducked near-miss) now binds only when the caller used the matching keyword, the argument is an assignable variable, and its type matches exactly - so a missing keyword or ref-invariance on that argument suppresses NTF004. - The blocked near-miss argument must itself be an assignable variable, so ref/out/in on an rvalue, property, or method result (a plain ref-variable error) no longer warns. - The expanded-params element fallback applies only to positional arguments; a named params argument (values: 1) must supply the whole array, so it is no longer accepted element-by-element. Adds tests for the second-by-ref-arg, non-assignable-argument, and named-params cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Thanks @codex — round 2 all valid, fixed in
Net effect: NTF004 stays limited to a clean sole-blocker near-miss. Added tests for all three cases. Generated by Claude Code |
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cedf4e64c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case IParameterSymbol: return true; | ||
| case IFieldSymbol field: return !field.IsConst; |
There was a problem hiding this comment.
When the structurally matching argument is a readonly storage location passed to a ref/out interface parameter, this helper still returns true for any parameter and any non-const field. That makes readonly Adder _a; m.H(ref _a) or an in Adder a parameter passed to ref ICalc emit NTF004 even though the call has an additional compiler error: readonly locations cannot be passed by ref/out. The assignability check needs to be ref-kind-aware so NTF004 remains limited to ducking-only blockers.
Useful? React with 👍 / 👎.
| ArgumentDuckFact?[] argFacts, CancellationToken cancellationToken) | ||
| { | ||
| var interpretations = new List<(IMethodSymbol, List<(int, int)>)>(); | ||
| foreach (var candidate in symbolInfo.CandidateSymbols.OfType<IMethodSymbol>()) |
There was a problem hiding this comment.
When the failed call has candidates only because Roslyn kept a non-invocable member, for example Mgr.H is private from the call site, this loop still analyzes that candidate as if overload resolution failed solely on the ref duck. That emits NTF004 even though accessibility is an additional compiler error, so candidate reasons such as NotAccessible need to be filtered before treating the by-ref interface as the lone blocker.
Useful? React with 👍 / 👎.
Two more sole-blocker refinements: - Ref-kind-aware assignability: ref/out arguments now require a writable variable, so a readonly field, an in-parameter, or a ref-readonly local passed by ref/out (a separate compiler error) no longer emits NTF004. in arguments still accept any readable variable. - Filter inaccessible candidates: a candidate that fails on accessibility (e.g. a private method called from another type) is no longer treated as a ref-duck near-miss, so accessibility errors don't get an NTF004 on top. Adds tests for the readonly-field, in-parameter, and inaccessible-candidate cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Thanks @codex — round 3 both valid, fixed in
Tests added for the readonly-field, Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2bdecd390
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case IFieldSymbol field: | ||
| return !field.IsConst && (!requiresWritable || !field.IsReadOnly); |
There was a problem hiding this comment.
When the argument is a field reached through a readonly value-type receiver, such as readonly Holder _h; H(ref _h.Value) where Value structurally matches the interface, GetSymbolInfo returns the inner field symbol and this branch only checks that field's own IsReadOnly. That reports NTF004 even though the call also has a readonly-location compiler error, so the by-ref duck is not the sole blocker; inspect the member-access receiver before accepting field arguments.
Useful? React with 👍 / 👎.
…nd 4) A struct field passed by ref/out is only writable through a writable receiver, so ref _h.Value where _h is a readonly Holder field is a readonly location - an additional compiler error, not a sole-blocker ref duck. IsRefKindAssignable now walks the receiver chain for value-type fields (a class field is not constrained by its receiver). Adds a test for the readonly-struct-field case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Thanks @codex — valid, fixed in @timonkrebs — a note on scope: this is the 4th review round, and the findings are now deep binder edge cases (readonly struct receivers) on a warning-only diagnostic. Each has been a real-but-rare false positive that the sole-blocker gate keeps narrowing, and the core NTF004 behavior has been solid and green throughout. I'm happy to keep hardening toward maximal precision if you'd like, but this is a reasonable point to call it and merge — your call. Absent other direction, I'll stay selective from here and only push fixes for clearly material findings. Generated by Claude Code |
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2a4d9d2f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return TryGetMethodArgumentDuck(invocation, semanticModel, symbolInfo, cancellationToken) | ||
| ?? TryGetRefKindNearMiss(invocation, semanticModel, symbolInfo, cancellationToken); |
There was a problem hiding this comment.
When a failed call has both a by-value argument that can be ducked and a ref/out/in structural near-miss, this short-circuits on TryGetMethodArgumentDuck and never runs the NTF004 path. For example, H(ref ICalc c, ILog l) called as H(ref adder, new Logger()) emits a forwarding extension for only l; that generated method still takes ref ICalc for c, so the user gets no NTF004 explanation and an unusable extension is emitted. The normal duck path needs to reject or account for non-ducked by-ref blockers before returning.
Useful? React with 👍 / 👎.
| case ILocalSymbol local: | ||
| return !local.IsConst && (!requiresWritable || local.RefKind == RefKind.None || local.RefKind == RefKind.Ref); |
There was a problem hiding this comment.
For a structurally matching local that is declared but not definitely assigned, such as Adder a; H(ref a) or H(in a), this returns true merely because the local is non-const. That lets NTF004 fire even though the call also has a definite-assignment compiler error, so the by-ref duck is not the sole blocker; ref and in arguments need a definite-assignment check while out can remain exempt.
Useful? React with 👍 / 👎.
| if (!TryMapArgumentsToParameters(arguments, candidate, out var parameterIndices)) continue; | ||
|
|
||
| var blocked = CollectRefKindBlockedArgs(semanticModel, candidate, arguments, parameterIndices, argFacts, cancellationToken); | ||
| if (blocked != null && blocked.Count > 0) interpretations.Add((candidate, blocked)); |
There was a problem hiding this comment.
When the candidate method is itself not invocable, this still records a ref near-miss once the syntax arguments bind. For example, T H<T>(ref ICalc c) called as m.H(ref adder) also fails because T cannot be inferred (similarly, bad explicit type arguments or unsatisfied constraints add their own compiler errors), yet NTF004 is reported as if the ref duck were the sole blocker. Filter out generic candidates whose type inference and constraints did not succeed before adding the interpretation.
Useful? React with 👍 / 👎.
…erable generics (review round 5) - Mixed site: when a failed call has both a duckable by-value argument and a non-ducked ref/out/in argument that doesn't convert to its parameter (e.g. a by-reference interface the caller hoped to duck), the duck path previously emitted a forwarding extension that still took the ref parameter and was uncallable. Reject such interpretations so the site stays silent (or reaches the NTF004 path) instead of generating dead code. - Generic candidates whose type arguments weren't inferred to concrete types fail on inference too, so they're no longer treated as ref-duck near-misses (no spurious NTF004). Adds tests for the mixed ref-near-miss/value-duck site and the uninferable generic case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Thanks @codex — round 5 triaged:
Tests added for the mixed-site and uninferable-generic cases. Generated by Claude Code |
Summary
Testing
|
…gression) The round-5 unbridgeable-by-ref guard used an expression conversion, but an 'out var x' declaration has no expression conversion to the parameter type, so it wrongly rejected the valid duck in CanDuckTypeWithStructOutParameters (HandleCreateOrigin(handler, out var point)) - breaking the build. Compare the argument's type to the parameter instead: a struct passthrough or an out variable of the parameter type converts implicitly and is kept, while a structural-only near-miss (Adder vs ICalc) still flags the dead-forwarding case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
CI is green again on Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19bddc157e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (parameter.RefKind != RefKind.None) | ||
| return ByReferenceArgumentBinds(semanticModel, argument, parameter, cancellationToken); | ||
|
|
||
| if (BindsImplicitly(semanticModel, expression, parameter.Type)) return true; |
There was a problem hiding this comment.
When a by-value argument is passed with an erroneous ref/out/in keyword, this still treats it as already bound because it only checks the expression conversion. For example, H(ref ICalc c, int n) called as H(ref adder, ref n) will report NTF004 even though the extra ref on n is a separate compiler error, so the by-ref duck is not the sole blocker. Please also require no argument ref-kind keyword for by-value parameters before returning true here.
Useful? React with 👍 / 👎.
| if (parameter.RefKind != RefKind.None && | ||
| !BindsImplicitly(semanticModel, arguments[syntaxIndex].Expression, parameter.Type)) |
There was a problem hiding this comment.
This new guard still checks only implicit conversion for non-ducked ref/out/in parameters, so it misses keyword, lvalue, and exact-type failures. In a mixed call like H(ICalc c, ref int n) invoked as m.H(new Adder(), n), n converts to int, so the generator emits a forwarding overload for the ducked first argument even though the generated overload still requires ref int and the original call remains uncallable. Please use the same by-reference binding checks as the NTF004 path before allowing normal ducking to proceed.
Useful? React with 👍 / 👎.
| // An inaccessible candidate fails on accessibility, not the ref duck, so it is not a | ||
| // sole-blocker near-miss - skip it (also keeps generated code from naming a member it | ||
| // could not call). | ||
| if (!semanticModel.IsAccessible(invocation.SpanStart, candidate)) continue; |
There was a problem hiding this comment.
Checking only accessibility still lets non-invocable receiver forms through this near-miss path. For example, if H is an instance method and the user writes Mgr.H(ref adder), the method is accessible and the ref argument structurally matches, so NTF004 is reported even though the call also fails because an instance member was invoked through the type; please also reject candidates whose static/instance receiver form is not applicable before treating the ref duck as the sole blocker.
Useful? React with 👍 / 👎.
| // near-miss. | ||
| if (candidate.IsGenericMethod && candidate.TypeArguments.Any(t => t.TypeKind == TypeKind.TypeParameter)) continue; | ||
| var arguments = invocation.ArgumentList.Arguments; | ||
| if (!TryMapArgumentsToParameters(arguments, candidate, out var parameterIndices)) continue; |
There was a problem hiding this comment.
This mapping accepts argument lists that Roslyn still rejects for ordering, so NTF004 can fire when the ref duck is not the only blocker. For example, H(int n, ref ICalc c) invoked as H(c: ref adder, 1) maps both arguments here, but C# rejects the positional argument after the out-of-position named argument; please verify the argument list is actually applicable before treating the ref interface parameter as a sole near-miss.
Useful? React with 👍 / 👎.
…(review round 6) - Dead forwarding (line 615): the unbridgeable-by-ref guard checked type but not the keyword, so H(ICalc c, ref int n) called H(adder, n) still emitted an uncallable forwarding (the ref int parameter stays, but the arg lacks 'ref'). Now flag a ref/out passthrough whose argument omits the matching keyword. (in stays keyword-optional, judged by type.) - NTF004 false positive (line 396): a by-value parameter passed with a ref/out/in keyword (H(ref adder, ref n) against int n) is a separate compiler error, so it's no longer treated as already-bound - the ref duck isn't the sole blocker. Adds tests for both. (Two further niche findings - static/instance receiver mismatch and invalid named-argument ordering - are deferred pending maintainer direction; they're rare false-positive warnings needing more binder replication.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Round 6 —
@timonkrebs — a decision worth making before I continue. We're 6 review rounds in. The pattern is clear: NTF004 (and the duck path's "sole-blocker" check) is manually replicating C# overload-applicability, and Codex keeps finding another way a call can fail in addition to the ref-duck — keyword, lvalue, readonly, accessibility, generics, receiver form, arg ordering, definite assignment… It's an unbounded tail, and one of these patches already caused a regression (caught by CI). All of it is on a warning-only feature. Two ways forward:
My recommendation is (a) ship — the cost/benefit of chasing rare warning-only false positives is poor — but (b) is the clean long-term answer if you want maximal precision. Let me know which and I'll proceed; until then I'll hold further niche findings rather than keep patching. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ef4c7922d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (field.ContainingType?.IsValueType == true && expression is MemberAccessExpressionSyntax member) | ||
| return IsVariable(semanticModel, member.Expression, requiresWritable: true, cancellationToken); |
There was a problem hiding this comment.
When a struct field is passed by ref from a readonly instance member using the implicit receiver, e.g. readonly void M() { H(ref _adder); }, the expression is an IdentifierNameSyntax, so this new receiver check is skipped and the method falls through to return true. Fresh evidence in the current code is that only MemberAccessExpressionSyntax receivers are inspected here; that still lets NTF004 fire even though the call also has the compiler's readonly-location error, so the by-ref duck is not the sole blocker.
Useful? React with 👍 / 👎.
Root-cause fix that subsumes the tail of by-reference near-miss edge cases: NTF004 now fires only when the invocation's sole error diagnostics are the ducked argument's conversions (CS1503/CS1502). Any other error - a missing or extra ref keyword, a readonly or unassigned location (including a struct field via a readonly implicit receiver), an inapplicable static/instance receiver form, invalid named-argument ordering, failed generic inference, inaccessibility - means the by-reference type mismatch is not the actionable problem, so the compiler's own error stands and NTF004 stays silent. The existing structural guards remain as a cheap first pass; this gate is the authoritative sole-blocker guarantee. Adds tests for the receiver-form, named-argument-ordering, definite-assignment, and readonly-implicit-receiver cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
The sole-blocker gate can't catch Mgr.H(ref adder) (instance method named through its type), because Roslyn folds the static/instance issue into the argument-mismatch CS1503 rather than emitting a distinct error. Add an explicit receiver-form check: an instance method named through its type, or a static method through an instance, is not callable and so is not a sole-blocker near-miss. Fixes NTF004_NotReported_WhenInstanceMethodInvokedThroughType. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs
|
Done — NTF004 now fires only when the invocation's only compiler errors are the ducked argument's conversions (CS1503/CS1502). Any other error — keyword misuse, a readonly or unassigned location (including a struct field via readonly This retires the whole tail in one place, including the four findings that were still open:
The earlier structural guards remain as a cheap first pass; the gate is the authoritative guarantee, so NTF004 is now behind a single robust check rather than a growing list of special cases. Tests added for all four cases. Ready to merge. Generated by Claude Code |
Three related DX improvements to NTypeForge's diagnostics and coverage.
1. NTF004 — diagnose the
ref/out/innear-miss (67e764f)Previously, an implicit call whose argument structurally matches an interface parameter but is passed by
ref/out/infailed silently — the user got a rawCS1503with no hint, even though their type would otherwise duck. A generated proxy can't be passed by reference, so it genuinely can't be bridged — but that's worth explaining.New NTF004 (Warning) does exactly that, held to the same high-confidence bar as NTF003:
CandidateAnalyzer.TryGetRefKindNearMiss; the model carries the ref-kind + parameter name (folded into the cache key); near-miss candidates route to a new diagnostic branch and are excluded from emit (they can't be ducked).Ambiguity and partial matches remain deliberately silent — that's an intentional design choice (avoid guessing / avoid restating plain type errors), now pinned by
SilentDuckFailureTests.2. Async-returning members (
9785647)Task/Task<T>/ValueTask<T>are ordinary return types to the proxy, but were untested. Adds a runtime test (AsyncMemberDuckingTests, awaited through the proxy via bothDuck<T>()and implicit ducking) and a codegen-validity case.3. Docs (
e961ee7+67e764f)README gains a "When implicit ducking can't bridge a call" subsection (ref/out/in → NTF004; ambiguous/incomplete → silent) and an NTF004 row in the Diagnostics table.
Verification
No local SDK, so CI is the check. Beyond
build+dotnet test, thecognitive-complexitygate (S3776 ≤ 15) matters here — the new detection was deliberately decomposed into small helpers to stay under it. NTF004 is contained to failing calls with a ref/out/in interface parameter, so it won't fire (and tripTreatWarningsAsErrors) in the sample or runtime-test projects, whose ref/out/in params are structs or passthroughs.🤖 Generated with Claude Code
https://claude.ai/code/session_01F7W1dyFLg9YmH8GpCiG7Cs