mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 06:11:21 +00:00
[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
This commit is contained in:
parent
ba7ed3a6f9
commit
99d0ac60b4
22 changed files with 220 additions and 140 deletions
|
@ -98,7 +98,7 @@ impl<S> tracing_subscriber::layer::Filter<S> 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
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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<Url, Vec<_>> = FxHashMap::default();
|
||||
|
|
|
@ -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<Index>>) -> &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(),
|
||||
}
|
||||
})
|
||||
|
|
|
@ -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(());
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue