Skip to content

Fix auto-import to prefer existing import type declarations#3244

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-auto-import-type-resolution
Open

Fix auto-import to prefer existing import type declarations#3244
Copilot wants to merge 3 commits intomainfrom
copilot/fix-auto-import-type-resolution

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

tryAddToExistingImport returned the first valid matching import declaration, which meant a value import appearing before an import type would always win. Types would get added as inline type modifiers on the value import instead of merging into the existing import type block.

// Before: type added to value import
import { Type, type Static } from "typebox";
import type { } from "typebox";  // ignored

// After: type merged into existing import type
import { Type } from "typebox";
import type { Static } from "typebox";

Changes

  • internal/ls/autoimport/fix.go: Port the preference logic from the TS reference (importFixes.ts L1101–1117). Instead of returning the first valid match, track a best candidate and return immediately on a "perfect match" — type-only imports for type additions, value imports for value additions. Also added the missing guard for addAsTypeOnly == Required && namedBindings != nil on default imports.
  • internal/fourslash/_scripts/failingTests.txt: Removed TestAutoImportTypeOnlyPreferred2 and TestImportNameCodeFix_importType2 — both now pass.

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

When adding a type import, the tryAddToExistingImport function now
gives preference to existing `import type` declarations instead of
returning the first valid match. This matches the TypeScript reference
implementation behavior.

Also adds missing check for default imports when addAsTypeOnly is
Required and namedBindings exist.

Fixes #3029

Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/a6cc4bfe-4eb1-48f4-8a31-62a398c4fbca

Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix auto-import to merge existing import type declarations Fix auto-import to prefer existing import type declarations Mar 26, 2026
Copilot AI requested a review from andrewbranch March 26, 2026 18:45
@andrewbranch
Copy link
Copy Markdown
Member

@copilot since this causes existing tests to be removed from the failing list, you can delete your new test.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 26, 2026

@copilot since this causes existing tests to be removed from the failing list, you can delete your new test.

Done in c1f878a — deleted the new test file since TestAutoImportTypeOnlyPreferred2 and TestImportNameCodeFix_importType2 already cover it.

@andrewbranch andrewbranch marked this pull request as ready for review March 26, 2026 19:08
Copilot AI review requested due to automatic review settings March 26, 2026 19:08
Copy link
Copy Markdown
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 updates the Go port’s auto-import “add to existing import” selection logic to prefer merging type-only additions into existing import type declarations (matching TypeScript’s reference behavior), rather than always picking the first matching import.

Changes:

  • Update tryAddToExistingImport to track a “best” candidate and prefer a “perfect match” (type-only import for type additions; value import for value additions) instead of returning the first match.
  • Add a missing guard to prevent attempting to add a type-only default import to an existing import that already has named bindings (which the current edit-application logic can’t safely express as type-only).
  • Remove two entries from the fourslash “known failing tests” list because they now pass.

Reviewed changes

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

File Description
internal/ls/autoimport/fix.go Adjusts existing-import selection to prefer import type for type additions and adds a safety guard for type-only default imports.
internal/fourslash/_scripts/failingTests.txt Removes two tests from the known-failures list after the behavior fix.

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.

4 participants