Skip to content

[ty] Add ty_server::Db trait #21241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 5, 2025
Merged

[ty] Add ty_server::Db trait #21241

merged 2 commits into from
Nov 5, 2025

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Nov 3, 2025

Summary

For notebooks, we'll need a way to query the metadata of an arbitrary notebook when mapping
e.g. the range of a find reference operation to the reference's location.

This PR introduces a new Db trait and implements it for ProjectDatabase.
The new Db trait allows quering the document metadata by path, assuming
that the ProjectDatabase uses the LSPSystem (which is always true).

@MichaReiser MichaReiser added internal An internal refactor or improvement server Related to the LSP server ty Multi-file analysis & type inference labels Nov 3, 2025
MichaReiser
MichaReiser commented Nov 3, 2025
View reviewed changes
crates/ty/src/lib.rs
}

fn report_checked_file(&self, db: &dyn Db, file: File, diagnostics: &[Diagnostic]) {
fn report_checked_file(&self, db: &ProjectDatabase, file: File, diagnostics: &[Diagnostic]) {
Copy link
Member Author

@MichaReiser MichaReiser Nov 3, 2025

Choose a reason for hiding this comment

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

I had to change the signature of ProgressReporter to take a ProjectDatabase as argument because we now need a ty_server::Db instead of a ty_project::Db in the LSP.

@MichaReiser MichaReiser force-pushed the micha/server-db-trait branch from 19309e0 to 4adcd15 Compare November 3, 2025 03:05
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025 •
edited
Loading

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #21241 will not alter performance

Comparing micha/server-db-trait (71115b3) with main (7009d60)

Summary

✅ 22 untouched
⏩ 30 skipped1

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 MichaReiser force-pushed the micha/server-db-trait branch from 4adcd15 to aaead0a Compare November 3, 2025 03:40
sharkdp
sharkdp approved these changes Nov 3, 2025
View reviewed changes
crates/ty_server/src/db.rs
Comment on lines +6 to +9
use ty_project::{Db as ProjectDb, ProjectDatabase};

#[salsa::db]
pub(crate) trait Db: ProjectDb {
Copy link
Contributor

@sharkdp sharkdp Nov 3, 2025

Choose a reason for hiding this comment

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

I find this name shadowing here confusing. Is there another name that we could use for the trait here? I assume you named it Db to avoid all kinds of downstream changes?

Copy link
Member Author

@MichaReiser MichaReiser Nov 3, 2025

Choose a reason for hiding this comment

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

The idea is that you can always import Db without having to think within which crate you're in (this is also how all our other Db traits are defined)

Copy link
Contributor

@sharkdp sharkdp Nov 3, 2025

Choose a reason for hiding this comment

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

Makes sense, thank you.

crates/ty_server/src/db.rs

#[salsa::db]
pub(crate) trait Db: ProjectDb {
fn document(&self, file: File) -> Option<&Document>;
Copy link
Contributor

@sharkdp sharkdp Nov 3, 2025

Choose a reason for hiding this comment

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

Maybe

Suggested change
fn document(&self, file: File) -> Option<&Document>;
fn document_from_file(&self, file: File) -> Option<&Document>;

and similar for notebook_document below?

Copy link
Member Author

@MichaReiser MichaReiser Nov 5, 2025

Choose a reason for hiding this comment

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

I find from more confusing because From implies that there's a conversion at play. But that's not the case. The operation here is closer to a map.get(file) where map is a HashMap<File, Document>.

Base automatically changed from micha/lsp-range to main November 5, 2025 13:24
@MichaReiser
Add ty_server::Db trait
32aa35b 
Introduce a new Db trait in ty_server that extends ty_project::Db and adds
server-specific database methods:

- document(): Access to Document from the LSP index
- notebook_document(): Convenient access to NotebookDocument

This trait provides the foundation for notebook support by allowing the server
to access document and notebook metadata through the database interface.

Use ty_server::Db consistently throughout ty_server

Replace all uses of ty_project::Db and ty_python_semantic::Db with
crate::Db (ty_server::Db) for consistency. This ensures all code
within ty_server uses the server-specific Db trait which extends
ty_project::Db.

Add ty_server::Db trait

Introduce a new Db trait in ty_server that extends ty_project::Db and adds
server-specific database methods:

- document(): Access to Document from the LSP index
- notebook_document(): Convenient access to NotebookDocument

This trait provides the foundation for notebook support by allowing the server
to access document and notebook metadata through the database interface.

Use ty_server::Db consistently throughout ty_server

Replace all uses of ty_project::Db and ty_python_semantic::Db with
crate::Db (ty_server::Db) for consistency. This ensures all code
within ty_server uses the server-specific Db trait which extends
ty_project::Db.
@MichaReiser
Add docs
71115b3
@MichaReiser MichaReiser force-pushed the micha/server-db-trait branch from aaead0a to 71115b3 Compare November 5, 2025 13:31
@MichaReiser MichaReiser merged commit 7569b09 into main Nov 5, 2025
40 checks passed
@MichaReiser MichaReiser deleted the micha/server-db-trait branch November 5, 2025 13:40
carljm added a commit to MatthewMckee4/ruff that referenced this pull request Nov 6, 2025
@carljm
Merge branch 'main' into unsupported-operator-diagnostics
1193553 
* main: (188 commits)
  [ty] Discover site-packages from the environment that ty is installed in (astral-sh#21286)
  [ty] Make special cases for `UnionType` slightly narrower (astral-sh#21276)
  Require ignore 0.4.24 in `Cargo.toml` (astral-sh#21292)
  [ty] Favour imported symbols over builtin symbols (astral-sh#21285)
  docs: revise Ruff setup instructions for Zed editor (astral-sh#20935)
  [ty] Update salsa (astral-sh#21281)
  [syntax-error]: no binding for nonlocal  PLE0117 as a semantic syntax error (astral-sh#21032)
  [ty] Constraining a typevar with itself (possibly via union or intersection) (astral-sh#21273)
  [`ruff`] Fix false positives on starred arguments (`RUF057`) (astral-sh#21256)
  [ty] Simplify unions containing multiple type variables during inference (astral-sh#21275)
  [ty] Add `ty_server::Db` trait (astral-sh#21241)
  [ty] Refactor `Range` to/from `TextRange` conversion as prep for notebook support (astral-sh#21230)
  [ty] Fix playground crash when file name includes path separator (astral-sh#21151)
  [`refurb`] Fix false negative for underscores before sign in `Decimal` constructor (`FURB157`) (astral-sh#21190)
  [ty] Allow values of type `None` in type expressions (astral-sh#21263)
  Run codspeed benchmarks with `profiling` profile (astral-sh#21261)
  [ty] Update expected diagnostic count in benchmarks (astral-sh#21269)
  Avoid extra parentheses for long `match` patterns with `as` captures (astral-sh#21176)
  [ty] Update salsa (astral-sh#21265)
  [ty] `dict` is not assignable to `TypedDict` (astral-sh#21238)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sharkdp sharkdp sharkdp approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

Assignees

No one assigned

Labels

internal An internal refactor or improvement 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.

3 participants

@MichaReiser @sharkdp