From 99d0ac60b417ee623bb9257a6502a7c21c444b28 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 18 Jul 2025 19:33:35 +0530 Subject: [PATCH] [ty] Track open files in the server (#19264) ## Summary This PR updates the server to keep track of open files both system and virtual files. This is done by updating the project by adding the file in the open file set in `didOpen` notification and removing it in `didClose` notification. This does mean that for workspace diagnostics, ty will only check open files because the behavior of different diagnostic builder is to first check `is_file_open` and only add diagnostics for open files. So, this required updating the `is_file_open` model to be `should_check_file` model which validates whether the file needs to be checked based on the `CheckMode`. If the check mode is open files only then it will check whether the file is open. If it's all files then it'll return `true` by default. Closes: astral-sh/ty#619 ## Test Plan ### Before There are two files in the project: `__init__.py` and `diagnostics.py`. In the video, I'm demonstrating the old behavior where making changes to the (open) `diagnostics.py` file results in re-parsing the file: https://github.com/user-attachments/assets/c2ac0ecd-9c77-42af-a924-c3744b146045 ### After Same setup as above. In the video, I'm demonstrating the new behavior where making changes to the (open) `diagnostics.py` file doesn't result in re-parting the file: https://github.com/user-attachments/assets/7b82fe92-f330-44c7-b527-c841c4545f8f --- crates/ruff_benchmark/benches/ty.rs | 4 +- crates/ruff_db/src/files.rs | 2 +- crates/ruff_graph/src/db.rs | 2 +- crates/ty_ide/src/db.rs | 2 +- crates/ty_project/src/db.rs | 65 ++++++---- crates/ty_project/src/files.rs | 4 +- crates/ty_project/src/lib.rs | 113 ++++++++---------- crates/ty_project/src/metadata.rs | 40 +++---- crates/ty_python_semantic/src/db.rs | 5 +- .../src/semantic_index/builder.rs | 2 +- .../ty_python_semantic/src/types/context.rs | 4 +- crates/ty_python_semantic/tests/corpus.rs | 2 +- crates/ty_server/src/logging.rs | 2 +- .../src/server/api/notifications/did_close.rs | 31 ++++- .../src/server/api/notifications/did_open.rs | 35 +++++- .../api/requests/workspace_diagnostic.rs | 5 +- crates/ty_server/src/session.rs | 20 +++- crates/ty_server/src/session/index.rs | 2 +- crates/ty_server/src/session/options.rs | 8 ++ crates/ty_test/src/db.rs | 2 +- crates/ty_wasm/src/lib.rs | 8 +- fuzz/fuzz_targets/ty_check_invalid_syntax.rs | 2 +- 22 files changed, 220 insertions(+), 140 deletions(-) diff --git a/crates/ruff_benchmark/benches/ty.rs b/crates/ruff_benchmark/benches/ty.rs index 04c79bff8e..4858bdd453 100644 --- a/crates/ruff_benchmark/benches/ty.rs +++ b/crates/ruff_benchmark/benches/ty.rs @@ -18,7 +18,7 @@ use ruff_python_ast::PythonVersion; use ty_project::metadata::options::{EnvironmentOptions, Options}; use ty_project::metadata::value::{RangedValue, RelativePathBuf}; use ty_project::watch::{ChangeEvent, ChangedKind}; -use ty_project::{Db, ProjectDatabase, ProjectMetadata}; +use ty_project::{CheckMode, Db, ProjectDatabase, ProjectMetadata}; struct Case { db: ProjectDatabase, @@ -102,6 +102,7 @@ fn setup_tomllib_case() -> Case { let re = re.unwrap(); + db.set_check_mode(CheckMode::OpenFiles); db.project().set_open_files(&mut db, tomllib_files); let re_path = re.path(&db).as_system_path().unwrap().to_owned(); @@ -237,6 +238,7 @@ fn setup_micro_case(code: &str) -> Case { let mut db = ProjectDatabase::new(metadata, system).unwrap(); let file = system_path_to_file(&db, SystemPathBuf::from(file_path)).unwrap(); + db.set_check_mode(CheckMode::OpenFiles); db.project() .set_open_files(&mut db, FxHashSet::from_iter([file])); diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index a5cb501f9f..3401adf178 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -486,7 +486,7 @@ impl fmt::Debug for File { /// /// This is a wrapper around a [`File`] that provides additional methods to interact with a virtual /// file. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub struct VirtualFile(File); impl VirtualFile { diff --git a/crates/ruff_graph/src/db.rs b/crates/ruff_graph/src/db.rs index 48ee62a348..cbca766de6 100644 --- a/crates/ruff_graph/src/db.rs +++ b/crates/ruff_graph/src/db.rs @@ -87,7 +87,7 @@ impl SourceDb for ModuleDb { #[salsa::db] impl Db for ModuleDb { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_ide/src/db.rs b/crates/ty_ide/src/db.rs index 3d9a9c564e..6baa74dcc6 100644 --- a/crates/ty_ide/src/db.rs +++ b/crates/ty_ide/src/db.rs @@ -96,7 +96,7 @@ pub(crate) mod tests { #[salsa::db] impl SemanticDb for TestDb { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_project/src/db.rs b/crates/ty_project/src/db.rs index b0e85a42b4..5270495e86 100644 --- a/crates/ty_project/src/db.rs +++ b/crates/ty_project/src/db.rs @@ -12,8 +12,8 @@ use ruff_db::diagnostic::Diagnostic; use ruff_db::files::{File, Files}; use ruff_db::system::System; use ruff_db::vendored::VendoredFileSystem; -use salsa::Event; use salsa::plumbing::ZalsaDatabase; +use salsa::{Event, Setter}; use ty_ide::Db as IdeDb; use ty_python_semantic::lint::{LintRegistry, RuleSelection}; use ty_python_semantic::{Db as SemanticDb, Program}; @@ -82,22 +82,25 @@ impl ProjectDatabase { Ok(db) } - /// Checks all open files in the project and its dependencies. + /// Checks the files in the project and its dependencies as per the project's check mode. + /// + /// Use [`set_check_mode`] to update the check mode. + /// + /// [`set_check_mode`]: ProjectDatabase::set_check_mode pub fn check(&self) -> Vec { - self.check_with_mode(CheckMode::OpenFiles) - } - - /// Checks all open files in the project and its dependencies, using the given reporter. - pub fn check_with_reporter(&self, reporter: &mut dyn ProgressReporter) -> Vec { - let reporter = AssertUnwindSafe(reporter); - self.project().check(self, CheckMode::OpenFiles, reporter) - } - - /// Check the project with the given mode. - pub fn check_with_mode(&self, mode: CheckMode) -> Vec { let mut reporter = DummyReporter; let reporter = AssertUnwindSafe(&mut reporter as &mut dyn ProgressReporter); - self.project().check(self, mode, reporter) + self.project().check(self, reporter) + } + + /// Checks the files in the project and its dependencies, using the given reporter. + /// + /// Use [`set_check_mode`] to update the check mode. + /// + /// [`set_check_mode`]: ProjectDatabase::set_check_mode + pub fn check_with_reporter(&self, reporter: &mut dyn ProgressReporter) -> Vec { + let reporter = AssertUnwindSafe(reporter); + self.project().check(self, reporter) } #[tracing::instrument(level = "debug", skip(self))] @@ -105,6 +108,12 @@ impl ProjectDatabase { self.project().check_file(self, file) } + /// Set the check mode for the project. + pub fn set_check_mode(&mut self, mode: CheckMode) { + tracing::debug!("Updating project to check {mode}"); + self.project().set_check_mode(self).to(mode); + } + /// Returns a mutable reference to the system. /// /// WARNING: Triggers a new revision, canceling other database handles. This can lead to deadlock. @@ -163,17 +172,28 @@ impl std::fmt::Debug for ProjectDatabase { } } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(test, derive(serde::Serialize))] pub enum CheckMode { - /// Checks only the open files in the project. + /// Checks the open files in the project. OpenFiles, /// Checks all files in the project, ignoring the open file set. /// - /// This includes virtual files, such as those created by the language server. + /// This includes virtual files, such as those opened in an editor. + #[default] AllFiles, } +impl fmt::Display for CheckMode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CheckMode::OpenFiles => write!(f, "open files"), + CheckMode::AllFiles => write!(f, "all files"), + } + } +} + /// Stores memory usage information. pub struct SalsaMemoryDump { total_fields: usize, @@ -389,12 +409,9 @@ impl IdeDb for ProjectDatabase {} #[salsa::db] impl SemanticDb for ProjectDatabase { - fn is_file_open(&self, file: File) -> bool { - let Some(project) = &self.project else { - return false; - }; - - project.is_file_open(self, file) + fn should_check_file(&self, file: File) -> bool { + self.project + .is_some_and(|project| project.should_check_file(self, file)) } fn rule_selection(&self, file: File) -> &RuleSelection { @@ -543,7 +560,7 @@ pub(crate) mod tests { #[salsa::db] impl ty_python_semantic::Db for TestDb { - fn is_file_open(&self, file: ruff_db::files::File) -> bool { + fn should_check_file(&self, file: ruff_db::files::File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_project/src/files.rs b/crates/ty_project/src/files.rs index 3021aa6143..fbf888b16d 100644 --- a/crates/ty_project/src/files.rs +++ b/crates/ty_project/src/files.rs @@ -18,7 +18,7 @@ use crate::{IOErrorDiagnostic, Project}; /// The implementation uses internal mutability to transition between the lazy and indexed state /// without triggering a new salsa revision. This is safe because the initial indexing happens on first access, /// so no query can be depending on the contents of the indexed files before that. All subsequent mutations to -/// the indexed files must go through `IndexedMut`, which uses the Salsa setter `package.set_file_set` to +/// the indexed files must go through `IndexedMut`, which uses the Salsa setter `project.set_file_set` to /// ensure that Salsa always knows when the set of indexed files have changed. #[derive(Debug)] pub struct IndexedFiles { @@ -280,7 +280,7 @@ mod tests { // Calling files a second time should not dead-lock. // This can e.g. happen when `check_file` iterates over all files and - // `is_file_open` queries the open files. + // `should_check_file` queries the open files. let files_2 = project.file_set(&db).get(); match files_2 { diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index 57cba59721..12f871c00a 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -14,6 +14,8 @@ use rustc_hash::FxHashSet; use salsa::Durability; use salsa::Setter; use std::backtrace::BacktraceStatus; +use std::collections::hash_set; +use std::iter::FusedIterator; use std::panic::{AssertUnwindSafe, UnwindSafe}; use std::sync::Arc; use thiserror::Error; @@ -54,13 +56,10 @@ pub fn default_lints_registry() -> LintRegistry { #[salsa::input] #[derive(Debug)] pub struct Project { - /// The files that are open in the project. - /// - /// Setting the open files to a non-`None` value changes `check` to only check the - /// open files rather than all files in the project. - #[returns(as_deref)] + /// The files that are open in the project, [`None`] if there are no open files. + #[returns(ref)] #[default] - open_fileset: Option>>, + open_fileset: FxHashSet, /// The first-party files of this project. #[default] @@ -110,6 +109,13 @@ pub struct Project { /// Diagnostics that were generated when resolving the project settings. #[returns(deref)] settings_diagnostics: Vec, + + /// The mode in which the project should be checked. + /// + /// This changes the behavior of `check` to either check only the open files or all files in + /// the project including the virtual files that might exists in the editor. + #[default] + check_mode: CheckMode, } /// A progress reporter. @@ -207,17 +213,20 @@ impl Project { self.reload_files(db); } - /// Checks all open files in the project and its dependencies. + /// Checks the project and its dependencies according to the project's check mode. pub(crate) fn check( self, db: &ProjectDatabase, - mode: CheckMode, mut reporter: AssertUnwindSafe<&mut dyn ProgressReporter>, ) -> Vec { let project_span = tracing::debug_span!("Project::check"); let _span = project_span.enter(); - tracing::debug!("Checking project '{name}'", name = self.name(db)); + tracing::debug!( + "Checking {} in project '{name}'", + self.check_mode(db), + name = self.name(db) + ); let mut diagnostics: Vec = Vec::new(); diagnostics.extend( @@ -226,11 +235,7 @@ impl Project { .map(OptionDiagnostic::to_diagnostic), ); - let files = match mode { - CheckMode::OpenFiles => ProjectFiles::new(db, self), - // TODO: Consider open virtual files as well - CheckMode::AllFiles => ProjectFiles::Indexed(self.files(db)), - }; + let files = ProjectFiles::new(db, self); reporter.set_files(files.len()); diagnostics.extend( @@ -284,7 +289,7 @@ impl Project { } pub(crate) fn check_file(self, db: &dyn Db, file: File) -> Vec { - if !self.is_file_open(db, file) { + if !self.should_check_file(db, file) { return Vec::new(); } @@ -292,8 +297,6 @@ impl Project { } /// Opens a file in the project. - /// - /// This changes the behavior of `check` to only check the open files rather than all files in the project. pub fn open_file(self, db: &mut dyn Db, file: File) { tracing::debug!("Opening file `{}`", file.path(db)); @@ -340,45 +343,40 @@ impl Project { } } - /// Returns the open files in the project or `None` if the entire project should be checked. - pub fn open_files(self, db: &dyn Db) -> Option<&FxHashSet> { + /// Returns the open files in the project or `None` if there are no open files. + pub fn open_files(self, db: &dyn Db) -> &FxHashSet { self.open_fileset(db) } /// Sets the open files in the project. - /// - /// This changes the behavior of `check` to only check the open files rather than all files in the project. #[tracing::instrument(level = "debug", skip(self, db))] pub fn set_open_files(self, db: &mut dyn Db, open_files: FxHashSet) { tracing::debug!("Set open project files (count: {})", open_files.len()); - self.set_open_fileset(db).to(Some(Arc::new(open_files))); + self.set_open_fileset(db).to(open_files); } /// This takes the open files from the project and returns them. - /// - /// This changes the behavior of `check` to check all files in the project instead of just the open files. fn take_open_files(self, db: &mut dyn Db) -> FxHashSet { tracing::debug!("Take open project files"); // Salsa will cancel any pending queries and remove its own reference to `open_files` // so that the reference counter to `open_files` now drops to 1. - let open_files = self.set_open_fileset(db).to(None); - - if let Some(open_files) = open_files { - Arc::try_unwrap(open_files).unwrap() - } else { - FxHashSet::default() - } + self.set_open_fileset(db).to(FxHashSet::default()) } - /// Returns `true` if the file is open in the project. + /// Returns `true` if the file should be checked. /// - /// A file is considered open when: - /// * explicitly set as an open file using [`open_file`](Self::open_file) - /// * It has a [`SystemPath`] and belongs to a package's `src` files - /// * It has a [`SystemVirtualPath`](ruff_db::system::SystemVirtualPath) - pub fn is_file_open(self, db: &dyn Db, file: File) -> bool { + /// This depends on the project's check mode: + /// * For [`OpenFiles`], it checks if the file is either explicitly set as an open file using + /// [`open_file`] or a system virtual path + /// * For [`AllFiles`], it checks if the file is either a system virtual path or a part of the + /// indexed files in the project + /// + /// [`open_file`]: Self::open_file + /// [`OpenFiles`]: CheckMode::OpenFiles + /// [`AllFiles`]: CheckMode::AllFiles + pub fn should_check_file(self, db: &dyn Db, file: File) -> bool { let path = file.path(db); // Try to return early to avoid adding a dependency on `open_files` or `file_set` which @@ -387,12 +385,12 @@ impl Project { return false; } - if let Some(open_files) = self.open_files(db) { - open_files.contains(&file) - } else if file.path(db).is_system_path() { - self.files(db).contains(&file) - } else { - file.path(db).is_system_virtual_path() + match self.check_mode(db) { + CheckMode::OpenFiles => self.open_files(db).contains(&file), + CheckMode::AllFiles => { + // Virtual files are always checked. + path.is_system_virtual_path() || self.files(db).contains(&file) + } } } @@ -524,11 +522,7 @@ pub(crate) fn check_file_impl(db: &dyn Db, file: File) -> Box<[Diagnostic]> { } } - if db - .project() - .open_fileset(db) - .is_none_or(|files| !files.contains(&file)) - { + if !db.project().open_fileset(db).contains(&file) { // Drop the AST now that we are done checking this file. It is not currently open, // so it is unlikely to be accessed again soon. If any queries need to access the AST // from across files, it will be re-parsed. @@ -554,24 +548,23 @@ enum ProjectFiles<'a> { impl<'a> ProjectFiles<'a> { fn new(db: &'a dyn Db, project: Project) -> Self { - if let Some(open_files) = project.open_files(db) { - ProjectFiles::OpenFiles(open_files) - } else { - ProjectFiles::Indexed(project.files(db)) + match project.check_mode(db) { + CheckMode::OpenFiles => ProjectFiles::OpenFiles(project.open_files(db)), + CheckMode::AllFiles => ProjectFiles::Indexed(project.files(db)), } } fn diagnostics(&self) -> &[IOErrorDiagnostic] { match self { ProjectFiles::OpenFiles(_) => &[], - ProjectFiles::Indexed(indexed) => indexed.diagnostics(), + ProjectFiles::Indexed(files) => files.diagnostics(), } } fn len(&self) -> usize { match self { ProjectFiles::OpenFiles(open_files) => open_files.len(), - ProjectFiles::Indexed(indexed) => indexed.len(), + ProjectFiles::Indexed(files) => files.len(), } } } @@ -583,16 +576,14 @@ impl<'a> IntoIterator for &'a ProjectFiles<'a> { fn into_iter(self) -> Self::IntoIter { match self { ProjectFiles::OpenFiles(files) => ProjectFilesIter::OpenFiles(files.iter()), - ProjectFiles::Indexed(indexed) => ProjectFilesIter::Indexed { - files: indexed.into_iter(), - }, + ProjectFiles::Indexed(files) => ProjectFilesIter::Indexed(files.into_iter()), } } } enum ProjectFilesIter<'db> { - OpenFiles(std::collections::hash_set::Iter<'db, File>), - Indexed { files: files::IndexedIter<'db> }, + OpenFiles(hash_set::Iter<'db, File>), + Indexed(files::IndexedIter<'db>), } impl Iterator for ProjectFilesIter<'_> { @@ -601,11 +592,13 @@ impl Iterator for ProjectFilesIter<'_> { fn next(&mut self) -> Option { match self { ProjectFilesIter::OpenFiles(files) => files.next().copied(), - ProjectFilesIter::Indexed { files } => files.next(), + ProjectFilesIter::Indexed(files) => files.next(), } } } +impl FusedIterator for ProjectFilesIter<'_> {} + #[derive(Debug, Clone)] pub struct IOErrorDiagnostic { file: Option, diff --git a/crates/ty_project/src/metadata.rs b/crates/ty_project/src/metadata.rs index 443b54f0e2..8405f8a61b 100644 --- a/crates/ty_project/src/metadata.rs +++ b/crates/ty_project/src/metadata.rs @@ -372,11 +372,11 @@ mod tests { with_escaped_paths(|| { assert_ron_snapshot!(&project, @r#" - ProjectMetadata( - name: Name("app"), - root: "/app", - options: Options(), - ) + ProjectMetadata( + name: Name("app"), + root: "/app", + options: Options(), + ) "#); }); @@ -410,11 +410,11 @@ mod tests { with_escaped_paths(|| { assert_ron_snapshot!(&project, @r#" - ProjectMetadata( - name: Name("backend"), - root: "/app", - options: Options(), - ) + ProjectMetadata( + name: Name("backend"), + root: "/app", + options: Options(), + ) "#); }); @@ -552,16 +552,16 @@ unclosed table, expected `]` with_escaped_paths(|| { assert_ron_snapshot!(root, @r#" - ProjectMetadata( - name: Name("project-root"), - root: "/app", - options: Options( - src: Some(SrcOptions( - root: Some("src"), - )), - ), - ) - "#); + ProjectMetadata( + name: Name("project-root"), + root: "/app", + options: Options( + src: Some(SrcOptions( + root: Some("src"), + )), + ), + ) + "#); }); Ok(()) diff --git a/crates/ty_python_semantic/src/db.rs b/crates/ty_python_semantic/src/db.rs index 815c37653c..645929235a 100644 --- a/crates/ty_python_semantic/src/db.rs +++ b/crates/ty_python_semantic/src/db.rs @@ -5,7 +5,8 @@ use ruff_db::files::File; /// Database giving access to semantic information about a Python program. #[salsa::db] pub trait Db: SourceDb { - fn is_file_open(&self, file: File) -> bool; + /// Returns `true` if the file should be checked. + fn should_check_file(&self, file: File) -> bool; /// Resolves the rule selection for a given file. fn rule_selection(&self, file: File) -> &RuleSelection; @@ -114,7 +115,7 @@ pub(crate) mod tests { #[salsa::db] impl Db for TestDb { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index ef273f2b14..a1fe3ffa1e 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -2522,7 +2522,7 @@ impl SemanticSyntaxContext for SemanticIndexBuilder<'_, '_> { } fn report_semantic_error(&self, error: SemanticSyntaxError) { - if self.db.is_file_open(self.file) { + if self.db.should_check_file(self.file) { self.semantic_syntax_errors.borrow_mut().push(error); } } diff --git a/crates/ty_python_semantic/src/types/context.rs b/crates/ty_python_semantic/src/types/context.rs index fb4d449c54..7c9e0cdd8c 100644 --- a/crates/ty_python_semantic/src/types/context.rs +++ b/crates/ty_python_semantic/src/types/context.rs @@ -399,7 +399,7 @@ impl<'db, 'ctx> LintDiagnosticGuardBuilder<'db, 'ctx> { // returns a rule selector for a given file that respects the package's settings, // any global pragma comments in the file, and any per-file-ignores. - if !ctx.db.is_file_open(ctx.file) { + if !ctx.db.should_check_file(ctx.file) { return None; } let lint_id = LintId::of(lint); @@ -573,7 +573,7 @@ impl<'db, 'ctx> DiagnosticGuardBuilder<'db, 'ctx> { id: DiagnosticId, severity: Severity, ) -> Option> { - if !ctx.db.is_file_open(ctx.file) { + if !ctx.db.should_check_file(ctx.file) { return None; } Some(DiagnosticGuardBuilder { ctx, id, severity }) diff --git a/crates/ty_python_semantic/tests/corpus.rs b/crates/ty_python_semantic/tests/corpus.rs index 799e76dd11..83ad7ae1ff 100644 --- a/crates/ty_python_semantic/tests/corpus.rs +++ b/crates/ty_python_semantic/tests/corpus.rs @@ -244,7 +244,7 @@ impl ruff_db::Db for CorpusDb { #[salsa::db] impl ty_python_semantic::Db for CorpusDb { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_server/src/logging.rs b/crates/ty_server/src/logging.rs index 97870753c2..ce92a803ab 100644 --- a/crates/ty_server/src/logging.rs +++ b/crates/ty_server/src/logging.rs @@ -98,7 +98,7 @@ impl tracing_subscriber::layer::Filter for LogLevelFilter { meta: &tracing::Metadata<'_>, _: &tracing_subscriber::layer::Context<'_, S>, ) -> bool { - let filter = if meta.target().starts_with("ty") { + let filter = if meta.target().starts_with("ty") || meta.target().starts_with("ruff") { self.filter.trace_level() } else { tracing::Level::WARN diff --git a/crates/ty_server/src/server/api/notifications/did_close.rs b/crates/ty_server/src/server/api/notifications/did_close.rs index 3c38607b00..751808fab4 100644 --- a/crates/ty_server/src/server/api/notifications/did_close.rs +++ b/crates/ty_server/src/server/api/notifications/did_close.rs @@ -8,7 +8,8 @@ use crate::system::AnySystemPath; use lsp_server::ErrorCode; use lsp_types::notification::DidCloseTextDocument; use lsp_types::{DidCloseTextDocumentParams, TextDocumentIdentifier}; -use ty_project::watch::ChangeEvent; +use ruff_db::Db as _; +use ty_project::Db as _; pub(crate) struct DidCloseTextDocumentHandler; @@ -38,11 +39,29 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler { .close_document(&key) .with_failure_code(ErrorCode::InternalError)?; - if let AnySystemPath::SystemVirtual(virtual_path) = key.path() { - session.apply_changes( - key.path(), - vec![ChangeEvent::DeletedVirtual(virtual_path.clone())], - ); + let path = key.path(); + let db = session.project_db_mut(path); + + match path { + AnySystemPath::System(system_path) => { + if let Some(file) = db.files().try_system(db, system_path) { + db.project().close_file(db, file); + } else { + // This can only fail when the path is a directory or it doesn't exists but the + // file should exists for this handler in this branch. This is because every + // close call is preceded by an open call, which ensures that the file is + // interned in the lookup table (`Files`). + tracing::warn!("Salsa file does not exists for {}", system_path); + } + } + AnySystemPath::SystemVirtual(virtual_path) => { + if let Some(virtual_file) = db.files().try_virtual_file(virtual_path) { + db.project().close_file(db, virtual_file.file()); + virtual_file.close(db); + } else { + tracing::warn!("Salsa virtual file does not exists for {}", virtual_path); + } + } } if !session.global_settings().diagnostic_mode().is_workspace() { diff --git a/crates/ty_server/src/server/api/notifications/did_open.rs b/crates/ty_server/src/server/api/notifications/did_open.rs index dfb1e1b472..5647bb2781 100644 --- a/crates/ty_server/src/server/api/notifications/did_open.rs +++ b/crates/ty_server/src/server/api/notifications/did_open.rs @@ -1,5 +1,9 @@ use lsp_types::notification::DidOpenTextDocument; use lsp_types::{DidOpenTextDocumentParams, TextDocumentItem}; +use ruff_db::Db as _; +use ruff_db::files::system_path_to_file; +use ty_project::Db as _; +use ty_project::watch::{ChangeEvent, CreatedKind}; use crate::TextDocument; use crate::server::Result; @@ -8,8 +12,6 @@ use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::session::Session; use crate::session::client::Client; use crate::system::AnySystemPath; -use ruff_db::Db; -use ty_project::watch::ChangeEvent; pub(crate) struct DidOpenTextDocumentHandler; @@ -46,13 +48,38 @@ impl SyncNotificationHandler for DidOpenTextDocumentHandler { let path = key.path(); + // This is a "maybe" because the `File` might've not been interned yet i.e., the + // `try_system` call will return `None` which doesn't mean that the file is new, it's just + // that the server didn't need the file yet. + let is_maybe_new_system_file = path.as_system().is_some_and(|system_path| { + let db = session.project_db(path); + db.files() + .try_system(db, system_path) + .is_none_or(|file| !file.exists(db)) + }); + match path { AnySystemPath::System(system_path) => { - session.apply_changes(path, vec![ChangeEvent::Opened(system_path.clone())]); + let event = if is_maybe_new_system_file { + ChangeEvent::Created { + path: system_path.clone(), + kind: CreatedKind::File, + } + } else { + ChangeEvent::Opened(system_path.clone()) + }; + session.apply_changes(path, vec![event]); + + let db = session.project_db_mut(path); + match system_path_to_file(db, system_path) { + Ok(file) => db.project().open_file(db, file), + Err(err) => tracing::warn!("Failed to open file {system_path}: {err}"), + } } AnySystemPath::SystemVirtual(virtual_path) => { let db = session.project_db_mut(path); - db.files().virtual_file(db, virtual_path); + let virtual_file = db.files().virtual_file(db, virtual_path); + db.project().open_file(db, virtual_file.file()); } } diff --git a/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs b/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs index 060bd71491..0fd58f2172 100644 --- a/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs +++ b/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs @@ -7,7 +7,6 @@ use lsp_types::{ WorkspaceFullDocumentDiagnosticReport, }; use rustc_hash::FxHashMap; -use ty_project::CheckMode; use crate::server::Result; use crate::server::api::diagnostics::to_lsp_diagnostic; @@ -33,6 +32,8 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler { let index = snapshot.index(); if !index.global_settings().diagnostic_mode().is_workspace() { + // VS Code sends us the workspace diagnostic request every 2 seconds, so these logs can + // be quite verbose. tracing::trace!("Workspace diagnostics is disabled; returning empty report"); return Ok(WorkspaceDiagnosticReportResult::Report( WorkspaceDiagnosticReport { items: vec![] }, @@ -42,7 +43,7 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler { let mut items = Vec::new(); for db in snapshot.projects() { - let diagnostics = db.check_with_mode(CheckMode::AllFiles); + let diagnostics = db.check(); // Group diagnostics by URL let mut diagnostics_by_url: FxHashMap> = FxHashMap::default(); diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 4ce3f0448c..2137efe483 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -103,8 +103,8 @@ impl Session { let index = Arc::new(Index::new(global_options.into_settings())); let mut workspaces = Workspaces::default(); - for (url, options) in workspace_folders { - workspaces.register(url, options.into_settings())?; + for (url, workspace_options) in workspace_folders { + workspaces.register(url, workspace_options.into_settings())?; } Ok(Self { @@ -347,7 +347,10 @@ impl Session { }); let (root, db) = match project { - Ok(db) => (root, db), + Ok(mut db) => { + db.set_check_mode(workspace.settings.diagnostic_mode().into_check_mode()); + (root, db) + } Err(err) => { tracing::error!( "Failed to create project for `{root}`: {err:#}. Falling back to default settings" @@ -747,17 +750,22 @@ impl DefaultProject { pub(crate) fn get(&self, index: Option<&Arc>) -> &ProjectState { self.0.get_or_init(|| { - tracing::info!("Initialize default project"); + tracing::info!("Initializing the default project"); - let system = LSPSystem::new(index.unwrap().clone()); + let index = index.unwrap(); + let system = LSPSystem::new(index.clone()); let metadata = ProjectMetadata::from_options( Options::default(), system.current_directory().to_path_buf(), None, ) .unwrap(); + + let mut db = ProjectDatabase::new(metadata, system).unwrap(); + db.set_check_mode(index.global_settings().diagnostic_mode().into_check_mode()); + ProjectState { - db: ProjectDatabase::new(metadata, system).unwrap(), + db, untracked_files_with_pushed_diagnostics: Vec::new(), } }) diff --git a/crates/ty_server/src/session/index.rs b/crates/ty_server/src/session/index.rs index 1dbf6780de..dc815292b6 100644 --- a/crates/ty_server/src/session/index.rs +++ b/crates/ty_server/src/session/index.rs @@ -176,7 +176,7 @@ impl Index { // may need revisiting in the future as we support more editors with notebook support. if let DocumentKey::NotebookCell { cell_url, .. } = key { if self.notebook_cells.remove(cell_url).is_none() { - tracing::warn!("Tried to remove a notebook cell that does not exist: {cell_url}",); + tracing::warn!("Tried to remove a notebook cell that does not exist: {cell_url}"); } return Ok(()); } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index d4a0ed2ccb..fdd651effa 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -3,6 +3,7 @@ use ruff_db::system::SystemPathBuf; use ruff_python_ast::PythonVersion; use rustc_hash::FxHashMap; use serde::Deserialize; +use ty_project::CheckMode; use ty_project::metadata::Options; use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::metadata::value::{RangedValue, RelativePathBuf}; @@ -76,6 +77,13 @@ impl DiagnosticMode { pub(crate) fn is_workspace(self) -> bool { matches!(self, DiagnosticMode::Workspace) } + + pub(crate) fn into_check_mode(self) -> CheckMode { + match self { + DiagnosticMode::OpenFilesOnly => CheckMode::OpenFiles, + DiagnosticMode::Workspace => CheckMode::AllFiles, + } + } } impl ClientOptions { diff --git a/crates/ty_test/src/db.rs b/crates/ty_test/src/db.rs index 76fff50e4e..1a47745e1d 100644 --- a/crates/ty_test/src/db.rs +++ b/crates/ty_test/src/db.rs @@ -77,7 +77,7 @@ impl SourceDb for Db { #[salsa::db] impl SemanticDb for Db { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() } diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index 4cf9662eaa..cc3d6ab29e 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -16,10 +16,10 @@ use ruff_source_file::{LineIndex, OneIndexed, SourceLocation}; use ruff_text_size::{Ranged, TextSize}; use ty_ide::signature_help; use ty_ide::{MarkupKind, goto_type_definition, hover, inlay_hints}; -use ty_project::ProjectMetadata; use ty_project::metadata::options::Options; use ty_project::metadata::value::ValueSource; use ty_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind}; +use ty_project::{CheckMode, ProjectMetadata}; use ty_project::{Db, ProjectDatabase}; use ty_python_semantic::Program; use wasm_bindgen::prelude::*; @@ -76,7 +76,11 @@ impl Workspace { let project = ProjectMetadata::from_options(options, SystemPathBuf::from(root), None) .map_err(into_error)?; - let db = ProjectDatabase::new(project, system.clone()).map_err(into_error)?; + let mut db = ProjectDatabase::new(project, system.clone()).map_err(into_error)?; + + // By default, it will check all files in the project but we only want to check the open + // files in the playground. + db.set_check_mode(CheckMode::OpenFiles); Ok(Self { db, diff --git a/fuzz/fuzz_targets/ty_check_invalid_syntax.rs b/fuzz/fuzz_targets/ty_check_invalid_syntax.rs index 4dd62cd0c5..9a6c6eb1f6 100644 --- a/fuzz/fuzz_targets/ty_check_invalid_syntax.rs +++ b/fuzz/fuzz_targets/ty_check_invalid_syntax.rs @@ -82,7 +82,7 @@ impl DbWithTestSystem for TestDb { #[salsa::db] impl SemanticDb for TestDb { - fn is_file_open(&self, file: File) -> bool { + fn should_check_file(&self, file: File) -> bool { !file.path(self).is_vendored_path() }