From d17f110f95b02d32e6100b1dda8991bf73447114 Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 9 Jun 2026 08:29:01 +0200 Subject: [PATCH 1/3] test: add failing regression tests for #5229 (recursive self-ref MutableVar) Adds positive and negative coverage for SemanticClassification of `as this` self-references in class bodies. These tests currently fail because isValRefMutable in SemanticClassification.fs treats the internal ref-cell compilation of CtorThisVal as a user-written mutable binding. Sprint 02 applies the surgical fix to make them pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SemanticClassificationRegressions.fs | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs index f2a2503da1f..9d287320125 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs @@ -271,3 +271,111 @@ type MyDelegate = delegate of int -> string && (let text = substringOfRange source c.Range text = "BeginInvoke" || text = "EndInvoke")) Assert.Empty(asyncInvokeMethods) + +// -------------------------------------------------------------------------- +// #5229: recursive object self-reference must NOT be classified as MutableVar. +// The compiler internally compiles `as this` / recursive `let rec` self refs +// through a ref cell; that compiler artefact must not leak into colorization. +// Parallel logic already lives in Symbols.fs (FSharpMemberOrFunctionOrValue.IsMutable +// excludes vref.IsCtorThisVal from the isRefCellTy branch). Sprint 02 mirrors +// that exclusion in SemanticClassification.isValRefMutable. +// -------------------------------------------------------------------------- + +/// Return every classification item whose source span equals exactly `ident`. +let private classificationsForIdent (source: string) (ident: string) = + getClassifications source + |> Array.filter (fun item -> + item.Range.StartLine = item.Range.EndLine + && substringOfRange source item.Range = ident) + +/// Assert no occurrence of `ident` in `source` is classified as MutableVar. +let private assertIdentNeverMutable (source: string) (ident: string) = + let mutableHits = + classificationsForIdent source ident + |> Array.filter (fun item -> item.Type = SemanticClassificationType.MutableVar) + Assert.True( + mutableHits.Length = 0, + sprintf + "#5229: %A occurrences of `%s` were classified as MutableVar, expected none. Items: %A" + mutableHits.Length + ident + (mutableHits |> Array.map (fun i -> i.Range.StartLine, i.Range.StartColumn, i.Range.EndColumn, i.Type))) + +/// (#5229) Primary case: `as this` on a class binds `this` to a CtorThisVal whose +/// internal compilation is a ref cell. That compiler artefact must NOT leak as MutableVar. +[] +let ``5229 class as this is not MutableVar`` () = + let source = """ +module Test +type C() as this = + member _.F() = this +""" + assertIdentNeverMutable source "this" + +/// (#5229) Variants where the `as this` self-reference must not classify as MutableVar. +/// Each row is (label, source, identifier-that-must-not-be-MutableVar). +[] +[(value: 'T) as this = + member _.Value = value + member _.Self = this +""", "this")>] +[] +let ``5229 as this variants are not MutableVar`` (_label: string) (source: string) (ident: string) = + assertIdentNeverMutable source ident + +/// (#5229 negative) Real mutable / ref-cell / byref bindings MUST still be classified as MutableVar. +/// Each row is (label, source, identifier-that-must-be-MutableVar). +[] +[] +[] +let ``5229 real mutable bindings remain MutableVar`` (_label: string) (source: string) (ident: string) = + let items = classificationsForIdent source ident + let mutableHits = + items |> Array.filter (fun item -> item.Type = SemanticClassificationType.MutableVar) + Assert.True( + mutableHits.Length > 0, + sprintf + "#5229 negative guard: expected at least one MutableVar classification for `%s`. Items: %A" + ident + (items |> Array.map (fun i -> i.Range.StartLine, i.Range.StartColumn, i.Range.EndColumn, i.Type))) + +/// (#5229 negative) Mutable record fields are an orthogonal code path +/// (isRecdFieldMutable, not isValRefMutable). They MUST be unaffected by the upcoming fix. +[] +let ``5229 mutable record field updates remain MutableRecordField`` () = + let source = """ +module Test +type R = { mutable Count: int } +let r = { Count = 0 } +let () = r.Count <- r.Count + 1 +""" + let items = getClassifications source + let mutableFieldHits = + items + |> Array.filter (fun item -> + item.Type = SemanticClassificationType.MutableRecordField + && substringOfRange source item.Range = "Count") + Assert.True( + mutableFieldHits.Length > 0, + sprintf "Expected MutableRecordField classification for `Count`, found: %A" + (items |> Array.map (fun i -> i.Range.StartLine, i.Range.StartColumn, i.Range.EndColumn, i.Type))) From d6c1414a64373d1d1f56abd6fca679eb6c0e8beb Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 9 Jun 2026 08:53:51 +0200 Subject: [PATCH 2/3] fix #5229: do not classify CtorThisVal ref-cell self refs as MutableVar The compiler compiles recursive object self-references (`as this`, recursive `let rec` self-refs) through a ref cell for initialization. SemanticClassification.isValRefMutable previously treated that internal ref cell as a user-written mutable binding, causing the IDE colorizer to paint the self-reference with the MutableVar shade. Mirror the existing guard in FSharpMemberOrFunctionOrValue.IsMutable (src/Compiler/Symbols/Symbols.fs) by excluding vref.IsCtorThisVal from the isRefCellTy branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Service/SemanticClassification.fs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Service/SemanticClassification.fs b/src/Compiler/Service/SemanticClassification.fs index da0193e4405..03353bb884e 100644 --- a/src/Compiler/Service/SemanticClassification.fs +++ b/src/Compiler/Service/SemanticClassification.fs @@ -91,8 +91,12 @@ module TcResolutionsExtensions = let isValRefMutable g (vref: ValRef) = // Mutable values, ref cells, and non-inref byrefs are mutable. + // Exclude CtorThisVal: the compiler wraps recursive object self-references + // (`as this`, `let rec` self-refs) in a ref cell for initialization, but the + // user-visible binding is immutable. Mirrors FSharpMemberOrFunctionOrValue.IsMutable + // in src/Compiler/Symbols/Symbols.fs. vref.IsMutable - || isRefCellTy g vref.Type + || (not vref.IsCtorThisVal && isRefCellTy g vref.Type) || (isByrefTy g vref.Type && not (isInByrefTy g vref.Type)) let isRecdFieldMutable g (rfinfo: RecdFieldInfo) = From a910d706293756d8b0a20e7dff0d01199e43732a Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 9 Jun 2026 12:21:16 +0200 Subject: [PATCH 3/3] polish #5229: fantomas + release notes Apply dotnet fantomas to the SemanticClassification fix and its regression tests (both already conformant, no diff); add a release-notes entry for the service-side change. No production logic changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 965957dbc0c..be446dc5632 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Semantic classification no longer marks recursive object self-references (`as this`, `let rec` self-refs) as mutable. ([Issue #5229](https://github.com/dotnet/fsharp/issues/5229)) * Suppress hover/symbol resolution for wildcard `_` patterns inside `member _.…` bodies that incorrectly showed `val _: T` tooltip. ([PR #19760](https://github.com/dotnet/fsharp/pull/19760)) * Deduplicate format specifier locations in computation expressions so editor tooling no longer reports duplicate entries for the same `%` specifier. ([Issue #16419](https://github.com/dotnet/fsharp/issues/16419), [PR #19791](https://github.com/dotnet/fsharp/pull/19791)) * Reject non-function bindings for single-case and partial active pattern names with FS1209, matching the existing multi-case behavior. ([PR #19763](https://github.com/dotnet/fsharp/pull/19763))