Skip to content

Commit

Permalink
LS: Skip warnings in deps
Browse files Browse the repository at this point in the history
fix #6310

commit-id:4536ca03
  • Loading branch information
Draggu committed Sep 5, 2024
1 parent 3f92254 commit 1524391
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 19 deletions.
81 changes: 63 additions & 18 deletions crates/cairo-lang-language-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use cairo_lang_defs::ids::{
FunctionTitleId, LanguageElementId, LookupItemId, MemberId, ModuleId, SubmoduleLongId,
TraitItemId,
};
use cairo_lang_diagnostics::{DiagnosticEntry, Severity};
use cairo_lang_diagnostics::{Diagnostics, ToOption};
use cairo_lang_filesystem::db::{
get_originating_location, AsFilesGroupMut, FilesGroup, FilesGroupEx, PrivRawFileContentQuery,
Expand All @@ -79,7 +80,7 @@ use cairo_lang_utils::{Intern, LookupIntern, Upcast};
use itertools::Itertools;
use salsa::{Cancelled, ParallelDatabase};
use serde_json::Value;
use state::{FileDiagnostics, Owned, StateSnapshot};
use state::{Dependencies, FileDiagnostics, Owned, StateSnapshot};
use tokio::sync::Semaphore;
use tokio::task::spawn_blocking;
use tower_lsp::jsonrpc::{Error as LSPError, Result as LSPResult};
Expand Down Expand Up @@ -372,6 +373,13 @@ impl Backend {
/// Refresh diagnostics and send diffs to client.
#[tracing::instrument(level = "debug", skip_all)]
async fn refresh_diagnostics(&self) -> LSPResult<()> {
fn is_virtual_file(db: &AnalysisDatabase, file: FileId) -> bool {
!matches!(
file.lookup_intern(<AnalysisDatabase as Upcast<dyn FilesGroup>>::upcast(&db)),
FileLongId::OnDisk(_)
)
}

// Making sure only a single thread is refreshing diagnostics at a time, and that at most
// one thread is waiting to start refreshing. This allows changed to be grouped
// together before querying the database, as well as releasing extra threads waiting to
Expand All @@ -395,10 +403,13 @@ impl Backend {

// Refresh open files modules first for better UX
async {
let db = self.db_snapshot().await;

for (file, file_modules_ids) in open_files_modules {
self.refresh_file_diagnostics(
&file,
&file_modules_ids,
is_virtual_file(&db, file), // Show warnings for open files even if this is dependency.
&mut processed_modules,
&mut files_with_set_diagnostics,
)
Expand All @@ -408,19 +419,30 @@ impl Backend {
.instrument(trace_span!("refresh_open_files_modules"))
.await;

let rest_of_files = async {
let (rest_of_files, skip_warnings_files) = async {
let mut rest_of_files: HashSet<FileId> = HashSet::default();
let db = self.db_snapshot().await;
let mut skip_warnings_files: HashSet<FileId> = HashSet::default();
let state = self.state_snapshot().await;
let db = state.db;
for crate_id in db.crates() {
let is_dependency = state.dependencies.contains(&crate_id);

for module_id in db.crate_modules(crate_id).iter() {
if let Ok(module_files) = db.module_files(*module_id) {
skip_warnings_files.extend(
module_files
.iter()
.filter(|file| is_dependency || is_virtual_file(&db, **file)),
);

let unprocessed_files =
module_files.iter().filter(|file| !open_files_ids.contains(file));

rest_of_files.extend(unprocessed_files);
}
}
}
rest_of_files
(rest_of_files, skip_warnings_files)
}
.instrument(trace_span!("get_rest_of_files"))
.await;
Expand All @@ -430,9 +452,12 @@ impl Backend {
// Refresh rest of files after, since they are not viewed currently
async {
for (file, file_modules_ids) in rest_of_files_modules {
let skip_warnings = skip_warnings_files.contains(&file);

self.refresh_file_diagnostics(
&file,
&file_modules_ids,
skip_warnings,
&mut processed_modules,
&mut files_with_set_diagnostics,
)
Expand Down Expand Up @@ -479,6 +504,7 @@ impl Backend {
&self,
file: &FileId,
modules_ids: &Vec<ModuleId>,
skip_warnings: bool,
processed_modules: &mut HashSet<ModuleId>,
files_with_set_diagnostics: &mut HashSet<Url>,
) {
Expand Down Expand Up @@ -519,10 +545,26 @@ impl Backend {

let parser_file_diagnostics = diags!(db.file_syntax_diagnostics(*file), |r| r);

let new_file_diagnostics = FileDiagnostics {
parser: parser_file_diagnostics,
semantic: Diagnostics::from_iter(semantic_file_diagnostics),
lowering: Diagnostics::from_iter(lowering_file_diagnostics),
let new_file_diagnostics = if skip_warnings {
#[inline(always)]
fn filter_warnings<T>(iter: impl IntoIterator<Item = T>) -> Diagnostics<T>
where
T: DiagnosticEntry,
{
iter.into_iter().filter(|d| d.severity() != Severity::Warning).collect()
}

FileDiagnostics {
parser: filter_warnings(parser_file_diagnostics.get_all()),
semantic: filter_warnings(semantic_file_diagnostics),
lowering: filter_warnings(lowering_file_diagnostics),
}
} else {
FileDiagnostics {
parser: parser_file_diagnostics,
semantic: Diagnostics::from_iter(semantic_file_diagnostics),
lowering: Diagnostics::from_iter(lowering_file_diagnostics),
}
};

if !new_file_diagnostics.is_empty() {
Expand Down Expand Up @@ -629,14 +671,15 @@ impl Backend {
})
.await?;
debug!("initial setup done");
self.ensure_diagnostics_queries_up_to_date(&mut new_db, config, open_files.iter().cloned())
.await;
debug!("initial compilation done");
debug!("starting");
self.with_state_mut(|state| {
state_mut_async!(state, self,
self.ensure_diagnostics_queries_up_to_date(&mut new_db, &mut state.dependencies, config, open_files.iter().cloned())
.await;
debug!("initial compilation done");
debug!("starting");

ensure_exists_in_db(&mut new_db, &state.db, state.open_files.iter().cloned());
state.db = new_db;
})
)
.await;

debug!("done");
Expand All @@ -648,6 +691,7 @@ impl Backend {
async fn ensure_diagnostics_queries_up_to_date(
&self,
db: &mut AnalysisDatabase,
deps: &mut Dependencies,
config: &Config,
open_files: impl Iterator<Item = Url>,
) {
Expand All @@ -659,7 +703,7 @@ impl Backend {
for uri in open_files {
let Some(file_id) = db.file_for_url(&uri) else { continue };
if let FileLongId::OnDisk(file_path) = file_id.lookup_intern(db) {
self.detect_crate_for(db, config, file_path).await;
self.detect_crate_for(db, deps, config, file_path).await;
}
query_diags(db, file_id);
}
Expand Down Expand Up @@ -710,6 +754,7 @@ impl Backend {
async fn detect_crate_for(
&self,
db: &mut AnalysisDatabase,
deps: &mut Dependencies,
config: &Config,
file_path: PathBuf,
) {
Expand Down Expand Up @@ -740,7 +785,7 @@ impl Backend {
};

if let Some(metadata) = metadata {
update_crate_roots(&metadata, db);
update_crate_roots(&metadata, db, deps);
} else {
// Try to set up a corelib at least.
try_to_init_unmanaged_core(db, config, &self.scarb_toolchain);
Expand Down Expand Up @@ -787,7 +832,7 @@ impl Backend {
for uri in state.open_files.iter() {
let Some(file_id) = db.file_for_url(uri) else { continue };
if let FileLongId::OnDisk(file_path) = db.lookup_intern_file(file_id) {
self.detect_crate_for(db, &state.config, file_path).await;
self.detect_crate_for(db, &mut state.dependencies, &state.config, file_path).await;
}
}
}
Expand Down Expand Up @@ -922,7 +967,7 @@ impl LanguageServer for Backend {
// The crate for virtual files is already known.
if uri.scheme() == "file" {
let Ok(path) = uri.to_file_path() else { return false };
self.detect_crate_for(&mut state.db, &state.config, path).await;
self.detect_crate_for(&mut state.db, &mut state.dependencies, &state.config, path).await;
}

let Some(file_id) = state.db.file_for_url(&uri) else { return false };
Expand Down
12 changes: 11 additions & 1 deletion crates/cairo-lang-language-server/src/project/scarb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ use std::path::Path;

use anyhow::{bail, ensure, Context, Result};
use cairo_lang_filesystem::db::{CrateSettings, Edition, ExperimentalFeaturesConfig};
use cairo_lang_filesystem::ids::CrateLongId;
use cairo_lang_utils::Intern;
use itertools::Itertools;
use scarb_metadata::{Metadata, PackageMetadata};
use tracing::{debug, error, warn};

use crate::lang::db::AnalysisDatabase;
use crate::project::crate_data::Crate;
use crate::state::Dependencies;

/// Updates crate roots in the database with the information from Scarb metadata.
///
Expand All @@ -24,10 +27,17 @@ use crate::project::crate_data::Crate;
// once. Often packages declare several targets (lib, starknet-contract, test), which currently
// causes overriding of the crate within single call of this function. This is an UX problem, for
// which we do not know the solution yet.
pub fn update_crate_roots(metadata: &Metadata, db: &mut AnalysisDatabase) {
pub fn update_crate_roots(metadata: &Metadata, db: &mut AnalysisDatabase, deps: &mut Dependencies) {
let mut crates = Vec::<Crate>::new();
let mut crates_grouped_by_group_id = HashMap::new();

*deps = metadata
.packages
.iter()
.filter(|p| !metadata.workspace.members.contains(&p.id))
.map(|p| CrateLongId::Real(p.name.clone().into()).intern(db))
.collect();

for compilation_unit in &metadata.compilation_units {
if compilation_unit.target.kind == "cairo-plugin" {
debug!("skipping cairo plugin compilation unit: {}", compilation_unit.id);
Expand Down
22 changes: 22 additions & 0 deletions crates/cairo-lang-language-server/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use tower_lsp::lsp_types::{ClientCapabilities, Url};

use crate::config::Config;
use crate::lang::db::AnalysisDatabase;
use cairo_lang_filesystem::ids::CrateId;

/// State of Language server.
pub struct State {
Expand All @@ -19,6 +20,24 @@ pub struct State {
pub file_diagnostics: Owned<HashMap<Url, FileDiagnostics>>,
pub config: Owned<Config>,
pub client_capabilities: Owned<ClientCapabilities>,
pub dependencies: Owned<Dependencies>,
}

#[derive(Debug, Clone, Default)]
pub struct Dependencies(HashSet<CrateId>);

impl Deref for Dependencies {
type Target = HashSet<CrateId>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl FromIterator<CrateId> for Dependencies {
fn from_iter<T: IntoIterator<Item = CrateId>>(iter: T) -> Self {
Self(iter.into_iter().collect())
}
}

#[derive(Clone, Default, PartialEq, Eq)]
Expand All @@ -43,6 +62,7 @@ impl State {
file_diagnostics: Default::default(),
config: Default::default(),
client_capabilities: Default::default(),
dependencies: Default::default(),
}
}

Expand All @@ -52,6 +72,7 @@ impl State {
open_files: self.open_files.snapshot(),
config: self.config.snapshot(),
client_capabilities: self.client_capabilities.snapshot(),
dependencies: self.dependencies.snapshot(),
}
}
}
Expand All @@ -62,6 +83,7 @@ pub struct StateSnapshot {
pub open_files: Snapshot<HashSet<Url>>,
pub config: Snapshot<Config>,
pub client_capabilities: Snapshot<ClientCapabilities>,
pub dependencies: Snapshot<Dependencies>,
}

impl std::panic::UnwindSafe for StateSnapshot {}
Expand Down

0 comments on commit 1524391

Please sign in to comment.