Skip to content

Commit 01bad01

Browse files
T-GroT-Gro
authored
Find All References and Rename Symbol Bug Fixes (dotnet#19252)
1 parent 8de1b33 commit 01bad01

33 files changed

Lines changed: 1041 additions & 790 deletions

File tree

‎.github/agents/fsharp-generic.md‎

Lines changed: 44 additions & 460 deletions
Large diffs are not rendered by default.

‎.github/copilot-instructions.md‎

Lines changed: 36 additions & 191 deletions
Original file line numberDiff line numberDiff line change
@@ -1,206 +1,51 @@
1-
# GitHub Copilot Instructions for F# Compiler
1+
# F# Compiler
22

3-
## DEBUGGING MINDSET - CRITICAL
3+
## Build
44

5-
**Your changes are the cause until proven otherwise.**
6-
7-
When encountering test failures or build issues after making changes:
8-
9-
1. **NEVER assume "pre-existing failure"** - This is incorrect 99% of the time
10-
2. **ALWAYS assume your PR diff caused the issue** - Even if it seems unrelated
11-
3. **Remember the bootstrap**: The F# compiler compiles itself. If you introduced broken code in earlier commits, even if you "reverted" it later, the bootstrap compiler may be using the broken version
12-
4. **Clean and rebuild**: When in doubt, `git clean -xfd artifacts` and rebuild from scratch to eliminate bootstrap contamination
13-
5. **Compare your diff**: Use `git diff <base_commit> HEAD` to see ALL changes in your PR, not just the latest commit
14-
6. **Verify with original code**: Temporarily revert your changes to confirm tests pass without them
15-
16-
**Forbidden phrases:**
17-
- "pre-existing issue"
18-
- "was already broken"
19-
- "not related to my changes"
20-
- "known limitation"
21-
22-
**Required verification before claiming something was already broken:**
23-
1. Clean build artifacts completely
24-
2. Checkout the base branch
25-
3. Build and run the same test
26-
4. Document the failure with the base branch commit hash
27-
28-
Only after this verification can you legitimately claim a pre-existing issue.
29-
30-
---
31-
32-
## STRUCTURE YOUR CHANGE (BEFORE EDITING)
33-
Keep scope tight.
34-
General guide:
35-
- Use F#
36-
- Target .NET Standard 2.0 for compatibility
37-
- Avoid external dependencies – the codebase is self-contained (do NOT add new NuGet packages)
38-
- Follow docs/coding-standards.md and docs/overview.md
39-
40-
**Test‑First** (bugs / regressions): Add/adjust a minimal test that fails on current main → confirm it fails → implement fix → run core command and ensure test passes → only then continue.
41-
42-
Plan your task:
43-
1. Write a 1–2 sentence intent (bug fix / API add / language tweak).
44-
2. Identify domain: Language (`LanguageFeature.fsi` touched) vs `src/FSharp.Core/` vs `vsintegration/` vs compiler/service.
45-
3. Public API? Edit matching `.fsi` simultaneously.
46-
4. New/changed diagnostics? Update FSComp.txt.
47-
5. IL shape change expected? Plan ILVerify baseline update.
48-
6. Expect baseline diffs? Plan `TEST_UPDATE_BSL=1` run.
49-
7. Add/adjust tests in existing projects.
50-
8. Decide release-notes sink now (Section 8).
51-
9. Run formatting only at the end.
52-
53-
---
54-
55-
# AFTER CHANGING CODE ( Agent-only. Ubuntu only )
56-
57-
Always run the core command. Always verify exit codes. No assumptions.
58-
59-
## 1. Core Command
5+
Default (set `BUILDING_USING_DOTNET=true` system-wide):
6+
```bash
7+
dotnet build <changed>.fsproj -c Debug
608
```
61-
./build.sh -c Release --testcoreclr
62-
```
63-
Non‑zero → classify & stop.
64-
65-
### CRITICAL TEST EXECUTION RULES
66-
**ALWAYS** run tests before claiming success. **NEVER** mark work complete without verified passing tests.
67-
68-
When running tests, **ALWAYS** report:
69-
- Total number of tests executed
70-
- Number passed / failed / skipped
71-
- Execution duration
72-
- Example: "Ran 5 tests: 5 passed, 0 failed, 0 skipped. Duration: 4.2 seconds"
73-
74-
**ASSUME YOUR CODE IS THE PROBLEM**: When tests fail, ALWAYS assume your implementation is incorrect FIRST. Only after thorough investigation with evidence should you consider other causes like build issues or test infrastructure problems.
75-
76-
**UNDERSTAND WHAT YOU'RE TESTING**: Before writing tests, understand exactly what behavior the feature controls. Research the codebase to see how the feature is actually used, not just how you think it should work.
77-
78-
**TEST INCREMENTALLY**: After each code change, immediately run the relevant tests to verify the change works as expected. Don't accumulate multiple changes before testing.
9+
Get target framework: `dotnet msbuild <proj> -getProperty:TargetFrameworks`
10+
FSharp.Core + compiler composite: `./build.sh -c Release`
11+
FSharp.Build changes: `./build.sh -c Release`
7912

80-
## 2. Bootstrap (Failure Detection Only)
81-
Two-phase build. No separate bootstrap command.
82-
Early proto/tool errors (e.g. "Error building tools") → `BootstrapFailure` (capture key lines). Stop.
13+
## No bullshit
8314

84-
## 3. Build Failure
85-
Proto ok but solution build fails → `BuildFailure`.
86-
Capture exit code, ≤15 error lines (`error FS`, `error F#`, `error MSB`), binlog path: `artifacts/log/Release/Build.*.binlog`.
87-
Do not proceed to tests.
15+
Build fails → 99% YOUR previous change broke it. You ARE the compiler.
16+
DON'T say "pre-existing", "infra issue", "unrelated".
17+
DO `git clean -xfd artifacts` and rebuild.
18+
Bootstrap contamination: early commits break compiler → later "fixes" still use broken bootstrap. Clean fully.
8819

89-
## 4. Tests
90-
Core command runs CoreCLR tests:
91-
- FSharp.Test.Utilities
92-
- FSharp.Compiler.ComponentTests
93-
- FSharp.Compiler.Service.Tests
94-
- FSharp.Compiler.Private.Scripting.UnitTests
95-
- FSharp.Build.UnitTests
96-
- FSharp.Core.UnitTests
97-
Failures → `TestFailure` (projects + failing lines + baseline hints).
20+
## Test
9821

99-
## 5. Baselines
100-
Drift → update then re-run.
22+
Default: `-c Debug`
23+
Use `-c Release` for: EmittedIL tests, Optimizer tests, full component runs
24+
spot check: `dotnet test <proj> --filter "Name~X" -c Debug`
25+
full component: `dotnet test tests/FSharp.Compiler.ComponentTests -c Release`
26+
IDE/service: `tests/FSharp.Compiler.Service.Tests`
27+
VS integration: `vsintegration/` (Windows only)
28+
update baselines: `TEST_UPDATE_BSL=1 <test command>`
10129

102-
General/component:
103-
```
104-
TEST_UPDATE_BSL=1
105-
./build.sh -c Release --testcoreclr
106-
```
107-
Surface area:
108-
```
109-
TEST_UPDATE_BSL=1
110-
dotnet test tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj --filter "SurfaceAreaTest" -c Release /p:BUILDING_USING_DOTNET=true
111-
```
112-
ILVerify:
113-
```
114-
TEST_UPDATE_BSL=1
115-
pwsh tests/ILVerify/ilverify.ps1
116-
```
117-
Classify: `BaselineDrift(SurfaceArea|ILVerify|GeneralBSL)` + changed files.
30+
## Spotcheck tests
11831

119-
## 6. Formatting
120-
```
121-
dotnet fantomas . --check
122-
```
123-
If fail:
124-
```
125-
dotnet fantomas .
126-
dotnet fantomas . --check
127-
```
128-
Still failing → `FormattingFailure`.
32+
- find new tests for bugfix/feature
33+
- find preexisting tests in same area
34+
- run siblings/related
12935

130-
## 7. Public API / IL
131-
If new/changed public symbol (`.fsi` touched or public addition):
132-
1. Update `.fsi`.
133-
2. Surface area baseline flow.
134-
3. ILVerify if IL shape changed.
135-
4. Release notes (Section 8).
136-
Missed baseline update → `BaselineDrift`.
36+
## Final validation (Copilot Coding Agent only)
13737

138-
## 8. Release Notes (Sink Rules – Compact)
139-
Most fixes → FSharp.Compiler.Service.
38+
Before submitting: `./build.sh -c Release --testcoreclr`
14039

141-
| Condition | Sink |
142-
|-----------|------|
143-
| `LanguageFeature.fsi` changed | Language |
144-
| Public API/behavior/perf change under `src/FSharp.Core/` | FSharp.Core |
145-
| Only `vsintegration/` impacted | VisualStudio |
146-
| Otherwise | FSharp.Compiler.Service |
40+
## Code
14741

148-
Action each needed sink:
149-
- Append bullet in latest version file under `docs/release-notes/<Sink>/`
150-
- Format: `* Description. ([PR #NNNNN](https://github.com/dotnet/fsharp/pull/NNNNN))`
151-
- Optional issue link before PR.
152-
Missing required entry → `ReleaseNotesMissing`.
42+
.fs: implementation
43+
.fsi: declarations, API docs, context comments
15344

154-
## 9. Classifications
155-
Use one or more exactly:
156-
- `BootstrapFailure`
157-
- `BuildFailure`
158-
- `TestFailure`
159-
- `FormattingFailure`
160-
- `BaselineDrift(SurfaceArea|ILVerify|GeneralBSL)`
161-
- `ReleaseNotesMissing`
45+
## Rules
16246

163-
Schema:
164-
```
165-
Classification:
166-
Command:
167-
ExitCode:
168-
KeySnippets:
169-
ActionTaken:
170-
Result:
171-
OutstandingIssues:
172-
```
173-
174-
## 10. Decision Flow
175-
1. Format check
176-
2. Core command
177-
3. If fail classify & stop
178-
4. Tests → `TestFailure` if any
179-
5. Baseline drift? update → re-run → classify if persists
180-
6. Public surface/IL? Section 7
181-
7. Release notes sink (Section 8)
182-
8. If no unresolved classifications → success summary
183-
184-
## 11. Success Example
185-
```
186-
AllChecksPassed:
187-
Formatting: OK
188-
Bootstrap: OK
189-
Build: OK
190-
Tests: Passed
191-
Baselines: Clean
192-
ReleaseNotes: FSharp.Compiler.Service
193-
```
194-
195-
## 12. Failure Example
196-
```
197-
BootstrapFailure:
198-
Command: ./build.sh -c Release --testcoreclr
199-
ExitCode: 1
200-
KeySnippets:
201-
- "Error building tools"
202-
ActionTaken: None
203-
Result: Stopped
204-
OutstandingIssues: Bootstrap must be fixed
205-
```
206-
(eof)
47+
Public API change → update .fsi
48+
New diagnostic → update `src/Compiler/FSComp.txt`
49+
API surface change → `TEST_UPDATE_BSL=1 dotnet test tests/FSharp.Compiler.Service.Tests --filter "SurfaceAreaTest" -c Release`
50+
After code changes → `dotnet fantomas .`
51+
When fully done → write release notes (see skill)

‎.github/skills/ilverify-failure/SKILL.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
name: ilverify-failure
3+
description: Fix ILVerify baseline failures when IL shape changes (codegen, new types, method signatures). Use when CI fails on ILVerify job.
4+
---
5+
6+
# ILVerify Baseline
7+
8+
## When to Use
9+
IL shape changed (codegen, new types, method signatures) and ILVerify CI job fails.
10+
11+
## Update Baselines
12+
```bash
13+
TEST_UPDATE_BSL=1 pwsh tests/ILVerify/ilverify.ps1
14+
```
15+
16+
## Baselines Location
17+
`tests/ILVerify/*.bsl`
18+
19+
## Verify
20+
Re-run without `TEST_UPDATE_BSL=1`, should pass.

‎.github/skills/release-notes/SKILL.md‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
name: release-notes
3+
description: Write release notes for completed changes. Use when PR modifies tracked paths and needs release notes entry.
4+
---
5+
6+
# Release Notes
7+
8+
## Version
9+
From GitHub repo variable `VNEXT` (e.g., `10.0.300`)
10+
- Language: `preview.md`
11+
- VisualStudio: `<VSMajorVersion>.vNext.md`
12+
13+
## Path
14+
`docs/release-notes/.<Sink>/<Version>.md`
15+
16+
## Sink Mapping
17+
- LanguageFeatures.fsi → `.Language`
18+
- src/FSharp.Core/ → `.FSharp.Core`
19+
- vsintegration/src/ → `.VisualStudio`
20+
- src/Compiler/ → `.FSharp.Compiler.Service`
21+
22+
## Format (Keep A Changelog)
23+
```markdown
24+
### Fixed
25+
* Bug fix description. ([Issue #NNN](...), [PR #NNN](...))
26+
27+
### Added
28+
* New feature description. ([PR #NNN](...))
29+
30+
### Changed
31+
* Behavior change description. ([PR #NNN](...))
32+
33+
### Breaking Changes
34+
* Breaking change description. ([PR #NNN](...))
35+
```
36+
37+
## Entry Format
38+
- Basic: `* Description. ([PR #NNNNN](https://github.com/dotnet/fsharp/pull/NNNNN))`
39+
- With issue: `* Description. ([Issue #NNNNN](...), [PR #NNNNN](...))`
40+
41+
## CI Check
42+
PR fails if changes in tracked paths without release notes entry containing PR URL.
43+
Add `NO_RELEASE_NOTES` label to skip.

‎docs/release-notes/.FSharp.Compiler.Service/10.0.300.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
### Fixed
22

3+
* Fixed Find All References not correctly finding active pattern cases in signature files. ([Issue #19173](https://github.com/dotnet/fsharp/issues/19173), [Issue #14969](https://github.com/dotnet/fsharp/issues/14969), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
4+
* Fixed Rename not correctly handling operators containing `.` (e.g., `-.-`). ([Issue #17221](https://github.com/dotnet/fsharp/issues/17221), [Issue #14057](https://github.com/dotnet/fsharp/issues/14057), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
5+
* Fixed Find All References not correctly applying `#line` directive remapping. ([Issue #9928](https://github.com/dotnet/fsharp/issues/9928), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
6+
* Fixed `SynPat.Or` pattern variables (non-left-most) incorrectly classified as bindings instead of uses. ([Issue #5546](https://github.com/dotnet/fsharp/issues/5546), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
7+
* Fixed Find All References not finding discriminated union types defined inside modules. ([Issue #5545](https://github.com/dotnet/fsharp/issues/5545), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
8+
* Fixed synthetic event handler values appearing in Find All References results. ([Issue #4136](https://github.com/dotnet/fsharp/issues/4136), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
9+
* Fixed Find All References not finding all usages of C# extension methods. ([Issue #16993](https://github.com/dotnet/fsharp/issues/16993), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
10+
* Fixed Find All References on discriminated union cases not including case tester properties (e.g., `.IsCase`). ([Issue #16621](https://github.com/dotnet/fsharp/issues/16621), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
11+
* Fixed Find All References on record types not including copy-and-update expressions. ([Issue #15290](https://github.com/dotnet/fsharp/issues/15290), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
12+
* Fixed Find All References on constructor definitions not finding all constructor usages. ([Issue #14902](https://github.com/dotnet/fsharp/issues/14902), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
313
* Fix false FS1182 (unused variable) warning for query expression variables used in where, let, join, and select clauses. ([Issue #422](https://github.com/dotnet/fsharp/issues/422))
414
* Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))
515
* Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))

‎docs/release-notes/.VisualStudio/18.vNext.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
### Fixed
22

3+
* Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
4+
* Fixed Find All References crash when F# project contains non-F# files like `.cshtml`. ([Issue #16394](https://github.com/dotnet/fsharp/issues/16394), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
5+
* Find All References for external DLL symbols now only searches projects that reference the specific assembly. ([Issue #10227](https://github.com/dotnet/fsharp/issues/10227), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))
6+
37
### Changed

‎src/Compiler/Checking/AugmentWithHashCompare.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,7 @@ let MakeValsForUnionAugmentation g (tcref: TyconRef) =
16781678
|> List.map (fun uc ->
16791679
// Unlike other generated items, the 'IsABC' properties are visible, not considered compiler-generated
16801680
let v =
1681-
mkImpliedValSpec g uc.Range tcref tmty vis None ("get_Is" + uc.CompiledName) (tps +-> (mkIsCaseTy g tmty)) unitArg true
1681+
mkImpliedValSpec g uc.Range tcref tmty vis None (PrettyNaming.unionCaseTesterPropertyPrefix + uc.CompiledName) (tps +-> (mkIsCaseTy g tmty)) unitArg true
16821682

16831683
g.AddValGeneratedAttributes v m
16841684
v)

‎src/Compiler/Checking/CheckPatterns.fs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,10 @@ and TcPatBindingName cenv env id ty isMemberThis vis1 valReprInfo (vFlags: TcPat
251251

252252
// isLeftMost indicates we are processing the left-most path through a disjunctive or pattern.
253253
// For those binding locations, CallNameResolutionSink is called in MakeAndPublishValue, like all other bindings
254-
// For non-left-most paths, we register the name resolutions here
254+
// For non-left-most paths, we register the name resolutions here as Use, not Binding (#5546)
255255
if not isLeftMost && not vspec.IsCompilerGenerated && not (vspec.LogicalName.StartsWithOrdinal("_")) then
256256
let item = Item.Value(mkLocalValRef vspec)
257-
CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Binding, env.AccessRights)
257+
CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.AccessRights)
258258

259259
PatternValBinding(vspec, typeScheme)
260260

0 commit comments

Comments
 (0)