Skip to content

fix: LSP diagnostics not refreshing on file changes and config updates#499

Merged
fansenze merged 1 commit intomainfrom
fix/lsp-push-diagnostics-20260310
Mar 10, 2026
Merged

fix: LSP diagnostics not refreshing on file changes and config updates#499
fansenze merged 1 commit intomainfrom
fix/lsp-push-diagnostics-20260310

Conversation

@fansenze
Copy link
Copy Markdown
Contributor

Summary

  • Switch LSP diagnostics from pull to push model (publishDiagnostics on didOpen/didChange/didSave)
  • Fix session lifecycle: call DidOpenFile/DidChangeFile/DidSaveFile/DidCloseFile at correct handler points
  • Clear diagnostics on didClose by pushing empty diagnostics before closing
  • Reload rslint config on file changes via workspace/didChangeWatchedFiles
  • Forward file watch events to Session so it detects tsconfig changes automatically
  • RefreshDiagnostics signals main dispatch loop via buffered channel for goroutine-safe relint
  • Enable watchEnabled based on client capabilities so Session can register tsconfig watchers
  • Fix isRslintConfigURI to require path separator (avoid matching not-rslint.json)

Related Links

Fixes issues where:

  1. Diagnostics were not pushed on file open/edit, only returned on pull requests
  2. Editing rslint.json did not trigger config reload and re-lint
  3. tsconfig.json changes were not detected by the LSP server
  4. Closing a file did not clear its diagnostics in the editor

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the responsiveness and accuracy of the LSP server by transitioning to a push-based diagnostic model. This change ensures that linting feedback is provided in real-time as files are opened, edited, or saved, and that diagnostics are correctly cleared when files are closed. Furthermore, it improves the server's ability to react to configuration changes, such as modifications to rslint.json or tsconfig.json, by implementing robust file watching and configuration reloading mechanisms. These updates collectively lead to a more fluid and reliable developer experience within the editor.

Highlights

  • LSP Diagnostics Model: Switched the LSP diagnostics from a pull-based model to a push-based model, using publishDiagnostics on didOpen, didChange, and didSave events.
  • Session Lifecycle Management: Fixed the session lifecycle by ensuring DidOpenFile, DidChangeFile, DidSaveFile, and DidCloseFile are called at the correct handler points.
  • Diagnostic Clearing on Close: Implemented clearing of diagnostics on didClose by pushing empty diagnostics before closing a file.
  • RSLint Config Reload: Enabled reloading of the rslint configuration on file changes via workspace/didChangeWatchedFiles.
  • TSConfig Change Detection: Forwarded file watch events to the LSP Session to automatically detect tsconfig changes.
  • Goroutine-Safe Relinting: Introduced a buffered channel for RefreshDiagnostics to signal the main dispatch loop, ensuring goroutine-safe re-linting.
  • Client Capabilities for File Watching: Enabled watchEnabled based on client capabilities, allowing the Session to register tsconfig watchers.
  • RSLint Config URI Matching: Fixed isRslintConfigURI to correctly identify rslint.json and rslint.jsonc files by requiring a path separator, preventing false positives.
