Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LS: Make FileDiagnostics use URLs exclusively to survive db swaps #6681

Open
wants to merge 1 commit into
base: spr/main/8a3daf09
Choose a base branch
from
Open

LS: Make FileDiagnostics use URLs exclusively to survive db swaps #6681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 14 additions & 8 deletions crates/cairo-lang-language-server/src/lang/diagnostics/file_diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use cairo_lang_parser::db::ParserGroup;
use cairo_lang_semantic::SemanticDiagnostic;
use cairo_lang_semantic::db::SemanticGroup;
use cairo_lang_utils::{LookupIntern, Upcast};
use lsp_types::Url;
use tracing::{error, info_span};

use crate::lang::db::AnalysisDatabase;
Expand All @@ -32,8 +33,8 @@ use crate::server::panic::is_cancelled;
/// to the given `file` will also be visited and their diagnostics collected.
#[derive(Clone, PartialEq, Eq)]
pub struct FileDiagnostics {
/// The file ID these diagnostics are associated with.
pub file: FileId,
/// The file URL these diagnostics are associated with.
pub url: Url,
pub parser: Diagnostics<ParserDiagnostic>,
pub semantic: Diagnostics<SemanticDiagnostic>,
pub lowering: Diagnostics<LoweringDiagnostic>,
Expand Down Expand Up @@ -69,6 +70,7 @@ impl FileDiagnostics {
};
}

let url = query!(db.url_for_file(file)).ok()??;
let module_ids = query!(db.file_modules(file)).ok()?.ok()?;

let mut semantic_file_diagnostics: Vec<SemanticDiagnostic> = vec![];
Expand Down Expand Up @@ -96,7 +98,7 @@ impl FileDiagnostics {
let parser_file_diagnostics = query!(db.file_syntax_diagnostics(file)).unwrap_or_default();

Some(FileDiagnostics {
file,
url,
parser: parser_file_diagnostics,
semantic: Diagnostics::from_iter(semantic_file_diagnostics),
lowering: Diagnostics::from_iter(lowering_file_diagnostics),
Expand All @@ -116,31 +118,35 @@ impl FileDiagnostics {
db: &AnalysisDatabase,
trace_macro_diagnostics: bool,
) -> Option<lsp_types::PublishDiagnosticsParams> {
let uri = db.url_for_file(self.file)?;
let file = db.file_for_url(&self.url)?;

let mut diagnostics = Vec::new();
map_cairo_diagnostics_to_lsp(
(*db).upcast(),
&mut diagnostics,
&self.parser,
self.file,
file,
trace_macro_diagnostics,
);
map_cairo_diagnostics_to_lsp(
(*db).upcast(),
&mut diagnostics,
&self.semantic,
self.file,
file,
trace_macro_diagnostics,
);
map_cairo_diagnostics_to_lsp(
(*db).upcast(),
&mut diagnostics,
&self.lowering,
self.file,
file,
trace_macro_diagnostics,
);

Some(lsp_types::PublishDiagnosticsParams { uri, diagnostics, version: None })
Some(lsp_types::PublishDiagnosticsParams {
uri: self.url.clone(),
diagnostics,
version: None,
})
}
}
10 changes: 5 additions & 5 deletions crates/cairo-lang-language-server/src/lang/diagnostics/project_diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,22 @@ impl ProjectDiagnostics {
/// Inserts new diagnostics for a file if they update the existing diagnostics.
///
/// Returns `true` if stored diagnostics were updated; otherwise, returns `false`.
pub fn insert(&self, file_url: &Url, new_file_diagnostics: FileDiagnostics) -> bool {
if let Some(old_file_diagnostics) = self
pub fn insert(&self, diags: FileDiagnostics) -> bool {
if let Some(old_diags) = self
.file_diagnostics
.read()
.expect("file diagnostics are poisoned, bailing out")
.get(file_url)
.get(&diags.url)
{
if *old_file_diagnostics == new_file_diagnostics {
if *old_diags == diags {
return false;
}
};

self.file_diagnostics
.write()
.expect("file diagnostics are poisoned, bailing out")
.insert(file_url.clone(), new_file_diagnostics);
.insert(diags.url.clone(), diags);
true
}

Expand Down
9 changes: 1 addition & 8 deletions crates/cairo-lang-language-server/src/lang/diagnostics/refresh.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ use std::collections::HashSet;

use cairo_lang_defs::ids::ModuleId;
use cairo_lang_filesystem::ids::FileId;
use cairo_lang_utils::LookupIntern;
use lsp_types::Url;
use lsp_types::notification::PublishDiagnostics;
use tracing::trace;

use crate::lang::db::AnalysisDatabase;
use crate::lang::diagnostics::file_diagnostics::FileDiagnostics;
Expand Down Expand Up @@ -46,16 +44,11 @@ fn refresh_file_diagnostics(
project_diagnostics: &ProjectDiagnostics,
notifier: &Notifier,
) {
let Some(file_uri) = db.url_for_file(file) else {
trace!("url for file not found: {:?}", file.lookup_intern(db));
return;
};

let Some(new_file_diagnostics) = FileDiagnostics::collect(db, file, processed_modules) else {
return;
};

if project_diagnostics.insert(&file_uri, new_file_diagnostics.clone()) {
if project_diagnostics.insert(new_file_diagnostics.clone()) {
if let Some(params) = new_file_diagnostics.to_lsp(db, trace_macro_diagnostics) {
notifier.notify::<PublishDiagnostics>(params);
}
Expand Down