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() }