Skip to content

fix(linter): fix LSP panic from stale directive spans#18082

Merged
graphite-app[bot] merged 1 commit intomainfrom
fix/lsp-stale-directives-panic
Jan 16, 2026
Merged

fix(linter): fix LSP panic from stale directive spans#18082
graphite-app[bot] merged 1 commit intomainfrom
fix/lsp-stale-directives-panic

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Jan 16, 2026

Summary

  • Clear cached disable directives before re-linting a file to prevent using outdated spans
  • Add remove method to DirectivesStore for per-path directive removal

Root Cause

The DirectivesStore caches disable directives per file path. When a file is edited and re-linted:

  1. If linting succeeds and produces directives → they are updated in the store
  2. If linting fails or doesn't produce directives (e.g., parse errors, partial loader files) → old directives remain in the store

When the file content changes between linting runs and the new linting fails to update directives, the stored directives contain spans calculated from the OLD source text. Using these stale spans with the NEW source text/rope causes the "byte index out of bounds" panic.

Fix

  1. Added remove method to DirectivesStore for clearing directives for a specific file
  2. Call remove before linting in IsolatedLintHandler::lint_path() to ensure stale directives are cleared

This ensures that when a file is re-linted, any stale directives from previous runs are cleared first. If the new linting run produces fresh directives, they'll be stored; if not, no stale directives will be used.

Test plan

  • Existing LSP tests pass (73 tests)
  • Unused directives tests pass

Closes #15415

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings January 16, 2026 13:38
@Boshen Boshen requested a review from camc314 as a code owner January 16, 2026 13:38
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-bug Category - Bug labels Jan 16, 2026
@Boshen Boshen requested a review from Sysix January 16, 2026 13:39
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 fixes an LSP panic caused by stale disable directive spans when re-linting files. The panic occurs when cached directives contain spans from old source text but are applied to new source text after the file changes.

Changes:

  • Added a remove method to DirectivesStore to clear directives for a specific file path
  • Clear cached directives before re-linting in IsolatedLintHandler::lint_path() to prevent stale span usage

Reviewed changes

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

File Description
crates/oxc_linter/src/lint_runner.rs Adds remove method to clear directives for a specific path
apps/oxlint/src/lsp/isolated_lint_handler.rs Clears stale directives before re-linting to prevent panics

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 16, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing fix/lsp-stale-directives-panic (41743cf) with main (9135b0b)

Summary

✅ 4 untouched benchmarks
⏩ 41 skipped benchmarks1

Footnotes

  1. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Member

@Sysix Sysix left a comment

Choose a reason for hiding this comment

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

One little comment which should be non-blocking

@Sysix Sysix added the 0-merge Merge with Graphite Merge Queue label Jan 16, 2026
Copy link
Member

Sysix commented Jan 16, 2026

Merge activity

## Summary

- Clear cached disable directives before re-linting a file to prevent using outdated spans
- Add `remove` method to `DirectivesStore` for per-path directive removal

## Root Cause

The `DirectivesStore` caches disable directives per file path. When a file is edited and re-linted:

1. If linting succeeds and produces directives → they are updated in the store
2. If linting fails or doesn't produce directives (e.g., parse errors, partial loader files) → **old directives remain in the store**

When the file content changes between linting runs and the new linting fails to update directives, the stored directives contain spans calculated from the OLD source text. Using these stale spans with the NEW source text/rope causes the "byte index out of bounds" panic.

## Fix

1. Added `remove` method to `DirectivesStore` for clearing directives for a specific file
2. Call `remove` before linting in `IsolatedLintHandler::lint_path()` to ensure stale directives are cleared

This ensures that when a file is re-linted, any stale directives from previous runs are cleared first. If the new linting run produces fresh directives, they'll be stored; if not, no stale directives will be used.

## Test plan

- Existing LSP tests pass (73 tests)
- Unused directives tests pass

Closes #15415

🤖 Generated with [Claude Code](https://claude.ai/code)
@graphite-app graphite-app bot force-pushed the fix/lsp-stale-directives-panic branch from 41743cf to 6956543 Compare January 16, 2026 15:04
@graphite-app graphite-app bot merged commit 6956543 into main Jan 16, 2026
22 checks passed
@graphite-app graphite-app bot deleted the fix/lsp-stale-directives-panic branch January 16, 2026 15:11
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linter: Panic: Byte index out of bounds: byte index 44513, Rope/RopeSlice byte length 44427

3 participants