Reject _ as a rename target for a binding that is used - #20599
Open
xperiandri wants to merge 8 commits into
Open
xperiandri wants to merge 8 commits into
xperiandri wants to merge 8 commits into
Conversation
`MakeAndPublishVal` skipped the sink for any `MemberThisVal` literally named `__`. The target was the self identifier that the auto-property desugaring synthesizes, which sits on the *property name's* range and would therefore shadow the property's own symbol -- a range collision, not a name problem. Filtering by name also hid every `member __.M()` a user writes. Declarations go through `MakeAndPublishVal` and were suppressed, while uses in the body resolve normally and were not, so FCS reported the uses of `__` without its declaration. Editor rename then rewrote the body and left `member __.` behind, silently producing code that no longer compiles. Mark the synthesized ident's range synthetic instead, as the adjacent backing field already does. `TcResultsSinkImpl.allowedRange` drops synthetic ranges for every sink notification, so the auto-property self identifier stays invisible and the name check becomes dead code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_` is not a referenceable name in F#: in expression position `_.M()` is the shorthand lambda, not a reference. Renaming a self identifier (or any local) that the body actually uses therefore produced a file that no longer compiles. Accept `_` only when the rename rewrites nothing but the declaration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FCS reports both the declaration and the uses of a healthy symbol, so a declaration that sits in the file being renamed yet is absent from that file's uses means the rename would rewrite the uses and leave the declaration behind -- the failure mode `member __.M()` just exhibited. Refuse to start the rename instead of corrupting the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing covered InlineRenameService before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Release notes checked
Warning No PR link found in some release notes, please consider adding it.
|
github-actions
Bot
added
the
⚠️ Affects-Design-Time
This comment has been minimized.
This comment has been minimized.
FSharp.Editor consumes Microsoft.CodeAnalysis.EditorFeatures with ExcludeAssets="runtime" because Visual Studio supplies it at run time. Nothing put it in the test output, so touching a rename type threw FileNotFoundException before any assertion ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`__.TestMethod()` also matched the declaration one line above, whose own self identifier has no uses, so the test placed the caret there and found one location instead of two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Design-Time, Affects-Restore
|
github-actions
Bot
added
⚠️ Affects-Build-Infra
T-Gro
reviewed
Sep 21, 2026
T-Gro
left a comment
T-Gro
left a comment
Member
There was a problem hiding this comment.
🤖 🕵️ AI review — verify independently.
| match symbolUse.Symbol.DeclarationLocation with | ||
| | Some declRange when String.Equals(declRange.FileName, symbolUse.Range.FileName, StringComparison.Ordinal) -> | ||
| checkFileResults.GetUsesOfSymbolInFile(symbolUse.Symbol, cancellationToken = ct) | ||
| |> Array.exists (fun su -> Range.equals su.Range declRange) |
Member
There was a problem hiding this comment.
🤖 🕵️ Rename refuses active-pattern cases in their declaring file — renaming Even to DivisibleByTwo worked before, but the whole-pattern declaration range never matches an individual case range.
module M
let (|Even|Odd|) n =
if n % 2 = 0 then Even else Odd
let f n =
match n with
| Even -> true
| Odd -> false| // `_` names a binding nothing refers to: in expression position `_.M()` is the shorthand lambda | ||
| let replacementTextValid = | ||
| Tokenizer.isValidNameForSymbol (symbolKind, symbol, replacementText) | ||
| && not (replacementText = "_" && locations.Length > 1) |
Member
There was a problem hiding this comment.
🤖 🕵️ Rename rejects an explicitly backticked underscore after normalizing it to bare _ — the escaped identifier remains referenceable, and this replacement compiled and was accepted before.
// Rename value to ``_``:
module M
let value = 1
let answer = value + 1
// Valid replacement, now marked invalid:
// let ``_`` = 1
// let answer = ``_`` + 1
T-Gro
self-requested a review
September 21, 2026 09:31
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
2 participants
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.
Fixes #20597
Stacked on #20598
_is not a referenceable name in F# — in expression position_.M()is the shorthand lambda — so renaming a self identifier (or any local) that its body uses produced a file that no longer compiles. Rename now accepts_only when nothing but the declaration would be rewritten.Rename also refuses outright when the symbol's declaration sits in the file being renamed but is missing from the locations to rewrite, rather than rewriting the uses and leaving the declaration behind.
🤖 Generated with Claude Code