Skip to content

Conversation

@jakebailey
Copy link
Member

@jakebailey jakebailey commented Nov 3, 2025

Fixes #2001 (mostly)

Issues:

  • The checker's ported code was for the non-suggestion side of things, meaning it entirely skipped these diags when the noUnusedLocals/noUnusedParameters was off, so the grayed out suggestion errors were never even calculated.
  • reportUnused didn't actually use suggestionDiagnostics at all, nor have the bit that sets the category to Suggestion on the fly; I re-ported that code.
  • The LSP code for checking client capabilities for diagnostics was "correct", except apparently the VS Code LSP client does not send the full set of capabilities for both push and pull diagnostics (specifically, tag support). Pull the info from publishDiagnostics as a backup, which is a hack but works around the problem until we can get v10 of the LSP client.
    "publishDiagnostics": {
        "relatedInformation": true,
        "versionSupport": false,
        "tagSupport": {
            "valueSet": [
                1,
                2
            ]
        },
        "codeDescriptionSupport": true,
        "dataSupport": true
    },
    "diagnostic": {
        "dynamicRegistration": true,
        "relatedDocumentSupport": false
    }
    

The warning vs error thing is actually a VS Code setting, something we will have to do once those are passed in and handled.

Copilot AI review requested due to automatic review settings November 3, 2025 16:46
@jakebailey jakebailey requested a review from ahejlsberg November 3, 2025 16:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes how unused identifier diagnostics are handled based on compiler options. Previously, unused variables were only reported as errors when noUnusedLocals or noUnusedParameters flags were enabled. Now, unused identifiers are always checked and reported either as errors (when the respective flag is enabled) or as suggestions (when disabled).

Key changes:

  • Unused identifiers are now always checked regardless of compiler options
  • When noUnusedLocals/noUnusedParameters are disabled, diagnostics are reported as suggestions instead of being omitted
  • Client capability checks for diagnostic tags have been removed from the language server, as the LSP spec requires clients to handle unknown tags

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/checker/checker.go Refactored unused identifier checking to always run and conditionally report as error or suggestion based on compiler options
internal/ls/diagnostics.go Removed client capability checks for diagnostic tags and added documentation comment; removed unused slices import

@sheetalkamat
Copy link
Member

are there any tests. i dont see any change in tests

@jakebailey
Copy link
Member Author

Unfortunately not, the current fourslash runner does not do diagnostics yet and that's a bigger ask I can tackle separately. I have tested this in the editor, however.

@jakebailey
Copy link
Member Author

Okay, this is now a pure port and should work fine.

@jakebailey jakebailey added this pull request to the merge queue Nov 3, 2025
Merged via the queue into main with commit 0c8895b Nov 3, 2025
22 checks passed
@jakebailey jakebailey deleted the jabaile/fix-2001 branch November 3, 2025 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unused declaration behavior differs from tsserver

3 participants