Uh oh!
There was an error while loading. Please reload this page.
Fix semantic classification errors for generics, slices, CEs, and open type - #19939
Fix semantic classification errors for generics, slices, CEs, and open type#19939T-Gro wants to merge 5 commits into
Conversation
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
Warning No PR link found in some release notes, please consider adding it.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
left a comment
There was a problem hiding this comment.
AI-generated review (Claude / expert-reviewer agent). Findings posted as inline comments — please apply human judgement before acting.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
T-Gro
left a comment
There was a problem hiding this comment.
🤖 AI-generated review (Claude / expert-reviewer agent) — complementary to T-Gro's earlier pass. Findings posted as inline comments; please apply human judgement before acting.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ate decl PR #19813 suppressed a narrow synthesized Invoke/BeginInvoke/EndInvoke classification in a delegate declaration via a one-directional rangeContainsRange check inside the Item.MethodGroup arm. A second wide-range entry whose range covers the whole 'delegate of . -> .' clause still slipped through because the parser-synthesized SynValSig for the delegate's abstract slot is resolved as Item.Value vref (not Item.MethodGroup), with the val's idRange covering the entire `delegate of x: int * y: int -> int` clause (this is the parser's lhs range of the DELEGATE OF topType production used both for the SynIdent and the SynValSig itself). Add a targeted suppression in the Item.Value vref arm (vref.IsMember branch): when the val is a dispatch slot named Invoke/BeginInvoke/EndInvoke whose MemberApparentEntity is an F# delegate tycon, do not emit a Method classification. This precisely matches the parser-synthesized abstract slot only - real call sites like `d.Invoke(42)` resolve as Item.MethodGroup which is handled by the existing arm and is unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…od/ctor calls In TcMethodItemThen and TcCtorItemThen, the DelayedTypeApp branches were calling CallNameResolutionSink / CallExprHasTypeSink with the merged 'mExprAndTypeArgs' / 'mExprAndArg' range instead of the identifier range 'mItem'. The classifier then attached one wide Method/DisposableType classification covering the type-argument list (items 3 and 6) and even the constructor argument list (item 4), drowning out the punctuation/lambda that should remain neutral. Use 'mItem' - the same range the type-provider fast path already uses in TcMethodItemThen. The Types CNR in TcTypeItemThen is intentionally left alone (the qualified-name case fixed in #18009 / #19800 relies on it). For items 4 and 6 the wide CNR is registered by upstream paths (ForNewConstructors in TcCtorCall and CallMethodGroupNameResolutionSink in ResolveExprDotLongIdentAndComputeRange) that must keep the wide range for find-references and goto-definition. Add classification-only narrowing in SemanticClassification.fs based on the item's DisplayNameCore length: - Method/Property: narrow to the last `displayName.Length` characters of the CNR range (trailing identifier, e.g. `Start` in `MailboxProcessor<int>.Start`). - CtorGroup: narrow to the leading `displayName.Length` characters (type name comes first in `new T<typeArgs>(...)`). This leaves the CapturedNameResolutions untouched so symbol uses, tooltips and navigation continue to use the wide ranges they always did. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The existing takeCustomBuilder dedup only handled the exactly-2-CNR group (one Item.Value + one Item.CustomBuilder at the same range). Inside a list / array comprehension the captured name resolutions for the builder identifier can include more than two CNRs at the same range, and the fall-through path returned them all, leaving the Item.Value to dominate the classifier output and color the builder identifier as a plain Value. Generalize: at any group of CNRs that share a range, if at least one is Item.CustomBuilder, keep only the CustomBuilder ones. This preserves the existing 2-CNR behavior and additionally fixes the comprehension case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
For 'xs[0..]' (and the set-slice variants) on nominal types, TcIndexingThen synthesizes a DelayedDotLookup for GetSlice/SetSlice/Item. The synthesized identifier carried mWholeExpr (the whole bracketed expression including ']'), which the resolver then turned into a Method-classification CNR that the editor showed on the closing ']'. Syntheticize the identifier range and the DelayedDotLookup range so the CNR is filtered out by the existing IsSynthetic check in the resolutions pipeline. The DelayedApp ranges are left intact so error diagnostics keep their real ranges. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a regression guard for the case where `open type X` is used purely via an unqualified static-member call (e.g. `Button ...` after `open type Helpers.View`). On HEAD, UnusedOpens already detects this as used; this test pins the behavior so future refactors of ServiceAnalysis.fs cannot silently regress it. The existing enum-only coverage in `Unused opens in non rec module smoke test 1` did not exercise the class-with-static-members path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
commented
Jun 17, 2026
Superseded by #19960 (rebased). |
Fixes#19905
Type.Method<int>()/new T<args>(...)/T<args>.M(...)no longer classify<…>as Function/Type; span narrows to the identifier.list[0..]trailing]no longer classified as Function.[ for .. do builder { } ]classified as ComputationExpression, not Value.open type Tno longer always reported as unused.