Report a hand-written `__` self identifier to editor tooling by xperiandri · Pull Request #20598 · dotnet/fsharp

xperiandri · 2026-09-20T20:14:47Z

Fixes #20566

FCS reported uses of a member __.M() self identifier but not its declaration. Visual Studio's Rename therefore did nothing when invoked on the declaration, and when invoked on a use it rewrote the body and left member __. behind, producing code that no longer compiles.

The suppression was aimed at the self identifier that the auto-property desugaring synthesizes, which sits on the property name's own range and would shadow the property's own symbol. That one is now marked synthetic so the sink drops it on range, and a hand-written __ is reported like this, self and _ already were.

🤖 Generated with Claude Code

github-actions · 2026-09-20T20:14:58Z

✅ Release notes checked

✅ Found changes and release notes in following paths:

Change path Release notes path Description

`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.200.md

T-Gro

🤖 🕵️ LGTM

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Updates FCS symbol reporting so hand-written __ self identifiers are available to editor tooling while synthesized auto-property identifiers remain hidden.

Changes:

Reports declarations and uses of __ self identifiers.
Marks synthesized auto-property self identifiers as synthetic.
Adds regression tests and release notes.

File	Description
`tests/FSharp.Compiler.Service.Tests/Symbols.fs`	Adds coverage for self-identifier reporting and auto-property suppression.
`src/Compiler/Checking/Expressions/CheckExpressions.fs`	Removes the blanket `__` suppression.
`src/Compiler/Checking/CheckDeclarations.fs`	Marks generated auto-property self identifiers synthetic.
`docs/release-notes/.FSharp.Compiler.Service/11.0.200.md`	Documents the fix.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-29T02:57:45Z

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Changes IDE refactoring behavior.

Generated by PR Tooling Safety Check · gpt56 2.5M · ◷

T-Gro

🤖 🕵️ Re-reviewed all recent changes as of 767b3f5; 1 findings

T-Gro

The diff is still way too big - apply the NoBloat instructions and CodeCompaction skill.

Especially the massive code comments would benefit not just with drastic reduction, but ideally with well named functions/arguments/bindings/types making the intention clear.

If a well named piece of code just tells what the comment says - delete the comment.

Delete the 3 implementation/test comment lines. Replace the base-range explanation with a named predicate such as isUserDeclaredThisVal; the test name should state that the synthesized self identifier does not shadow the property range.
Compact the FCS release note to the observable fix: a hand-written __ self identifier is reported at its declaration. Remove the rename story, desugaring history, and synthetic-range mechanism.

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri · 2026-10-29T20:28:12Z

Compacted as requested in 32b7ec0, on top of current main (fbc9be3aa8).

All three comment lines are gone. The two about base became the name shadowsCtorParams; MakeAndPublishBaseVal is the only code that builds a base val and it gives it no member info, so the MemberInfo match went with them. The test comment is covered by the new test name, Synthesized auto-property self identifier does not shadow the property range.
I did not use isUserDeclaredThisVal: the guard no longer concerns a this-val, because the synthesized __ is dropped by its synthetic range.
The release note states only the observable fix: a hand-written __ self identifier is reported at its declaration.
The tests reuse chooseMemberOrFunctionOrValue, getRangeCoords and shouldEqual instead of hand-rolled matches.

The diff is +38/-12 (it was +49/-8) with no added comment lines (it was 3, 294 characters). FSharp.Compiler.Service.Tests (SurfaceArea, Symbols, EditorTests) and the FindReferences and SemanticClassificationRegressions component tests pass locally.

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

The compacted notes of dotnet#20598, dotnet#20599 and dotnet#20637 replace the lines they rewrote; the union merge had kept both. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

T-Gro

🤖🕵️ LGTM ✅

Dimensions covered (expand)

Exact merge and compaction delta (direct review)
Explicit self and base symbol reporting (direct review)
Public API compatibility (direct review and CI evidence)
Symbol, property, and completion regressions (CI evidence)

Approval withdrawn: current-head check suite 111005229142 is action_required. Build failure workflow tests must clear before approval. The technical source verdict is unchanged.

T-Gro

🤖🕵️ LGTM ✅

Dimensions covered (expand)

Exact merge and compaction delta (direct review)
Explicit self and base symbol reporting (direct review)
Public API compatibility (direct review and CI evidence)
Symbol, property, and completion regressions (CI evidence)

`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 Sonnet 5.5 <noreply@anthropic.com>