Changelog
  • internal/lsp/config_watch_test.go
    • Added new tests for isRslintConfigURI to ensure correct identification of rslint configuration files.
    • Added comprehensive tests for handleDidChangeWatchedFiles covering various scenarios including nil parameters, empty changes, non-config files, config file detection, deletion, mixed changes, and nil session handling.
    • Added tests for RefreshDiagnostics to verify channel signaling and non-blocking behavior under multiple calls.
    • Added tests for reloadConfigAndRelint to confirm proper config clearing and document re-linting.
    • Added tests for ptrIsTrue utility function.
    • Added tests for handleInitialized to validate watchEnabled status based on client capabilities.
    • Added tests for dispatchLoop to ensure refresh signals trigger document re-linting.
  • internal/lsp/server.go
    • Initialized refreshCh as a buffered channel in the NewServer constructor.
    • Added rslintConfigPath and refreshCh fields to the Server struct for managing rslint configuration and diagnostic refresh signals.
    • Modified RefreshDiagnostics to signal the refreshCh for asynchronous diagnostic updates, removing direct LSP refresh requests.
    • Updated the dispatchLoop to listen for signals on refreshCh and trigger re-linting of all open documents upon receiving a signal.
    • Registered handleDidClose and handleDidChangeWatchedFiles as notification handlers in the LSP server's handler map.
  • internal/lsp/service.go
    • Removed the DiagnosticProvider capability from the handleInitialize response, aligning with the new push-based diagnostic model.
    • Modified handleInitialized to dynamically enable watchEnabled based on the client's DidChangeWatchedFiles dynamic registration capability and passed this setting to the project.NewSession.
    • Extracted rslint configuration loading logic into a new reloadConfig method and updated handleInitialized to use it.
    • Added handleDidChangeWatchedFiles to process file change notifications, forwarding them to the LSP session and triggering rslint config reloads for relevant files.
    • Introduced isRslintConfigURI helper function to accurately identify rslint configuration files.
    • Added reloadConfigAndRelint function to re-discover and reload the rslint configuration, then re-lint all open documents.
    • Updated handleDidOpen, handleDidChange, and handleDidSave to notify the LSP session about file changes and immediately push diagnostics.
    • Added handleDidClose to remove documents and their diagnostics from the server state, and to publish empty diagnostics to clear them in the client.
    • Simplified handleCodeAction by removing the logic that attempted to generate diagnostics if none were present, relying instead on the push model.
    • Removed the handleDocumentDiagnostic request handler, as diagnostics are now pushed proactively.
    • Adjusted runLintWithSession to remove the DidOpenFile call, as session overlay management is now handled by the didOpen/didChange handlers.
    • Added pushDiagnostics function to encapsulate the logic for running the linter for a given URI and publishing the results to the client.
  • internal/lsp/service_test.go
    • Added newTestServer helper function for creating a minimal server instance for unit tests.
    • Added makeDidChangeParams helper for constructing DidChangeTextDocumentParams for full-sync mode.
    • Added tests for handleDidOpen including re-opening documents.
    • Added tests for handleDidChange covering basic changes, empty changes, rapid successive changes, and changes to unopened documents.
    • Added tests for handleDidSave including cases with and without provided text.
    • Added tests for handleDidClose to verify document and diagnostic removal, handling of nonexistent documents, and non-interference with other documents.
    • Added lifecycle integration tests for OpenChangeClose and MultipleDocuments_IndependentLifecycles scenarios.
    • Added TestRapidChanges_VersionTracking to ensure correct content after rapid edits.
    • Added newTestServerWithQueue helper for testing PublishDiagnostics calls.
    • Added TestHandleDidClose_NoPublishWhenSessionNil to confirm no diagnostics are published when the session is nil.
    • Added TestPublishDiagnostics_EmptySlice to verify publishing of empty diagnostic arrays.
    • Added TestCloseAndReopen to validate server state consistency after a close and reopen cycle.
  • packages/rslint/fixtures/src/close-test.ts
    • Added a new fixture file to be used in tests for verifying diagnostic clearing upon file closure.
  • packages/rslint/fixtures/src/error-transitions.ts
    • Added a new fixture file for testing diagnostic transitions between different error states.
  • packages/rslint/fixtures/src/styles.css
    • Added a new fixture file to confirm that non-TypeScript files do not trigger diagnostics.
  • packages/vscode-extension/tests/suite/extension.test.ts
    • Added waitForDiagnosticsToChange helper function to await diagnostic updates with a changed count.
    • Added waitForDiagnosticsWithMessage helper function to await diagnostic updates containing a specific message.
    • Added new tests for diagnostics refreshing after edits, including scenarios for removing errors, introducing errors, and rapid successive edits.
    • Added a test to verify diagnostics clear completely when all errors are removed.
    • Added a test to ensure diagnostics update correctly across multiple edit cycles.
    • Added a comprehensive test for diagnostic transitions through clean, error A, error B, and back to clean states.
    • Added a test to confirm diagnostics are correct after a close and reopen cycle.
    • Added a test to ensure no diagnostics are reported for non-TypeScript files like CSS.
  • scripts/dictionary.txt
    • Added 'relint', 'relints', 'relinting', and 'lifecycles' to the dictionary.
Activity
  • The author has updated tests to cover the new diagnostic refresh behavior and configuration watching.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@fansenze fansenze force-pushed the fix/lsp-push-diagnostics-20260310 branch from bf9b1ac to 423d8ea Compare March 10, 2026 07:30
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly refactors the LSP server's diagnostic handling to improve responsiveness and correctness, particularly concerning file changes and configuration updates. Key changes include introducing a refreshCh buffered channel to coalesce and signal diagnostic refreshes, ensuring relinting occurs on the main goroutine. The server now implements handleDidChangeWatchedFiles to process file system events, triggering reloadConfigAndRelint for rslint.json changes and forwarding other file changes to the underlying project.Session. Document lifecycle events (DidOpen, DidChange, DidSave, DidClose) are now integrated with the session and trigger immediate diagnostic pushes via a new pushDiagnostics method, replacing the previous DocumentDiagnostic request handler. New test fixtures and extensive tests for diagnostic lifecycle, rapid edits, and config watching have been added. Review comments highlight a security concern regarding sensitive information leakage due to logging the full params object in handleDidOpen and handleDidSave, suggesting logging only the document URI. Additionally, an efficiency improvement is recommended by moving the config.RegisterAllRules() call to server initialization to avoid redundant executions.

@fansenze fansenze force-pushed the fix/lsp-push-diagnostics-20260310 branch 2 times, most recently from 4ad37bb to 37071b1 Compare March 10, 2026 08:03
- Add debounce (200ms) for didChange to avoid linting on every keystroke
- Move RegisterAllRules to handleInitialized for efficiency
- Sanitize log output to print URI instead of full params
- Clean up pendingLintURIs on document close to prevent stale lints
- Add waitForDiagnosticsCount helper in E2E tests for reliable CI
@fansenze fansenze force-pushed the fix/lsp-push-diagnostics-20260310 branch from 37071b1 to cd6bd92 Compare March 10, 2026 08:09
@fansenze fansenze merged commit 520107f into main Mar 10, 2026
14 checks passed
@fansenze fansenze deleted the fix/lsp-push-diagnostics-20260310 branch March 10, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants