From 974355233bdecb8dadbdabb642f90dca148524c0 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 26 May 2026 10:40:33 +0200 Subject: [PATCH 1/4] Fix #16268: classify IDisposable in type-occurrence positions as Interface, not DisposableType Reorder the classification checks in SemanticClassification.fs so that isInterfaceTy is checked before isDisposableTy. This ensures that interface types (including System.IDisposable itself) used in type-occurrence positions (e.g. `interface IDisposable with`, `#IDisposable` constraints, upcasts) are colorized as Interface rather than DisposableType. Concrete disposable classes such as MemoryStream remain classified as DisposableType via the unchanged Item.CtorGroup arm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Service/SemanticClassification.fs | 4 +- .../SemanticClassificationRegressions.fs | 82 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Service/SemanticClassification.fs b/src/Compiler/Service/SemanticClassification.fs index da0193e440..8d3b101d4b 100644 --- a/src/Compiler/Service/SemanticClassification.fs +++ b/src/Compiler/Service/SemanticClassification.fs @@ -321,7 +321,9 @@ module TcResolutionsExtensions = | Item.Types(_, ty :: _), LegitTypeOccurrence, m -> let ty = stripTyEqns g ty - if isDisposableTy g amap ty then + if isInterfaceTy g ty then + add m SemanticClassificationType.Interface + elif isDisposableTy g amap ty then add m SemanticClassificationType.DisposableType else match tryTcrefOfAppTy g ty with diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs index f2a2503da1..97517b310b 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs @@ -271,3 +271,85 @@ type MyDelegate = delegate of int -> string && (let text = substringOfRange source c.Range text = "BeginInvoke" || text = "EndInvoke")) Assert.Empty(asyncInvokeMethods) + +/// (#16268) IDisposable appearing in `interface IDisposable with` should be +/// classified as Interface, not DisposableType. +[] +let ``IDisposable in interface impl classified as interface`` () = + let source = """ +open System +type MyClass() = + interface IDisposable with + member _.Dispose() = () +""" + let classifications = getClassifications source + let idisposableOnLine4 = + classifications + |> Array.filter (fun c -> + c.Range.StartLine = 4 && substringOfRange source c.Range = "IDisposable") + Assert.True(idisposableOnLine4.Length > 0, "Expected at least one classification covering IDisposable on line 4") + Assert.True( + idisposableOnLine4 + |> Array.forall (fun c -> c.Type = SemanticClassificationType.Interface), + sprintf "Expected IDisposable to be classified as Interface, got: %A" + (idisposableOnLine4 |> Array.map (fun c -> c.Type))) + +/// (#16268) Negative: a concrete disposable class (MemoryStream) must NOT be +/// classified as Interface - the fix must not regress non-interface disposables. +[] +let ``Concrete disposable class not classified as interface`` () = + let source = """ +open System.IO +let s = new MemoryStream() +""" + let classifications = getClassifications source + let memStream = + classifications + |> Array.filter (fun c -> substringOfRange source c.Range = "MemoryStream") + Assert.True(memStream.Length > 0, "Expected at least one classification covering MemoryStream") + Assert.True( + memStream + |> Array.forall (fun c -> c.Type <> SemanticClassificationType.Interface), + sprintf "MemoryStream must not be classified as Interface, got: %A" + (memStream |> Array.map (fun c -> c.Type))) + +/// (#16268) IDisposable used as a type constraint should be Interface. +[] +let ``IDisposable as type constraint classified as interface`` () = + let source = """ +open System +let dispose (x: #IDisposable) = x.Dispose() +""" + let classifications = getClassifications source + let idisposable = + classifications + |> Array.filter (fun c -> substringOfRange source c.Range = "IDisposable") + Assert.True(idisposable.Length > 0, "Expected at least one classification covering IDisposable") + Assert.True( + idisposable + |> Array.forall (fun c -> c.Type = SemanticClassificationType.Interface), + sprintf "Expected IDisposable type-constraint occurrence to be Interface, got: %A" + (idisposable |> Array.map (fun c -> c.Type))) + +/// (#16268) Non-IDisposable interface in `interface ... with` position stays Interface. +/// This guards against accidentally narrowing the fix to IDisposable only. +[] +let ``Non-IDisposable interface classified as interface`` () = + let source = """ +type IMyInterface = + abstract member DoStuff: unit -> unit +type MyClass() = + interface IMyInterface with + member _.DoStuff() = () +""" + let classifications = getClassifications source + let iface = + classifications + |> Array.filter (fun c -> + substringOfRange source c.Range = "IMyInterface" && c.Range.StartLine = 5) + Assert.True(iface.Length > 0, "Expected at least one IMyInterface classification on line 5") + Assert.True( + iface + |> Array.forall (fun c -> c.Type = SemanticClassificationType.Interface), + sprintf "Expected IMyInterface on line 5 to be Interface, got: %A" + (iface |> Array.map (fun c -> c.Type))) From 82355d14a3735efc5e4422170bfa616287bb6815 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 26 May 2026 11:13:21 +0200 Subject: [PATCH 2/4] Apply remaining changes --- .executor-pid | 1 + 1 file changed, 1 insertion(+) create mode 100644 .executor-pid diff --git a/.executor-pid b/.executor-pid new file mode 100644 index 0000000000..ae8c553f18 --- /dev/null +++ b/.executor-pid @@ -0,0 +1 @@ +23212 \ No newline at end of file From c8f61ff2300a3b270f57b83d09c44851ef4ee6b8 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 26 May 2026 14:30:36 +0200 Subject: [PATCH 3/4] Remove executor artifact --- .executor-pid | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .executor-pid diff --git a/.executor-pid b/.executor-pid deleted file mode 100644 index ae8c553f18..0000000000 --- a/.executor-pid +++ /dev/null @@ -1 +0,0 @@ -23212 \ No newline at end of file From 298dba9c543490c930978e2c02472993c6f67015 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 14:44:28 +0000 Subject: [PATCH 4/4] Add release notes for #16268 fix and add trailing newline to test file Co-authored-by: T-Gro <46543583+T-Gro@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 43601b6e34..314330ea51 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -67,6 +67,7 @@ * Reference assembly MVIDs are now deterministic across compiler invocations. Previously, `--refout` / `true` produced a different MVID every build because the implied signature hash used .NET's randomized `String.GetHashCode()`. ([Issue #19751](https://github.com/dotnet/fsharp/issues/19751), [PR #19801](https://github.com/dotnet/fsharp/pull/19801)) * Parser: recover on unfinished if and binary expressions ([PR #19724](https://github.com/dotnet/fsharp/pull/19724)) +* Fix semantic classification of `IDisposable` and other interface types in type-occurrence positions being incorrectly classified as `DisposableType` instead of `Interface`. ([Issue #16268](https://github.com/dotnet/fsharp/issues/16268), [PR #19809](https://github.com/dotnet/fsharp/pull/19809)) ### Added