Pass only narrow terminal-identifier range from name resolution#19818
Open
T-Gro wants to merge 24 commits into
Open
Pass only narrow terminal-identifier range from name resolution#19818T-Gro wants to merge 24 commits into
T-Gro wants to merge 24 commits into
Conversation
…14284, #3920) Narrow the FS0041 'No overloads match' error range from the whole expression to just the method name. For T.Instance.Method(""), the error now covers only 'Method' instead of 'T.Instance.Method("")'. Also narrow name-resolution sink ranges (used by Find Usages, symbol highlight, and semantic classification) to the terminal identifier of a dotted long identifier instead of the whole object-expression path. Changes: - NameResolution.fs: ComputeItemRange now returns (itemRange, itemIdentRange). itemRange is the structural whole-long-id span for typed-tree construction. itemIdentRange is the terminal identifier's range for diagnostics and sinks. - CheckExpressions.fs: Intercept UnresolvedOverloading errors in TcMethodApplication and narrow the error range to the method name. - ServiceParamInfoLocations.fs: Update getAllCurriedArgsAtPosition to handle narrowed symbol-use ranges by also checking the leaf function expression. Fixes #14284, #3920 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove the unused _mItemIdent parameter from TcItemThen signature, decomposing the 6-tuple result at call sites (TcLongIdentThen, TcTypeItemThen, TcNameOfExpr) to strip itemIdentRange before passing to TcItemThen. Addresses @abonie's feedback. - Improve ComputeItemRange doc comment to clarify that itemRange is consumed by TcItemThen for typed-tree construction/delayRest, while itemIdentRange is used only in name-resolution sink callbacks for IDE symbol reporting. Addresses @auduchinok's question. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bbc-9d1f-393ac1d7c4ba
Replace error-prone column arithmetic for overload-error narrowing with the terminal-identifier range (mItemIdent) that ComputeItemRange already computes. This addresses @auduchinok's review feedback to not add error-prone range calculations and instead use the known Method range. Changes: - Add mItemIdent parameter to TcMethodApplicationThen and TcMethodApplication - Thread mItemIdent from ResolveLongIdentAsExprAndComputeRange through TcLongIdentThen -> TcItemThen -> TcMethodItemThen - Thread mItemIdent from ResolveExprDotLongIdentAndComputeRange through TcLookupThen -> TcLookupItemThen - Simplify error narrowing in TcMethodApplication to just use mItemIdent - Remove DoesIdentifierNeedBackticks/mkPos/withStart workarounds - Update backtick-escaped test: now correctly narrows to the identifier (ComputeItemRange's idRange includes backtick delimiters) This also addresses @abonie's comment - the previously discarded mItemIdent parameter from ResolveLongIdentAsExprAndComputeRange is now used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…9048891-ac83c611-04a8-4bbc-9d1f-393ac1d7c4ba
Address review feedback: reduce verbose doc comment to concise description of itemRange vs itemIdentRange distinction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add mItemIdentOpt parameter to TcCtorCall so that constructor overload-error diagnostics narrow to the terminal identifier (e.g. 'String' in 'System.String(...)'). - Pass mItemIdent from TcCtorItemThen to TcCtorCall so that the narrowing is applied when resolving constructor overloads. - Revert E_Slices01.bsl: synthetic GetSlice idents carry the whole-expression range so narrowing is a no-op. - Update E_LessThanDotOpenParen001.bsl: .(+++) correctly narrows to the operator identifier +++. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert the broad sink-range narrowing that changed Find Usages, symbol highlighting, and parameter info behavior. Keep only the overload-resolution error range narrowing (FS0041) through TcMethodApplication, which is the focused fix for #14284. Reverted: - Sink calls in NameResolution.fs back to using itemRange - ServiceParamInfoLocations.fs workaround (getLeafFuncExpr) - FindReferences, Symbols, ProjectAnalysis, EditorTests expectations - Release notes and doc comments (removed #3920 scope) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove restating comments on mItemIdent parameters and TcItemThen - Trim verbose error-range narrowing comment to one line - Fix multiline test to assert actual range, not just error code - Remove trailing dead planning comments from test file - Shorten backtick test name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Condense .fsi doc comments to single lines - Remove ComputeItemRange doc comment (internal, self-explanatory) - Shorten error-narrowing comment to issue reference only - Add constructor overload error test covering TcCtorCall path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused _itemIdentRange from goto-def refinement path - Restore one-line comment explaining mItem vs mItemIdent on TcItemThen - Fix constructor test to use struct type and assert exact range Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ameter
Per Eugene's feedback ('Do we need all 3 ranges?'), eliminate the
separate mItemIdent parameter from TcMethodApplicationThen and
TcMethodApplication. Instead, callers pass the terminal identifier
range directly as mItem. This also eliminates the post-hoc error
rewrap since ResolveOverloadingForCall now receives the narrow
range directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Passing the narrow identifier range as mItem to TcMethodApplication also narrowed Obsolete/Experimental/implicit-conversion diagnostics (FS0101, FS3395, FS3387), breaking 15+ tests. ResolveOverloadingForCall produces multiple diagnostic types; only FS0041 should be narrowed. Restore mItemIdent as a separate parameter, keep mItem wide for other diagnostics, pass mMethExpr to ResolveOverloadingForCall, and rewrap only the UnresolvedOverloading error with the narrow range. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the mItemIdent parameter from TcMethodApplicationThen and TcMethodApplication. Callers now pass the terminal-identifier range directly as mItem, achieving the 3→2 range unification Eugene requested. Key changes: - TcMethodItemThen: pass mItemIdent where mItem was - TcCtorItemThen/TcCtorCall: pass mItemIdent, remove mItemIdentOpt - TcLookupItemThen: pass mItemIdent for instance methods/properties - Remove FS0041 rewrap logic (mItem already narrow) - Update Obsolete/Experimental test baselines for narrower ranges - Add 2 new Obsolete narrowing tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reset baselines to main and apply only the valid FS0041 range narrowings. The prior commit incorrectly included duplicate FS0001, FS0840, and FS1204 diagnostics that were artifacts of running TEST_UPDATE_BSL=1 with the in-process FCS API path (which emits duplicates due to dual attribute checking) rather than the FSC external process path (which stops after the first error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Thread mItemIdent through TcPropertyItemThen to match the pattern used by TcMethodItemThen, TcCtorItemThen, and TcLookupItemThen. Previously TcPropertyItemThen passed mItem (wide range) twice to TcMethodApplicationThen; now it passes mItemIdent (terminal ident) for the diagnostic range parameter. Add test for static indexed property overload error range. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Same fix as neg106.bsl — reset to main and apply only the valid FS0041 range narrowings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update test baselines across all platforms for diagnostics narrowed to terminal identifier range: - TypeDirectedConversionTests: Revert Warning 64 (FS0064) change - uses tp.Range not mItem, so not affected by narrowing - DefaultInterfaceMemberTests: 4 FS0629 ranges narrowed - ObsoleteAttributeCheckingTests: 2 property baselines Col 1 -> Col 7 - AttributeUsage: 4 FS0685 baselines narrowed - Basic accessibility: FS0629 MemberwiseClone Col 24 -> Col 26 - neg10.bsl: 4 FS1187 indexer ranges narrowed - neg45.bsl: 4 FS0685 ranges narrowed - neg101.bsl: 35 FS3220 tuple ItemN ranges narrowed + Rest range Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FS0120 and FS10021 (CompilerMessage attribute) on property access now points to the terminal property identifier instead of the full expression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On Desktop (net472), the CompilerMessage attribute check for property access fires from two code paths, producing both the wide expression range and the narrow terminal-identifier range. The error variant (FS10021) only fires once because error halts further attribute checking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the wider mItem range (covering all resolved long-ident segments) from NameResolution return types and TcItemThen dispatch. Only the narrow terminal-identifier range is now threaded through the type checking pipeline. Resolution sinks internally still use the wide range for IDE features like Parameter Info. - ResolveLongIdentAsExprAndComputeRange returns 5-tuple (was 6) - ResolveExprDotLongIdentAndComputeRange returns 4-tuple (was 5) - TcItemThen and all routing functions take one range instead of two - All mItemIdent references replaced with mItem (now always narrow) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adjusted diagnostic column ranges across FixedBindings, ObsoleteAttribute, TypeMismatch, NullableCsharpImport, StringInterpolation, Shadowing, and neg56_a tests to reflect narrower terminal-identifier ranges. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
❗ Release notes required
Warning No PR link found in some release notes, please consider adding it.
|
Replace duplicate (List.last lid).idRange computations with the already-computed itemIdentRange in both ResolveLongIdentAsExprAndComputeRange and ResolveExprDotLongIdentAndComputeRange (#16621 sink calls). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
AI-Tooling-Check-Bypassed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #19505 per Eugene's review feedback.
Removes the wide
mItemrange from the type checking pipeline, passing only the narrow terminal-identifier range (mItemIdent) everywhere.Changes
NameResolution.fsi/fs:
ResolveLongIdentAsExprAndComputeRange: returns 5-tuple (was 6) — drops onerangeResolveExprDotLongIdentAndComputeRange: returns 4-tuple (was 5) — drops onerangeitemRange(wide) is still computed and used for IDE sinks (CallMethodGroupNameResolutionSink,FilterMethodGroups,SendEntityPathToSink)CheckExpressions.fs:
TcItemThentuple reduced from 6 to 5 elementsmItemIdentparameter from all routing functions:TcMethodItemThen,TcCtorItemThen,TcPropertyItemThen,TcLookupItemThen,TcLookupThen,TcTypeItemThen,TcTyparExprThenTest baselines:
Safety
The wide range (
mItem) differs frommItemIdentonly whenrest = []inComputeItemRange(no delayed dot-lookups). IDE resolution sinks internally still receive the wide range. Expression nodes get narrower, more precise ranges.