Skip to content

Fix #16268: classify IDisposable in type-occurrence positions as Interface, not DisposableType#19809

Open
T-Gro wants to merge 3 commits into
mainfrom
fix/issue-16268
Open

Fix #16268: classify IDisposable in type-occurrence positions as Interface, not DisposableType#19809
T-Gro wants to merge 3 commits into
mainfrom
fix/issue-16268

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented May 26, 2026

Description

Fixes #16268

When a type like IDisposable appears in a type-occurrence position (e.g., type annotations, interface implementations), it was being classified as DisposableType instead of Interface. This caused incorrect semantic highlighting in editors.

Changes

  • src/Compiler/Service/SemanticClassification.fs: In the type-occurrence classification logic, check isInterfaceTy before isDisposableTy. If the type is an interface (like IDisposable), classify it as Interface rather than falling through to the DisposableType check.
  • ** ests/FSharp.Compiler.ComponentTests/Language/SemanticClassificationRegressions.fs**: Added regression tests verifying that IDisposable and other interface types in type annotations are correctly classified as Interface.

T-Gro and others added 2 commits May 26, 2026 10:40
…rface, 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>
@github-actions
Copy link
Copy Markdown
Contributor

❗ Release notes required

@T-Gro,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

You can open this PR in browser to add release notes: open in github.dev

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md No release notes found or release notes format is not correct

@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label May 26, 2026
Copy link
Copy Markdown
Member Author

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Looks good — clean, correct fix with solid test coverage.

The fix: Reordering isInterfaceTy before isDisposableTy is the right approach. Interface types (including IDisposable itself) should always be classified as Interface in semantic classification — the DisposableType classification is meant for concrete classes that implement IDisposable, not for interface types that happen to extend it.

Tests: Good coverage across four distinct scenarios: interface impl position, concrete disposable class (negative test guarding against regression), type constraint usage, and a custom non-IDisposable interface.

Minor nit (non-blocking): The test file is missing a trailing newline (the diff shows \ No newline at end of file).

@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

Inconsistent coloring of interface inheriting IDisposable

1 participant