Skip to content

fix(lsp): organizeImports without resolving specifiers - #31230

Merged
nayeemrmn merged 1 commit into
denoland:mainfrom
tugrulates:organize-imports
Nov 17, 2025
Merged

nayeemrmn merged 1 commit into
denoland:mainfrom
tugrulates:organize-imports

Conversation

@tugrulates

Copy link
Copy Markdown
Contributor

Organize imports action should move, group, or delete imports but not rewrite the imported modules.

The current behavior is causing a bug with workspaces, rewriting relative imports to scoped imports if the workspace members are loaded in the server. The two specifiers are not functionally equivalent, at least due to jsr-io/jsr#1220.

This change removes the rewriting and keeps the imports as they are sent from tsc.

Fixes #31141

Organize imports action should move, group, or delete imports but not
rewrite the imported modules.

The current behavior is causing a bug with workspaces, rewriting
relative imports to scoped imports if the workspace members are loaded
in the server. The two specifiers are not functionally equivalent, at
least due to jsr-io/jsr#1220.

This change removes the rewriting and keeps the imports as they are sent
from tsc.

Fixes #31141

[ci]
@CLAassistant

CLAassistant commented Nov 8, 2025 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@tugrulates
tugrulates marked this pull request as ready for review November 8, 2025 17:18
Copilot AI review requested due to automatic review settings November 8, 2025 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 a bug where the "Organize Imports" LSP action was incorrectly rewriting relative imports to scoped imports in workspace scenarios. The fix ensures that import specifiers are preserved as-is from TypeScript, with only their organization (ordering, grouping, deletion) being modified.

  • Removes the call to fix_ts_import_changes which was resolving and rewriting import specifiers
  • Simplifies the code path to directly collect organize imports edits without specifier resolution
  • Adds comprehensive test coverage for workspace import organization scenarios

Reviewed Changes

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

File Description
cli/lsp/language_server.rs Removes the fix_ts_import_changes call that was rewriting import specifiers, replacing it with direct collection of organize imports edits
tests/integration/lsp_tests.rs Adds test case verifying that organize imports preserves relative and scoped imports without rewriting them in workspace scenarios

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dsherret
dsherret requested a review from nayeemrmn November 13, 2025 17:12

@nayeemrmn nayeemrmn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nayeemrmn
nayeemrmn merged commit e7ef895 into denoland:main Nov 17, 2025
24 checks passed
@tugrulates
tugrulates deleted the organize-imports branch November 17, 2025 12:45
bartlomieju pushed a commit to bartlomieju/deno that referenced this pull request Nov 17, 2025
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.

Bug: lsp organizeImports makes code unpublishable on JSR

4 participants