Skip to content

Commit

Permalink
Compute cross module diagnostics (#437)
Browse files Browse the repository at this point in the history
* Implement lsp did_close

* Compute cross-module diagnostics
  • Loading branch information
timsueberkrueb authored Jan 15, 2025
1 parent 7df0e1d commit 16e1ef6
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 22 deletions.
6 changes: 2 additions & 4 deletions lang/driver/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,8 @@ impl Database {
}

/// Invalidate the file behind the given URI and all its reverse dependencies
pub async fn invalidate(&mut self, uri: &Url) -> Result<(), Error> {
pub async fn invalidate(&mut self, uri: &Url) {
self.invalidate_impl(uri);
self.build_dependency_dag().await?;
let rev_deps: HashSet<Url> =
self.deps.reverse_dependencies(uri).into_iter().cloned().collect();
log::debug!(
Expand All @@ -396,7 +395,6 @@ impl Database {
for rev_dep in &rev_deps {
self.invalidate_impl(rev_dep);
}
Ok(())
}

fn invalidate_impl(&mut self, uri: &Url) {
Expand Down Expand Up @@ -432,7 +430,7 @@ impl Database {
}

pub async fn write_source(&mut self, uri: &Url, source: &str) -> Result<(), Error> {
self.invalidate(uri).await?;
self.invalidate(uri).await;
self.source.write_string(uri, source).await.map_err(|err| err.into())
}

Expand Down
20 changes: 20 additions & 0 deletions lang/driver/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ pub trait FileSource: Send + Sync {
fn manage(&mut self, uri: &Url) -> bool;
/// Check if the source manages a file with the given URI
fn manages(&self, uri: &Url) -> bool;
/// Stop managing a file with the given URI
///
/// This is mostly relevant for in-memory sources.
/// Returns `true` if the source was managing the file.
fn forget(&mut self, uri: &Url) -> bool;
/// Read the contents of a file with the given URI
async fn read_to_string(&mut self, uri: &Url) -> Result<String, DriverError>;
/// Write the contents of a file with the given URI
Expand Down Expand Up @@ -61,6 +66,11 @@ mod file_system {
self.root.join(filepath).exists()
}

fn forget(&mut self, uri: &Url) -> bool {
let filepath = uri.to_file_path().expect("Failed to convert URI to filepath");
self.root.join(filepath).exists()
}

async fn read_to_string(&mut self, uri: &Url) -> Result<String, DriverError> {
let filepath = uri.to_file_path().expect("Failed to convert URI to filepath");
let path = self.root.join(filepath);
Expand Down Expand Up @@ -116,6 +126,12 @@ impl FileSource for InMemorySource {
self.files.contains_key(uri)
}

fn forget(&mut self, uri: &Url) -> bool {
let managed = self.files.remove(uri).is_some();
self.modified.remove(uri);
managed
}

async fn read_to_string(&mut self, uri: &Url) -> Result<String, DriverError> {
if self.manages(uri) {
self.modified.insert(uri.clone(), false);
Expand Down Expand Up @@ -158,6 +174,10 @@ where
self.first.manages(uri) || self.second.manages(uri)
}

fn forget(&mut self, uri: &Url) -> bool {
self.first.forget(uri) || self.second.forget(uri)
}

async fn read_to_string(&mut self, uri: &Url) -> Result<String, DriverError> {
match self.first.read_to_string(uri).await {
Ok(source) => Ok(source),
Expand Down
33 changes: 27 additions & 6 deletions lang/lsp/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,38 @@ use miette_util::FromMiette;
use crate::conversion::ToLsp;

pub trait Diagnostics {
fn diagnostics(&self, uri: &Url, result: Result<(), Error>) -> Vec<lsp_types::Diagnostic> {
match result {
Ok(()) => vec![],
Err(err) => self.error_diagnostics(uri, err),
}
}
/// Compute the diagnostics for the given URI and all of its reverse dependencies.
async fn diagnostics(&mut self, uri: &Url, result: Result<(), Error>) -> DiagnosticsPerUri;

fn error_diagnostics(&self, uri: &Url, error: Error) -> Vec<lsp_types::Diagnostic>;
}

pub type DiagnosticsPerUri = ast::HashMap<Url, Vec<lsp_types::Diagnostic>>;

impl Diagnostics for Database {
async fn diagnostics(&mut self, uri: &Url, result: Result<(), Error>) -> DiagnosticsPerUri {
// When computing the diagnostics for an URI, we also need to recompute the diagnostics for all of its reverse dependencies.
let rev_deps: Vec<_> = self.deps.reverse_dependencies(uri).into_iter().cloned().collect();
let mut diagnostics = ast::HashMap::default();

for uri in rev_deps {
let mut diagnostics_for_uri = vec![];
let ast = self.ast(&uri).await;
if let Err(err) = ast {
diagnostics_for_uri.extend(self.error_diagnostics(&uri, err));
}
diagnostics.insert(uri, diagnostics_for_uri);
}

if let Err(err) = result {
diagnostics.insert(uri.clone(), self.error_diagnostics(uri, err));
} else {
diagnostics.insert(uri.clone(), vec![]);
}

diagnostics
}

fn error_diagnostics(&self, uri: &Url, error: Error) -> Vec<lsp_types::Diagnostic> {
// Compute the range where the error should be displayed.
// The range is computed from the first available label, otherwise
Expand Down
38 changes: 26 additions & 12 deletions lang/lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use driver::Database;
#[cfg(not(target_arch = "wasm32"))]
use driver::{FileSource, FileSystemSource, InMemorySource};

use crate::conversion::FromLsp;
use crate::conversion::{FromLsp, ToLsp};

use super::capabilities::*;
use super::diagnostics::*;
Expand Down Expand Up @@ -74,8 +74,23 @@ impl LanguageServer for Server {
source_mut.write_string(&text_document.uri.from_lsp(), &text_document.text).await.unwrap();

let res = db.ast(&text_document.uri.from_lsp()).await.map(|_| ());
let diags = db.diagnostics(&text_document.uri.from_lsp(), res);
self.send_diagnostics(text_document.uri, diags).await;
let diags = db.diagnostics(&text_document.uri.from_lsp(), res).await;
self.send_diagnostics(diags).await;
}

async fn did_close(&self, params: DidCloseTextDocumentParams) {
let text_document = params.text_document;
let mut db = self.database.write().await;

self.client
.log_message(
MessageType::INFO,
format!("Closed file: {}", text_document.uri.from_lsp()),
)
.await;

let source_mut = db.file_source_mut();
source_mut.forget(&text_document.uri.from_lsp());
}

async fn did_change(&self, params: DidChangeTextDocumentParams) {
Expand All @@ -96,15 +111,12 @@ impl LanguageServer for Server {
assert!(source_mut.manage(&text_document.uri.from_lsp()));
source_mut.write_string(&text_document.uri.from_lsp(), &text).await.unwrap();

let res = db.invalidate(&text_document.uri.from_lsp()).await;
db.invalidate(&text_document.uri.from_lsp()).await;

let res = match res {
Ok(()) => db.ast(&text_document.uri.from_lsp()).await.map(|_| ()),
Err(err) => Err(err),
};
let res = db.ast(&text_document.uri.from_lsp()).await.map(|_| ());
let diags = db.diagnostics(&text_document.uri.from_lsp(), res).await;

let diags = db.diagnostics(&text_document.uri.from_lsp(), res);
self.send_diagnostics(text_document.uri, diags).await;
self.send_diagnostics(diags).await;
}

async fn goto_definition(
Expand All @@ -131,7 +143,9 @@ impl LanguageServer for Server {
}

impl Server {
pub(crate) async fn send_diagnostics(&self, uri: Uri, diags: Vec<Diagnostic>) {
self.client.publish_diagnostics(uri, diags, None).await;
pub(crate) async fn send_diagnostics(&self, diags: DiagnosticsPerUri) {
for (uri, diags) in diags {
self.client.publish_diagnostics(uri.to_lsp(), diags, None).await;
}
}
}
4 changes: 4 additions & 0 deletions web/crates/browser/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ impl FileSource for FetchSource {
true
}

fn forget(&mut self, _uri: &reqwest::Url) -> bool {
true
}

async fn read_to_string(&mut self, uri: &reqwest::Url) -> Result<String, DriverError> {
let url =
get_base_url().join(uri.path()).map_err(|_| DriverError::FileNotFound(uri.clone()))?;
Expand Down

0 comments on commit 16e1ef6

Please sign in to comment.