Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,12 +450,12 @@ impl ty_project::ProgressReporter for IndicatifReporter {
self.bar.set_draw_target(self.printer.progress_target());
}

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

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.

self.collector.report_checked_file(db, file, diagnostics);
self.bar.inc(1);
}

fn report_diagnostics(&mut self, db: &dyn Db, diagnostics: Vec<Diagnostic>) {
fn report_diagnostics(&mut self, db: &ProjectDatabase, diagnostics: Vec<Diagnostic>) {
self.collector.report_diagnostics(db, diagnostics);
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ty_project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,12 @@ pub trait ProgressReporter: Send + Sync {
fn set_files(&mut self, files: usize);

/// Report the completion of checking a given file along with its diagnostics.
fn report_checked_file(&self, db: &dyn Db, file: File, diagnostics: &[Diagnostic]);
fn report_checked_file(&self, db: &ProjectDatabase, file: File, diagnostics: &[Diagnostic]);

/// Reports settings or IO related diagnostics. The diagnostics
/// can belong to different files or no file at all.
/// But it's never a file for which [`Self::report_checked_file`] gets called.
fn report_diagnostics(&mut self, db: &dyn Db, diagnostics: Vec<Diagnostic>);
fn report_diagnostics(&mut self, db: &ProjectDatabase, diagnostics: Vec<Diagnostic>);
}

/// Reporter that collects all diagnostics into a `Vec`.
Expand All @@ -149,7 +149,7 @@ impl CollectReporter {

impl ProgressReporter for CollectReporter {
fn set_files(&mut self, _files: usize) {}
fn report_checked_file(&self, _db: &dyn Db, _file: File, diagnostics: &[Diagnostic]) {
fn report_checked_file(&self, _db: &ProjectDatabase, _file: File, diagnostics: &[Diagnostic]) {
if diagnostics.is_empty() {
return;
}
Expand All @@ -160,7 +160,7 @@ impl ProgressReporter for CollectReporter {
.extend(diagnostics.iter().map(Clone::clone));
}

fn report_diagnostics(&mut self, _db: &dyn Db, diagnostics: Vec<Diagnostic>) {
fn report_diagnostics(&mut self, _db: &ProjectDatabase, diagnostics: Vec<Diagnostic>) {
self.0.get_mut().unwrap().extend(diagnostics);
}
}
Expand Down
33 changes: 33 additions & 0 deletions crates/ty_server/src/db.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use crate::NotebookDocument;
use crate::session::index::Document;
use crate::system::LSPSystem;
use ruff_db::Db as _;
use ruff_db::files::{File, FilePath};
use ty_project::{Db as ProjectDb, ProjectDatabase};

#[salsa::db]
pub(crate) trait Db: ProjectDb {
Comment on lines +6 to +9
Copy link
Contributor

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

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

Choose a reason for hiding this comment

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

Makes sense, thank you.

/// Returns the LSP [`Document`] corresponding to `File` or
/// `None` if the file isn't open in the editor.
fn document(&self, file: File) -> Option<&Document>;
Copy link
Contributor

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

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>.


/// Returns the LSP [`NotebookDocument`] corresponding to `File` or
/// `None` if the file isn't open in the editor or if it isn't a notebook.
fn notebook_document(&self, file: File) -> Option<&NotebookDocument> {
self.document(file)?.as_notebook()
}
}

#[salsa::db]
impl Db for ProjectDatabase {
fn document(&self, file: File) -> Option<&Document> {
self.system()
.as_any()
.downcast_ref::<LSPSystem>()
.and_then(|system| match file.path(self) {
FilePath::System(path) => system.system_path_to_document(path),
FilePath::SystemVirtual(path) => system.system_virtual_path_to_document(path),
FilePath::Vendored(_) => None,
})
}
}
7 changes: 4 additions & 3 deletions crates/ty_server/src/document/location.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::PositionEncoding;
use crate::document::{FileRangeExt, ToRangeExt};
use lsp_types::Location;
use ruff_db::files::FileRange;
use ty_ide::{NavigationTarget, ReferenceTarget};
use ty_python_semantic::Db;

use crate::Db;
use crate::PositionEncoding;
use crate::document::{FileRangeExt, ToRangeExt};

pub(crate) trait ToLink {
fn to_location(&self, db: &dyn Db, encoding: PositionEncoding) -> Option<Location>;
Expand Down
2 changes: 1 addition & 1 deletion crates/ty_server/src/document/range.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::PositionEncoding;
use crate::Db;
use crate::system::file_to_url;
use ty_python_semantic::Db;

use lsp_types as types;
use lsp_types::{Location, Position, Url};
Expand Down
2 changes: 2 additions & 0 deletions crates/ty_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ use anyhow::Context;
use lsp_server::Connection;
use ruff_db::system::{OsSystem, SystemPathBuf};

use crate::db::Db;
pub use crate::logging::{LogLevel, init_logging};
pub use crate::server::{PartialWorkspaceProgress, PartialWorkspaceProgressParams, Server};
pub use crate::session::{ClientOptions, DiagnosticMode};
pub use document::{NotebookDocument, PositionEncoding, TextDocument};
pub(crate) use session::Session;

mod capabilities;
mod db;
mod document;
mod logging;
mod server;
Expand Down
3 changes: 2 additions & 1 deletion crates/ty_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ use rustc_hash::FxHashMap;
use ruff_db::diagnostic::{Annotation, Severity, SubDiagnostic};
use ruff_db::files::FileRange;
use ruff_db::system::SystemPathBuf;
use ty_project::{Db, ProjectDatabase};
use ty_project::{Db as _, ProjectDatabase};

use crate::Db;
use crate::document::{FileRangeExt, ToRangeExt};
use crate::session::DocumentSnapshot;
use crate::session::client::Client;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::system::AnySystemPath;
use lsp_types as types;
use lsp_types::{FileChangeType, notification as notif};
use rustc_hash::FxHashMap;
use ty_project::Db;
use ty_project::Db as _;
use ty_project::watch::{ChangeEvent, ChangedKind, CreatedKind, DeletedKind};

pub(crate) struct DidChangeWatchedFiles;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use lsp_types::request::DocumentSymbolRequest;
use lsp_types::{DocumentSymbol, DocumentSymbolParams, SymbolInformation, Url};
use ruff_db::files::File;
use ty_ide::{HierarchicalSymbols, SymbolId, SymbolInfo, document_symbols};
use ty_project::{Db, ProjectDatabase};
use ty_project::ProjectDatabase;

use crate::Db;
use crate::document::{PositionEncoding, ToRangeExt};
use crate::server::api::symbols::{convert_symbol_kind, convert_to_lsp_symbol_information};
use crate::server::api::traits::{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use lsp_server::ErrorCode;
use lsp_types::{self as types, request as req};
use std::fmt::Write;
use std::str::FromStr;
use ty_project::Db;
use ty_project::Db as _;

pub(crate) struct ExecuteCommand;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::sync::Mutex;
use std::time::{Duration, Instant};
use ty_project::{Db, ProgressReporter};
use ty_project::{ProgressReporter, ProjectDatabase};

/// Handler for [Workspace diagnostics](workspace-diagnostics)
///
Expand Down Expand Up @@ -230,7 +230,7 @@ impl ProgressReporter for WorkspaceDiagnosticsProgressReporter<'_> {
state.report_progress(&self.work_done);
}

fn report_checked_file(&self, db: &dyn Db, file: File, diagnostics: &[Diagnostic]) {
fn report_checked_file(&self, db: &ProjectDatabase, file: File, diagnostics: &[Diagnostic]) {
// Another thread might have panicked at this point because of a salsa cancellation which
// poisoned the result. If the response is poisoned, just don't report and wait for our thread
// to unwind with a salsa cancellation next.
Expand Down Expand Up @@ -260,7 +260,7 @@ impl ProgressReporter for WorkspaceDiagnosticsProgressReporter<'_> {
state.response.maybe_flush();
}

fn report_diagnostics(&mut self, db: &dyn Db, diagnostics: Vec<Diagnostic>) {
fn report_diagnostics(&mut self, db: &ProjectDatabase, diagnostics: Vec<Diagnostic>) {
let mut by_file: BTreeMap<File, Vec<Diagnostic>> = BTreeMap::new();

for diagnostic in diagnostics {
Expand Down Expand Up @@ -358,7 +358,12 @@ impl<'a> ResponseWriter<'a> {
}
}

fn write_diagnostics_for_file(&mut self, db: &dyn Db, file: File, diagnostics: &[Diagnostic]) {
fn write_diagnostics_for_file(
&mut self,
db: &ProjectDatabase,
file: File,
diagnostics: &[Diagnostic],
) {
let Some(url) = file_to_url(db, file) else {
tracing::debug!("Failed to convert file path to URL at {}", file.path(db));
return;
Expand Down
2 changes: 1 addition & 1 deletion crates/ty_server/src/server/api/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

use lsp_types::{SymbolInformation, SymbolKind};
use ty_ide::SymbolInfo;
use ty_project::Db;

use crate::Db;
use crate::document::{PositionEncoding, ToRangeExt};

/// Convert `ty_ide` `SymbolKind` to LSP `SymbolKind`
Expand Down
25 changes: 14 additions & 11 deletions crates/ty_server/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::fmt::Display;
use std::panic::RefUnwindSafe;
use std::sync::Arc;

use crate::Db;
use crate::document::DocumentKey;
use crate::session::index::{Document, Index};
use lsp_types::Url;
Expand All @@ -16,7 +17,6 @@ use ruff_db::system::{
};
use ruff_notebook::{Notebook, NotebookError};
use ty_ide::cached_vendored_path;
use ty_python_semantic::Db;

/// Returns a [`Url`] for the given [`File`].
pub(crate) fn file_to_url(db: &dyn Db, file: File) -> Option<Url> {
Expand Down Expand Up @@ -112,25 +112,28 @@ impl LSPSystem {
self.index.as_ref().unwrap()
}

fn make_document_ref(&self, path: AnySystemPath) -> Option<&Document> {
fn document(&self, path: AnySystemPath) -> Option<&Document> {
let index = self.index();
index.document(&DocumentKey::from(path)).ok()
}

fn system_path_to_document_ref(&self, path: &SystemPath) -> Option<&Document> {
pub(crate) fn system_path_to_document(&self, path: &SystemPath) -> Option<&Document> {
let any_path = AnySystemPath::System(path.to_path_buf());
self.make_document_ref(any_path)
self.document(any_path)
}

fn system_virtual_path_to_document_ref(&self, path: &SystemVirtualPath) -> Option<&Document> {
pub(crate) fn system_virtual_path_to_document(
&self,
path: &SystemVirtualPath,
) -> Option<&Document> {
let any_path = AnySystemPath::SystemVirtual(path.to_path_buf());
self.make_document_ref(any_path)
self.document(any_path)
}
}

impl System for LSPSystem {
fn path_metadata(&self, path: &SystemPath) -> Result<Metadata> {
let document = self.system_path_to_document_ref(path);
let document = self.system_path_to_document(path);

if let Some(document) = document {
Ok(Metadata::new(
Expand All @@ -152,7 +155,7 @@ impl System for LSPSystem {
}

fn read_to_string(&self, path: &SystemPath) -> Result<String> {
let document = self.system_path_to_document_ref(path);
let document = self.system_path_to_document(path);

match document {
Some(Document::Text(document)) => Ok(document.contents().to_string()),
Expand All @@ -161,7 +164,7 @@ impl System for LSPSystem {
}

fn read_to_notebook(&self, path: &SystemPath) -> std::result::Result<Notebook, NotebookError> {
let document = self.system_path_to_document_ref(path);
let document = self.system_path_to_document(path);

match document {
Some(Document::Text(document)) => Notebook::from_source_code(document.contents()),
Expand All @@ -172,7 +175,7 @@ impl System for LSPSystem {

fn read_virtual_path_to_string(&self, path: &SystemVirtualPath) -> Result<String> {
let document = self
.system_virtual_path_to_document_ref(path)
.system_virtual_path_to_document(path)
.ok_or_else(|| virtual_path_not_found(path))?;

if let Document::Text(document) = &document {
Expand All @@ -187,7 +190,7 @@ impl System for LSPSystem {
path: &SystemVirtualPath,
) -> std::result::Result<Notebook, NotebookError> {
let document = self
.system_virtual_path_to_document_ref(path)
.system_virtual_path_to_document(path)
.ok_or_else(|| virtual_path_not_found(path))?;

match document {
Expand Down
Loading