Skip to content

[ty] Fix goto definition for relative imports in third-party files#22457

Merged
MichaReiser merged 1 commit intomainfrom
micha/remove-default-database
Jan 9, 2026
Merged

[ty] Fix goto definition for relative imports in third-party files#22457
MichaReiser merged 1 commit intomainfrom
micha/remove-default-database

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jan 8, 2026

Summary

This PR removes our default database handling from ty's LSP.

We used the default database for files that don't belong to any project,
because we want to treat them differently:

  • We shouldn't show any diagnsotics (funnily enough, this doesn't work today)
  • In the future, we want to disable formatting

This PR removes the default database and:

  • relies on the project inclusion/exlusion to disable diagnostics for non-project files, the same as we do it in the CLI
  • We can disable formatting and other features that should only run on project files the very same way

This fixes an issue where goto definition didn't work in files in the project's virtual environment
for relative imports because the default database uses different search paths than the project.

The only downside that I see using this approach is that ty doesn't warn
about missing imports in non-project files if those modules are available in the project's virtual environment.
If this is a concern, than I think we'll want to use a similar approach to scripts (TBD). Either way, I don't think
we want to use a default database.

Fixes astral-sh/ty#2290

Test Plan

Tested that goto definition now works for projects using venv as their virtual environment (the example in astral-sh/ty#2290)

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Jan 8, 2026
@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Jan 8, 2026
@MichaReiser MichaReiser requested review from dhruvmanila and removed request for carljm, dcreager and sharkdp January 8, 2026 12:59
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 8, 2026 •
edited
Loading

Merging this PR will not alter performance

Summary

✅ 23 untouched benchmarks
⏩ 30 skipped benchmarks1

Comparing micha/remove-default-database (511478e) with main (f9f7a69)

Open in CodSpeed

Footnotes

  1. 30 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. ↩

@MichaReiser
Copy link
Member Author

MichaReiser commented Jan 8, 2026

Uhm what?

@Gankra
Copy link
Contributor

Gankra commented Jan 8, 2026 •
edited
Loading

Uhh wait but we still have the default database for when config handling fails on init right..?

@MichaReiser
Copy link
Member Author

MichaReiser commented Jan 8, 2026

Uhh wait but we still have the default database for when config handling fails on init right..?

That's different and, confusingly, we used the same term for them. We use a fallback db if the initialization of the project's database fails during workspace initialization. But we associate that db with the project's path.

The default database was different. We only used it for paths that have no project with which they share a prefix.

Base automatically changed from micha/fix-is-directory to main January 8, 2026 17:27
Gankra
Gankra approved these changes Jan 8, 2026
View reviewed changes
Copy link
Contributor

@Gankra Gankra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As long as we don't produce diagnostics for things-that-would-otherwise-be-default-database this seems relatively reasonable..?

@Gankra
Copy link
Contributor

Gankra commented Jan 8, 2026

Mostly this just leaves me feeling like "god we have to stop tiptoeing around actually handling multiple workspaces in a session".

@MichaReiser MichaReiser force-pushed the micha/remove-default-database branch from c2ee00f to 511478e Compare January 9, 2026 08:09
@MichaReiser
Copy link
Member Author

MichaReiser commented Jan 9, 2026

Mostly this just leaves me feeling like "god we have to stop tiptoeing around actually handling multiple workspaces in a session".

Yeah, I'm also getting more and more to the conclusion that not having to do that would be great

As long as we don't produce diagnostics for things-that-would-otherwise-be-default-database this seems relatively reasonable..?

I'm not aware of a situationb where this would be the case.

@MichaReiser MichaReiser merged commit c5f6a74 into main Jan 9, 2026
44 checks passed
@MichaReiser MichaReiser deleted the micha/remove-default-database branch January 9, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Gankra Gankra Gankra approved these changes

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila

Assignees

No one assigned

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Go to definition cannot solve relative import in third-party files when using a virtual env location other than .venv

2 participants

@MichaReiser @Gankra