… tests The two comment lines about the range of `base` become the name `shadowsCtorParams`, which also makes the `MemberInfo` match redundant: only `MakeAndPublishBaseVal` builds a base val and it has none. The tests reuse `chooseMemberOrFunctionOrValue`, `getRangeCoords` and `shouldEqual`, and the release note states the observable fix only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

The ranges are an array now, and the tests use Assert.Equal and Assert.Empty instead of FSharp.Test.Assert.shouldEqual. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri · 2026-11-05T19:56:59Z

@T-Gro this branch was rebased onto current main (291f2bf28a) after your approval of 32b7ec0, so the approval does not cover the head any more. The only change in content since then is the review fix in ed5b6ed: the tests compare with collection asserts instead of shouldEqual. CI on ed5b6ed is green (51 of 51 jobs). Could you re-review?

xperiandri requested a review from a team as a code owner September 20, 2026 20:14

github-project-automation Bot added this to F# Compiler and Tooling Sep 20, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Sep 20, 2026

github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 20, 2026

xperiandri mentioned this pull request Sep 20, 2026

Reject _ as a rename target for a binding that is used #20599

Open

T-Gro approved these changes Sep 21, 2026

View reviewed changes

github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 21, 2026

T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 21, 2026

T-Gro self-requested a review September 21, 2026 08:45

xperiandri force-pushed the fix/self-identifier-symbol branch from 0ee7a07 to 4c9b110 Compare September 23, 2026 18:38

github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 23, 2026

This comment has been minimized.

Sign in to view

Copilot AI lite review requested due to automatic review settings September 24, 2026 18:33

Copilot started reviewing on behalf of xperiandri September 24, 2026 18:34

View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

xperiandri force-pushed the fix/self-identifier-symbol branch 2 times, most recently from ef9c825 to 767b3f5 Compare October 8, 2026 01:11

T-Gro reviewed Oct 9, 2026

View reviewed changes

Comment thread docs/release-notes/.FSharp.Compiler.Service/11.0.200.md Resolved

xperiandri force-pushed the fix/self-identifier-symbol branch from 767b3f5 to 32d5fd2 Compare October 10, 2026 22:28

T-Gro requested changes Oct 14, 2026

View reviewed changes

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 19, 2026

Bring dotnet#20598 up to its pull request head

fa446df

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 19, 2026

Bring dotnet#20598 up to its pull request head

c9b9aa4

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 23, 2026

Bring dotnet#20598 up to its pull request head

f08b6c3

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 23, 2026

Bring dotnet#20598 up to its pull request head

2e21a67

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri requested a review from T-Gro October 29, 2026 19:51

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Oct 29, 2026

Bring dotnet#20598 up to its pull request head

774151d

Applies what the pull request gained after e21fe4f. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

T-Gro previously approved these changes Oct 30, 2026

View reviewed changes

T-Gro approved these changes Oct 30, 2026

View reviewed changes

xperiandri commented Nov 5, 2026

View reviewed changes

Comment thread tests/FSharp.Compiler.Service.Tests/Symbols.fs Resolved

xperiandri and others added 4 commits November 5, 2026 18:53

Add release note

3e551d9

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Compare the self identifier ranges with collection asserts

ed5b6ed

The ranges are an array now, and the tests use Assert.Equal and Assert.Empty instead of FSharp.Test.Assert.shouldEqual. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

xperiandri force-pushed the fix/self-identifier-symbol branch from 32b7ec0 to ed5b6ed Compare November 5, 2026 18:17

Conversation

xperiandri commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Release notes checked

Uh oh!

T-Gro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Sep 29, 2026

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

xperiandri commented Oct 29, 2026

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xperiandri commented Nov 5, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 20, 2026 •

edited

Loading

T-Gro left a comment •

edited

Loading