From d3705ad9a48dc1371c16fa88e0d236beed3f7c17 Mon Sep 17 00:00:00 2001 From: hongjr03 Date: Sat, 27 Jun 2026 01:56:09 +0800 Subject: [PATCH 1/2] refactor: run slang semantic diagnostics asynchronously --- crates/hir/src/base_db/project.rs | 6 + crates/ide/src/analysis.rs | 11 + crates/ide/src/diagnostics.rs | 47 +- src/global_state.rs | 25 +- src/global_state/event_loop.rs | 1 + .../handlers/request/code_action.rs | 87 ++- src/global_state/process_changes.rs | 40 +- src/global_state/qihe.rs | 1 + src/global_state/semantic_compiler.rs | 537 ++++++++++++++++++ src/global_state/snapshot.rs | 12 +- src/global_state/task.rs | 39 ++ src/tests.rs | 68 +++ src/tests/code_actions.rs | 24 +- src/tests/diagnostics.rs | 372 ++++++------ 14 files changed, 1001 insertions(+), 269 deletions(-) create mode 100644 src/global_state/semantic_compiler.rs diff --git a/crates/hir/src/base_db/project.rs b/crates/hir/src/base_db/project.rs index 76669e48..4f8712f7 100644 --- a/crates/hir/src/base_db/project.rs +++ b/crates/hir/src/base_db/project.rs @@ -107,6 +107,12 @@ impl ProjectConfig { !self.profiles.is_empty() } + pub fn profile_ids(&self) -> Vec { + (0..self.profiles.len()) + .map(|idx| CompilationProfileId(u32::try_from(idx).unwrap_or(u32::MAX))) + .collect() + } + pub fn preprocess_for_profile( &self, profile_id: Option, diff --git a/crates/ide/src/analysis.rs b/crates/ide/src/analysis.rs index 8cdfde81..b3789e41 100644 --- a/crates/ide/src/analysis.rs +++ b/crates/ide/src/analysis.rs @@ -75,6 +75,13 @@ impl Analysis { self.with_db(|db| diagnostics::diagnostics(db, file_id)) } + pub fn compilation_diagnostics( + &self, + file_id: FileId, + ) -> Cancellable> { + self.with_db(|db| diagnostics::compilation_diagnostics(db, file_id)) + } + pub fn source_root_diagnostics( &self, file_id: FileId, @@ -123,6 +130,10 @@ impl Analysis { self.with_db(|db| db.project_config().has_compilation_profiles()) } + pub fn compilation_profile_ids(&self) -> Cancellable> { + self.with_db(|db| db.project_config().profile_ids()) + } + pub fn compilation_profile_file_ids( &self, profile_id: CompilationProfileId, diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 447e31ec..6a403e1f 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -146,7 +146,7 @@ pub(crate) fn parse_diagnostics(db: &RootDb, file_id: FileId) -> Vec .collect() } -fn compilation_diagnostics(db: &RootDb, file_id: FileId) -> Vec { +pub(crate) fn compilation_diagnostics(db: &RootDb, file_id: FileId) -> Vec { db.file_compilation_diagnostics(file_id) .iter() .map(|diag| slang_diagnostic(diag.file_id, diag.source, &diag.diagnostic)) @@ -234,17 +234,7 @@ pub(crate) fn diagnostics(db: &RootDb, file_id: FileId) -> Vec { return Vec::new(); } - let mut diagnostics = if slang_semantic_diagnostics_active(db, file_id) { - inactive_preprocessor_branch_diagnostics(db, file_id) - } else { - syntax_diagnostics(db, file_id) - }; - - diagnostics.extend( - compilation_diagnostics(db, file_id).into_iter().filter(|diag| diag.file_id == file_id), - ); - - diagnostics + syntax_diagnostics(db, file_id) } pub(crate) fn source_root_diagnostics(db: &RootDb, file_id: FileId) -> Vec { @@ -258,28 +248,7 @@ pub(crate) fn source_root_diagnostics(db: &RootDb, file_id: FileId) -> Vec {} } - let mut diagnostics = Vec::new(); - - if slang_semantic_diagnostics_active(db, file_id) { - diagnostics.extend(compilation_diagnostics(db, file_id)); - diagnostics.extend( - source_root - .iter() - .flat_map(|file_id| inactive_preprocessor_branch_diagnostics(db, file_id)), - ); - } else { - for file_id in source_root.iter() { - diagnostics.extend(syntax_diagnostics(db, file_id)); - } - - diagnostics.extend(db.source_root_semantic_diagnostics(file_id).iter().map( - |(diag_file_id, diag)| { - slang_diagnostic(*diag_file_id, SlangDiagnosticSource::Semantic, diag) - }, - )); - } - - diagnostics + source_root.iter().flat_map(|file_id| syntax_diagnostics(db, file_id)).collect() } pub(crate) fn source_root_file_ids(db: &RootDb, file_id: FileId) -> Vec { @@ -468,7 +437,7 @@ mod tests { use super::{ AMBIGUOUS_MODULE_INSTANTIATION, DIAGNOSTIC_INACTIVE_PREPROCESSOR_BRANCH, DiagnosticSource, - DiagnosticTag, INACTIVE_PREPROCESSOR_BRANCH, diagnostics, source_root_diagnostics, + DiagnosticTag, INACTIVE_PREPROCESSOR_BRANCH, compilation_profile_diagnostics, diagnostics, }; use crate::db::root_db::RootDb; @@ -747,7 +716,7 @@ mod tests { true, ); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics.iter().any(|diag| diag.message.contains("port 'b' has no connection")), @@ -909,7 +878,7 @@ mod tests { ))); db.apply_change(change); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics.iter().any(|diag| diag.message.contains("missing_name")), @@ -962,7 +931,7 @@ mod tests { assert!(plan.include_only.contains(&FileId(1))); assert_eq!(plan.roots, vec![FileId(0)]); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics @@ -1033,7 +1002,7 @@ mod tests { assert!(plan.include_only.contains(&FileId(1))); assert!(plan.include_only.contains(&FileId(2))); - let diagnostics = source_root_diagnostics(&db, FileId(0)); + let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics diff --git a/src/global_state.rs b/src/global_state.rs index c2f74907..f90674a2 100644 --- a/src/global_state.rs +++ b/src/global_state.rs @@ -11,6 +11,7 @@ mod qihe; pub mod reload; pub mod respond; mod response_effect; +mod semantic_compiler; pub(crate) mod snapshot; pub(crate) mod task; mod trace; @@ -20,7 +21,7 @@ use std::{sync::Arc as StdArc, time::Instant}; use crossbeam_channel::{Receiver, Sender, unbounded}; use hir::base_db::{ - project::{ProjectConfig, SharedProjectConfig}, + project::{CompilationProfileId, ProjectConfig, SharedProjectConfig}, salsa::Durability, source_db::SourceDb, source_root::SourceRootConfig, @@ -48,7 +49,7 @@ use self::{ }, mem_docs::MemDocs, snapshot::GlobalStateSnapshot, - task::{QiheTask, Task, TaskPool}, + task::{QiheTask, SemanticCompilerTask, Task, TaskPool}, trace::LspTrace, workspace_state::WorkspaceVfsReadiness, }; @@ -72,6 +73,7 @@ pub(crate) struct GlobalState { pub(crate) diagnostics: DiagnosticsState, pub(crate) workspace: WorkspaceState, pub(crate) qihe: qihe::Qihe, + pub(crate) semantic_compiler: semantic_compiler::SemanticCompiler, pub(crate) external_sources: Vec>, pub(crate) tasks: TaskState, } @@ -151,7 +153,11 @@ impl GlobalState { let qihe_diagnostics = qihe::QiheDiagnostics::new(); let qihe = qihe::Qihe::new(qihe_diagnostics); let qihe_source: StdArc = StdArc::new(qihe.diagnostics_snapshot()); - let external_sources = vec![qihe_source]; + let semantic_diagnostics = semantic_compiler::SemanticDiagnostics::new(); + let semantic_compiler = semantic_compiler::SemanticCompiler::new(semantic_diagnostics); + let semantic_source: StdArc = + StdArc::new(semantic_compiler.diagnostics_snapshot()); + let external_sources = vec![qihe_source, semantic_source]; GlobalState { client: ClientState { @@ -188,6 +194,7 @@ impl GlobalState { registered_client_file_watcher_globs: None, }, qihe, + semantic_compiler, external_sources, tasks: TaskState { task_pool }, } @@ -232,6 +239,18 @@ impl GlobalState { qihe::with_global_ctx(self, |qihe, ctx| qihe.handle(task, ctx)); } + pub(crate) fn schedule_semantic_compiler(&mut self, profile_ids: Vec) { + semantic_compiler::with_global_ctx(self, |semantic_compiler, ctx| { + semantic_compiler.schedule(profile_ids, ctx) + }); + } + + pub(crate) fn handle_semantic_compiler_task(&mut self, task: SemanticCompilerTask) { + semantic_compiler::with_global_ctx(self, |semantic_compiler, ctx| { + semantic_compiler.handle(task, ctx) + }); + } + pub(crate) fn cancel_work_done_progress( &mut self, params: lsp_types::WorkDoneProgressCancelParams, diff --git a/src/global_state/event_loop.rs b/src/global_state/event_loop.rs index 1303b405..0c05fa9f 100644 --- a/src/global_state/event_loop.rs +++ b/src/global_state/event_loop.rs @@ -313,6 +313,7 @@ impl GlobalState { } Task::Diagnostics(diags) => self.publish_diagnostics_tasks(diags), Task::Qihe(task) => self.handle_qihe_task(task), + Task::SemanticCompiler(task) => self.handle_semantic_compiler_task(task), } } diff --git a/src/global_state/handlers/request/code_action.rs b/src/global_state/handlers/request/code_action.rs index 9b47d054..869cf3a8 100644 --- a/src/global_state/handlers/request/code_action.rs +++ b/src/global_state/handlers/request/code_action.rs @@ -6,6 +6,8 @@ use ide::{ }, diagnostics as ide_diagnostics, }; +use serde::Deserialize; +use syntax::DiagnosticSeverity; use utils::text_edit::TextRange; use vfs::FileId; @@ -92,7 +94,8 @@ fn server_diagnostics_for_code_action( client_diagnostics: &[lsp_types::Diagnostic], line_info: &utils::lines::LineInfo, ) -> anyhow::Result> { - let server_diagnostics = snap.diagnostics(file_id)?; + let mut server_diagnostics = snap.diagnostics(file_id)?; + server_diagnostics.extend(cached_external_diagnostics(snap, file_id, line_info)); let client_locators = client_diagnostics .iter() .filter_map(|diag| DiagnosticLocator::from_lsp(line_info, diag)) @@ -116,6 +119,88 @@ fn server_diagnostics_for_code_action( Ok(diagnostics) } +fn cached_external_diagnostics( + snap: &GlobalStateSnapshot, + file_id: FileId, + line_info: &utils::lines::LineInfo, +) -> Vec { + let freshness = snap.diagnostic_commit_freshness(); + snap.external_sources + .iter() + .flat_map(|source| source.diagnostics(file_id, &freshness)) + .filter_map(|diag| cached_lsp_diagnostic_to_ide(file_id, line_info, diag)) + .collect() +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct CachedDiagnosticData { + source: String, + subsystem: u16, + code: u16, + name: String, + #[serde(rename = "option")] + option_name: Option, + #[serde(default)] + groups: Vec, + #[serde(default)] + args: Vec, +} + +fn cached_lsp_diagnostic_to_ide( + file_id: FileId, + line_info: &utils::lines::LineInfo, + diag: lsp_types::Diagnostic, +) -> Option { + let data = serde_json::from_value::(diag.data.clone()?).ok()?; + let source = match data.source.as_str() { + "parse" => ide_diagnostics::DiagnosticSource::SlangParse, + "semantic" => ide_diagnostics::DiagnosticSource::SlangSemantic, + _ => return None, + }; + Some(ide_diagnostics::Diagnostic { + file_id, + code: data.code, + subsystem: data.subsystem, + name: data.name, + option_name: data.option_name, + groups: data.groups, + source, + range: from_proto::text_range(line_info, diag.range).ok()?, + severity: lsp_severity_to_ide(diag.severity), + message: diag.message, + args: data.args, + message_key: None, + message_args: Vec::new(), + tags: lsp_tags_to_ide(diag.tags), + }) +} + +fn lsp_severity_to_ide(severity: Option) -> DiagnosticSeverity { + match severity { + Some(lsp_types::DiagnosticSeverity::ERROR) => DiagnosticSeverity::Error, + Some(lsp_types::DiagnosticSeverity::WARNING) => DiagnosticSeverity::Warning, + Some(lsp_types::DiagnosticSeverity::INFORMATION | lsp_types::DiagnosticSeverity::HINT) => { + DiagnosticSeverity::Note + } + _ => DiagnosticSeverity::Error, + } +} + +fn lsp_tags_to_ide( + tags: Option>, +) -> Vec { + tags.unwrap_or_default() + .into_iter() + .filter_map(|tag| match tag { + lsp_types::DiagnosticTag::UNNECESSARY => { + Some(ide_diagnostics::DiagnosticTag::Unnecessary) + } + _ => None, + }) + .collect() +} + #[derive(Debug, Clone, PartialEq, Eq)] struct DiagnosticLocator { range: TextRange, diff --git a/src/global_state/process_changes.rs b/src/global_state/process_changes.rs index 6aea90c3..5e97e91c 100644 --- a/src/global_state/process_changes.rs +++ b/src/global_state/process_changes.rs @@ -1,6 +1,6 @@ use std::collections::hash_map::Entry::{Occupied, Vacant}; -use hir::base_db::change::Change; +use hir::base_db::{change::Change, project::CompilationProfileId}; use itertools::Itertools; use lsp_types::request::WorkspaceDiagnosticRefresh; use nohash_hasher::IntMap; @@ -169,6 +169,9 @@ impl GlobalState { return; } + let semantic_profile_ids = self.semantic_compiler_profiles_for_invalidation(&invalidation); + self.schedule_semantic_compiler(semantic_profile_ids); + if self.config_state.config.cli_pull_diagnostics_support() && self.config_state.config.cli_workspace_diagnostic_refresh_support() && match &invalidation { @@ -191,6 +194,41 @@ impl GlobalState { self.request_diagnostics(file_ids); } + fn semantic_compiler_profiles_for_invalidation( + &self, + invalidation: &DiagnosticInvalidation, + ) -> Vec { + let config = self.config_state.config.diagnostics_config(); + if !config.enabled || !config.semantic.enabled { + return Vec::new(); + } + + let snapshot = self.make_snapshot(); + let profile_ids = match snapshot.compilation_profile_ids() { + Ok(profile_ids) => profile_ids, + Err(error) => { + tracing::debug!("skipping semantic compiler profile discovery: {error:#}"); + return Vec::new(); + } + }; + + match invalidation { + DiagnosticInvalidation::WorkspaceChanged => profile_ids, + DiagnosticInvalidation::FileChanges(changed_file_ids) => profile_ids + .into_iter() + .filter(|profile_id| { + snapshot.analysis.compilation_profile_file_ids(*profile_id).is_ok_and( + |profile_file_ids| { + profile_file_ids + .iter() + .any(|file_id| changed_file_ids.contains(file_id)) + }, + ) + }) + .collect(), + } + } + fn clear_deleted_push_diagnostics(&mut self, deleted_files: &[(FileId, VfsPath)]) { if deleted_files.is_empty() || self.config_state.config.cli_pull_diagnostics_support() { return; diff --git a/src/global_state/qihe.rs b/src/global_state/qihe.rs index a9ae3190..f2f14c79 100644 --- a/src/global_state/qihe.rs +++ b/src/global_state/qihe.rs @@ -679,6 +679,7 @@ pub(super) fn with_global_ctx( diagnostics, workspace, qihe, + semantic_compiler: _, external_sources, tasks, } = state; diff --git a/src/global_state/semantic_compiler.rs b/src/global_state/semantic_compiler.rs new file mode 100644 index 00000000..86af8c01 --- /dev/null +++ b/src/global_state/semantic_compiler.rs @@ -0,0 +1,537 @@ +use std::{ + panic::{self, AssertUnwindSafe}, + sync::Arc as StdArc, +}; + +use anyhow::Result; +use hir::base_db::project::CompilationProfileId; +use parking_lot::{Mutex, MutexGuard}; +use rustc_hash::{FxHashMap, FxHashSet}; +use triomphe::Arc; +use utils::{ + cancellation::{CancellationError, CancellationToken}, + thread::ThreadIntent, +}; +use vfs::FileId; + +use super::{ + AnalysisState, ClientState, ConfigState, DEFAULT_REQ_HANDLER, DiagnosticsState, GlobalState, + TaskState, WorkspaceState, + diagnostics::{ + DiagnosticCommitFreshness, DiagnosticExternalRevision, DiagnosticOwner, + DiagnosticPublishFreshness, DiagnosticSource, + publisher::{DiagnosticsPublisher, PublishDiagnosticsBatch, PublishDiagnosticsTask}, + }, + snapshot::GlobalStateSnapshot, + task::{SemanticCompilerTask, Task}, +}; +use crate::lsp_ext::to_proto; + +const SLANG_SEMANTIC: &str = "slang-semantic"; + +#[derive(Debug)] +pub(crate) struct SemanticCompilerUpdate { + by_file: FxHashMap>, + touched_files: FxHashSet, + freshness: DiagnosticCommitFreshness, +} + +impl SemanticCompilerUpdate { + pub(crate) fn touched_file_count(&self) -> usize { + self.touched_files.len() + } + + pub(crate) fn diagnostic_count(&self) -> usize { + self.by_file.values().map(Vec::len).sum() + } +} + +#[derive(Clone)] +pub(crate) struct SemanticDiagnostics { + states: Arc>>, +} + +impl SemanticDiagnostics { + pub(crate) fn new() -> Self { + Self { states: Arc::new(Mutex::new(FxHashMap::default())) } + } + + fn lock(&self) -> MutexGuard<'_, FxHashMap> { + self.states.lock() + } +} + +impl DiagnosticSource for SemanticDiagnostics { + fn diagnostics( + &self, + file_id: FileId, + freshness: &DiagnosticCommitFreshness, + ) -> Vec { + self.lock() + .get(&file_id) + .filter(|state| state.freshness == *freshness) + .map(|state| state.diagnostics.clone()) + .unwrap_or_default() + } + + fn external_revision( + &self, + file_id: FileId, + freshness: &DiagnosticCommitFreshness, + ) -> Option { + self.lock().get(&file_id).filter(|state| state.freshness == *freshness).map(|state| { + DiagnosticExternalRevision::new( + DiagnosticOwner::External { source: SLANG_SEMANTIC, file: file_id }, + state.generation, + ) + }) + } + + fn remove_deleted(&self, files: &FxHashSet) { + if files.is_empty() { + return; + } + + let mut diagnostics = self.lock(); + for file_id in files { + diagnostics.remove(file_id); + } + } +} + +#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Hash)] +pub(crate) struct SemanticCompilerRunId(u64); + +impl SemanticCompilerRunId { + fn next(self) -> Self { + Self(self.0.saturating_add(1)) + } +} + +pub(crate) struct SemanticCompiler { + run_generation: SemanticCompilerRunId, + active_cancel_token: Option, + pending_profiles: FxHashSet, + diagnostics: SemanticDiagnostics, +} + +impl SemanticCompiler { + pub(crate) fn new(diagnostics: SemanticDiagnostics) -> Self { + Self { + run_generation: SemanticCompilerRunId::default(), + active_cancel_token: None, + pending_profiles: FxHashSet::default(), + diagnostics, + } + } + + pub(crate) fn diagnostics_snapshot(&self) -> SemanticDiagnostics { + self.diagnostics.clone() + } + + pub(crate) fn schedule( + &mut self, + profile_ids: Vec, + ctx: &mut C, + ) { + let profile_ids = normalize_profile_ids(profile_ids); + if profile_ids.is_empty() { + return; + } + + if self.active_cancel_token.is_some() { + self.pending_profiles.extend(profile_ids); + return; + } + + self.start_run(profile_ids, ctx); + } + + pub(crate) fn handle( + &mut self, + task: SemanticCompilerTask, + ctx: &mut C, + ) { + match task { + SemanticCompilerTask::Finished { run_id, update } => { + if run_id != self.run_generation { + tracing::debug!( + ?run_id, + current = ?self.run_generation, + "stale semantic compiler result ignored" + ); + return; + } + + if self.active_cancel_token.as_ref().is_some_and(CancellationToken::is_cancelled) { + self.active_cancel_token = None; + self.start_pending(ctx); + return; + } + + self.active_cancel_token = None; + let current_freshness = ctx.diagnostic_commit_freshness(); + if update.freshness != current_freshness { + tracing::debug!( + ?run_id, + freshness = ?update.freshness, + current = ?current_freshness, + "stale semantic compiler diagnostics ignored" + ); + self.start_pending(ctx); + return; + } + + let changed_files = self.replace_diagnostics(update, current_freshness); + self.publish_diagnostics(changed_files, ctx); + self.start_pending(ctx); + } + SemanticCompilerTask::Cancelled { run_id } => { + if run_id != self.run_generation { + tracing::debug!( + ?run_id, + current = ?self.run_generation, + "stale semantic compiler cancellation ignored" + ); + return; + } + self.active_cancel_token = None; + self.start_pending(ctx); + } + SemanticCompilerTask::Failed { run_id, message } => { + if run_id != self.run_generation { + tracing::debug!( + ?run_id, + current = ?self.run_generation, + "stale semantic compiler failure ignored" + ); + return; + } + self.active_cancel_token = None; + tracing::warn!(message, "semantic compiler diagnostics failed"); + self.start_pending(ctx); + } + } + } + + fn start_run( + &mut self, + profile_ids: Vec, + ctx: &mut C, + ) { + self.run_generation = self.run_generation.next(); + let run_id = self.run_generation; + let cancellation = ctx.task_cancel_token(); + let snapshot = ctx.make_snapshot(cancellation.clone()); + self.active_cancel_token = Some(cancellation.clone()); + + ctx.spawn_semantic_compiler_task(move |sender| { + let task = Task::SemanticCompiler( + panic::catch_unwind(AssertUnwindSafe(|| { + run_semantic_compiler_task(snapshot, profile_ids, run_id, cancellation) + })) + .unwrap_or_else(|panic| { + let message = panic_message(&panic) + .map(|message| format!("semantic compiler panicked: {message}")) + .unwrap_or_else(|| "semantic compiler panicked".to_owned()); + SemanticCompilerTask::Failed { run_id, message } + }), + ); + if sender.send(task).is_err() { + tracing::debug!( + "semantic compiler result dropped because main loop receiver is closed" + ); + } + }); + } + + fn start_pending(&mut self, ctx: &mut C) { + let profile_ids = self.pending_profiles.drain().collect::>(); + let profile_ids = normalize_profile_ids(profile_ids); + if !profile_ids.is_empty() { + self.start_run(profile_ids, ctx); + } + } + + fn publish_diagnostics( + &mut self, + changed_files: FxHashSet, + ctx: &mut C, + ) { + ctx.publish_semantic_diagnostics(changed_files); + } + + fn replace_diagnostics( + &mut self, + update: SemanticCompilerUpdate, + freshness: DiagnosticCommitFreshness, + ) -> FxHashSet { + let SemanticCompilerUpdate { mut by_file, mut touched_files, freshness: _ } = update; + touched_files.extend(by_file.keys().copied()); + + let mut cache = self.diagnostics.lock(); + let mut changed_files = touched_files + .iter() + .filter_map(|file_id| { + cache + .get(file_id) + .is_some_and(|state| !state.diagnostics.is_empty()) + .then_some(*file_id) + }) + .collect::>(); + changed_files.extend(by_file.keys().copied()); + + for file_id in touched_files { + let diagnostics = by_file.remove(&file_id).unwrap_or_default(); + let generation = + cache.get(&file_id).map_or(1, |state| state.generation.saturating_add(1)); + cache.insert(file_id, SemanticDiagnosticState { freshness, generation, diagnostics }); + } + + changed_files + } +} + +#[derive(Debug, Clone, Default)] +struct SemanticDiagnosticState { + freshness: DiagnosticCommitFreshness, + generation: u64, + diagnostics: Vec, +} + +pub(crate) trait SemanticCompilerCtx { + fn diagnostic_commit_freshness(&self) -> DiagnosticCommitFreshness; + fn make_snapshot(&self, cancellation: CancellationToken) -> GlobalStateSnapshot; + fn spawn_semantic_compiler_task(&mut self, task: F) + where + F: FnOnce(crossbeam_channel::Sender) + Send + 'static; + fn task_cancel_token(&self) -> CancellationToken; + fn publish_semantic_diagnostics(&mut self, changed_files: FxHashSet); +} + +pub(super) struct SemanticCompilerGlobalCtx<'a> { + client: &'a mut ClientState, + config_state: &'a mut ConfigState, + analysis: &'a mut AnalysisState, + diagnostics: &'a mut DiagnosticsState, + workspace: &'a mut WorkspaceState, + external_sources: &'a [StdArc], + tasks: &'a mut TaskState, +} + +impl SemanticCompilerGlobalCtx<'_> { + fn diagnostic_publish_freshness(&self) -> DiagnosticPublishFreshness { + DiagnosticPublishFreshness::new( + self.diagnostics.diagnostics_revision, + self.diagnostics.diagnostic_target_revision, + self.workspace.workspace_vfs.diagnostic_readiness_revision(), + ) + } + + fn send(&self, message: lsp_server::Message) { + if self.client.sender.send(message).is_err() { + tracing::debug!("LSP message dropped because client connection is closed"); + } + } + + fn send_request(&mut self, params: R::Params) { + let request = self.client.req_queue.outgoing.register( + R::METHOD.to_string(), + params, + DEFAULT_REQ_HANDLER, + ); + self.send(request.into()); + } + + fn refresh_pull_diagnostics(&mut self, changed_files: FxHashSet) { + if changed_files.is_empty() { + return; + } + + if !self.workspace.workspace_vfs.is_ready() { + self.workspace.workspace_vfs.defer_diagnostics_until_ready(); + tracing::debug!( + ?changed_files, + "semantic diagnostics refresh deferred until workspace/VFS is ready" + ); + return; + } + + if self.config_state.config.cli_workspace_diagnostic_refresh_support() { + self.send_request::(()); + } + } +} + +impl SemanticCompilerCtx for SemanticCompilerGlobalCtx<'_> { + fn diagnostic_commit_freshness(&self) -> DiagnosticCommitFreshness { + self.diagnostic_publish_freshness().commit() + } + + fn make_snapshot(&self, cancellation: CancellationToken) -> GlobalStateSnapshot { + GlobalStateSnapshot { + config: Arc::clone(&self.config_state.config), + workspaces: Arc::clone(&self.workspace.workspaces), + analysis: self.analysis.analysis_host.make_analysis(), + vfs: Arc::clone(&self.workspace.vfs), + mem_docs: self.analysis.mem_docs.clone(), + sema_tokens_cache: Arc::clone(&self.analysis.semantic_tokens_cache), + external_sources: self.external_sources.to_vec(), + diagnostic_publish_freshness: self.diagnostic_publish_freshness(), + diagnostic_file_revisions: self.diagnostics.diagnostic_file_revisions.clone(), + cancellation, + accepted_response_effects: Default::default(), + } + } + + fn spawn_semantic_compiler_task(&mut self, task: F) + where + F: FnOnce(crossbeam_channel::Sender) + Send + 'static, + { + self.tasks.task_pool.handle.spawn_and_send_cps(ThreadIntent::Worker, task); + } + + fn task_cancel_token(&self) -> CancellationToken { + self.tasks.task_pool.handle.task_token() + } + + fn publish_semantic_diagnostics(&mut self, changed_files: FxHashSet) { + if changed_files.is_empty() { + return; + } + + if self.config_state.config.cli_pull_diagnostics_support() { + self.refresh_pull_diagnostics(changed_files); + return; + } + + let snapshot = self.make_snapshot(self.task_cancel_token()); + let mut publish_tasks = Vec::with_capacity(changed_files.len()); + let mut touched_file_ids = FxHashSet::default(); + for file_id in changed_files.iter().copied() { + let targets = match snapshot.diagnostic_publish_targets(file_id) { + Ok(targets) => targets, + Err(error) => { + tracing::debug!( + ?file_id, + "skipping semantic diagnostics for file without URI: {error:#}" + ); + continue; + } + }; + let diagnostics = match snapshot.lsp_diagnostics(file_id) { + Ok(diagnostics) => diagnostics, + Err(error) if error.is::() => { + tracing::debug!(?file_id, "semantic diagnostic publish cancelled"); + continue; + } + Err(error) => { + tracing::debug!(?file_id, "semantic diagnostic publish failed: {error:#}"); + continue; + } + }; + touched_file_ids.insert(file_id); + + publish_tasks.extend( + targets + .into_iter() + .map(|target| PublishDiagnosticsTask::from_target(target, diagnostics.clone())), + ); + } + let current_freshness = self.diagnostic_publish_freshness(); + DiagnosticsPublisher::new( + &self.config_state.config, + &mut self.workspace.workspace_vfs, + &mut self.diagnostics.published_diagnostics, + &self.client.sender, + current_freshness, + ) + .publish(PublishDiagnosticsBatch::for_touched_files( + touched_file_ids, + publish_tasks, + snapshot.diagnostic_publish_freshness, + )); + } +} + +pub(super) fn with_global_ctx( + state: &mut GlobalState, + f: impl FnOnce(&mut SemanticCompiler, &mut SemanticCompilerGlobalCtx<'_>) -> T, +) -> T { + let GlobalState { + client, + config_state, + analysis, + diagnostics, + workspace, + qihe: _, + semantic_compiler, + external_sources, + tasks, + } = state; + let mut ctx = SemanticCompilerGlobalCtx { + client, + config_state, + analysis, + diagnostics, + workspace, + external_sources, + tasks, + }; + f(semantic_compiler, &mut ctx) +} + +fn run_semantic_compiler_task( + snapshot: GlobalStateSnapshot, + profile_ids: Vec, + run_id: SemanticCompilerRunId, + cancellation: CancellationToken, +) -> SemanticCompilerTask { + match collect_semantic_diagnostics(&snapshot, profile_ids, &cancellation) { + Ok(update) => SemanticCompilerTask::Finished { run_id, update }, + Err(err) if err.is::() => SemanticCompilerTask::Cancelled { run_id }, + Err(err) => SemanticCompilerTask::Failed { run_id, message: err.to_string() }, + } +} + +fn collect_semantic_diagnostics( + snapshot: &GlobalStateSnapshot, + profile_ids: Vec, + cancellation: &CancellationToken, +) -> Result { + let freshness = snapshot.diagnostic_commit_freshness(); + let mut by_file = FxHashMap::default(); + let mut touched_files = FxHashSet::default(); + + for profile_id in profile_ids { + cancellation.check()?; + touched_files.extend(snapshot.analysis.compilation_profile_file_ids(profile_id)?); + let diagnostics = snapshot.analysis.compilation_profile_diagnostics(profile_id)?; + for diagnostic in diagnostics { + cancellation.check()?; + if diagnostic.source != ide::diagnostics::DiagnosticSource::SlangSemantic { + continue; + } + let file_id = diagnostic.file_id; + let line_info = snapshot.line_info(file_id)?; + let diagnostic = to_proto::diagnostic(snapshot.config.i18n, &line_info, diagnostic); + by_file.entry(file_id).or_insert_with(Vec::new).push(diagnostic); + } + } + cancellation.check()?; + + Ok(SemanticCompilerUpdate { by_file, touched_files, freshness }) +} + +fn normalize_profile_ids(mut profile_ids: Vec) -> Vec { + profile_ids.sort_unstable_by_key(|profile_id| profile_id.0); + profile_ids.dedup(); + profile_ids +} + +fn panic_message(panic: &(dyn std::any::Any + Send)) -> Option<&str> { + panic + .downcast_ref::() + .map(String::as_str) + .or_else(|| panic.downcast_ref::<&str>().copied()) +} diff --git a/src/global_state/snapshot.rs b/src/global_state/snapshot.rs index 14d61730..0ef5f9bc 100644 --- a/src/global_state/snapshot.rs +++ b/src/global_state/snapshot.rs @@ -360,11 +360,7 @@ impl GlobalStateSnapshot { ) -> Cancellable> { match producer.owner() { DiagnosticOwner::CompilationProfile(profile_id) => { - if self.config.diagnostics_config().semantic.enabled { - self.analysis.compilation_profile_diagnostics(profile_id) - } else { - self.analysis.compilation_profile_syntax_diagnostics(profile_id) - } + self.analysis.compilation_profile_syntax_diagnostics(profile_id) } DiagnosticOwner::SourceRoot(_) => { self.analysis.source_root_diagnostics(producer.representative_file_id()) @@ -374,6 +370,12 @@ impl GlobalStateSnapshot { } } + pub(crate) fn compilation_profile_ids( + &self, + ) -> Cancellable> { + self.analysis.compilation_profile_ids() + } + pub(crate) fn diagnostic_target_file_ids_for_changes( &self, changed_file_ids: &FxHashSet, diff --git a/src/global_state/task.rs b/src/global_state/task.rs index b2f0ea4b..20fbf9f1 100644 --- a/src/global_state/task.rs +++ b/src/global_state/task.rs @@ -14,6 +14,7 @@ use super::{ qihe::{QiheRunId, QiheUpdate}, reload::FetchWorkspaceProgress, response_effect::AcceptedResponseEffect, + semantic_compiler::{SemanticCompilerRunId, SemanticCompilerUpdate}, }; #[derive(Debug)] @@ -23,6 +24,7 @@ pub(crate) enum Task { FetchWorkspace(FetchWorkspaceProgress), Diagnostics(PublishDiagnosticsBatch), Qihe(QiheTask), + SemanticCompiler(SemanticCompilerTask), } impl Task { @@ -40,6 +42,7 @@ impl Task { Task::FetchWorkspace(FetchWorkspaceProgress::End { .. }) => "task.fetch_workspace.end", Task::Diagnostics(_) => "task.diagnostics", Task::Qihe(task) => task.kind(), + Task::SemanticCompiler(task) => task.kind(), } } @@ -65,6 +68,7 @@ impl Task { ) } Task::Qihe(task) => task.summary(), + Task::SemanticCompiler(task) => task.summary(), } } } @@ -257,3 +261,38 @@ impl QiheTask { } } } + +#[derive(Debug)] +pub(crate) enum SemanticCompilerTask { + Finished { run_id: SemanticCompilerRunId, update: SemanticCompilerUpdate }, + Cancelled { run_id: SemanticCompilerRunId }, + Failed { run_id: SemanticCompilerRunId, message: String }, +} + +impl SemanticCompilerTask { + pub(super) fn kind(&self) -> &'static str { + match self { + SemanticCompilerTask::Finished { .. } => "task.semantic_compiler.finished", + SemanticCompilerTask::Cancelled { .. } => "task.semantic_compiler.cancelled", + SemanticCompilerTask::Failed { .. } => "task.semantic_compiler.failed", + } + } + + pub(super) fn summary(&self) -> String { + match self { + SemanticCompilerTask::Finished { run_id, update } => { + format!( + "task semantic compiler finished run={run_id:?} files={} diagnostics={}", + update.touched_file_count(), + update.diagnostic_count() + ) + } + SemanticCompilerTask::Cancelled { run_id } => { + format!("task semantic compiler cancelled run={run_id:?}") + } + SemanticCompilerTask::Failed { run_id, message } => { + format!("task semantic compiler failed run={run_id:?} message={message}") + } + } + } +} diff --git a/src/tests.rs b/src/tests.rs index d9224fcd..744f0b33 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -277,6 +277,14 @@ fn shutdown_test_server( if notification.method == lsp_types::notification::PublishDiagnostics::METHOD => {} Message::Notification(notification) if notification.method == ProjectStatusNotification::METHOD => {} + Message::Request(request) + if request.method == lsp_types::request::WorkspaceDiagnosticRefresh::METHOD => + { + client + .sender + .send(Message::Response(lsp_server::Response::new_ok(request.id, ()))) + .unwrap(); + } other => panic!("unexpected message while shutting down test server: {other:?}"), } } @@ -337,6 +345,25 @@ fn request_document_diagnostics( request_document_diagnostics_with_previous_result_id(client, uri, request_id, None) } +fn request_document_diagnostics_until( + client: &Connection, + uri: Url, + first_request_id: i32, + mut predicate: impl FnMut(Option<&str>, &[lsp_types::Diagnostic]) -> bool, + context: &str, +) -> (Option, Vec) { + for attempt in 0..50 { + let (result_id, diagnostics) = + request_document_diagnostics(client, uri.clone(), first_request_id + attempt); + if predicate(result_id.as_deref(), &diagnostics) { + return (result_id, diagnostics); + } + wait_for_workspace_diagnostic_refresh_or_tick(client, context); + } + + panic!("document diagnostics did not satisfy predicate during {context}"); +} + fn request_document_diagnostics_with_previous_result_id( client: &Connection, uri: Url, @@ -728,6 +755,47 @@ fn request_workspace_diagnostic_report( report } +fn request_workspace_diagnostic_report_until( + client: &Connection, + first_request_id: i32, + mut predicate: impl FnMut(&lsp_types::WorkspaceDiagnosticReport) -> bool, + context: &str, +) -> lsp_types::WorkspaceDiagnosticReport { + for attempt in 0..50 { + let report = + request_workspace_diagnostic_report(client, first_request_id + attempt, Vec::new()); + if predicate(&report) { + return report; + } + wait_for_workspace_diagnostic_refresh_or_tick(client, context); + } + + panic!("workspace diagnostics did not satisfy predicate during {context}"); +} + +fn wait_for_workspace_diagnostic_refresh_or_tick(client: &Connection, context: &str) { + let deadline = Instant::now() + Duration::from_millis(100); + while let Some(message) = recv_lsp_message_until(client, deadline, context) { + match message { + Message::Request(request) + if request.method == lsp_types::request::WorkspaceDiagnosticRefresh::METHOD => + { + client + .sender + .send(Message::Response(lsp_server::Response::new_ok(request.id, ()))) + .unwrap(); + return; + } + Message::Request(request) => handle_test_server_request(client, request, context), + Message::Notification(notification) + if notification.method == lsp_types::notification::Progress::METHOD => {} + Message::Notification(notification) + if notification.method == lsp_types::notification::PublishDiagnostics::METHOD => {} + _ => {} + } + } +} + fn request_workspace_diagnostic_uris(client: &Connection, request_id: i32) -> Vec { request_workspace_diagnostic_report(client, request_id, Vec::new()) .items diff --git a/src/tests/code_actions.rs b/src/tests/code_actions.rs index f3ec21c3..3322df41 100644 --- a/src/tests/code_actions.rs +++ b/src/tests/code_actions.rs @@ -137,23 +137,13 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_configured_diagnostics_test(code_action_client_caps(), UserConfig::default(), text); - let diagnostics_id = lsp_server::RequestId::from(210); - client - .sender - .send(Message::Request(Request::new( - diagnostics_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri: uri.clone() }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let (_result_id, mut diagnostics) = recv_document_diagnostics(&client, diagnostics_id); - assert!(!diagnostics.is_empty(), "expected mixed connection diagnostic"); + let (_result_id, mut diagnostics) = request_document_diagnostics_until( + &client, + uri.clone(), + 210, + |_result_id, diagnostics| !diagnostics.is_empty(), + "semantic diagnostics for code action", + ); for diagnostic in &mut diagnostics { diagnostic.data = None; } diff --git a/src/tests/diagnostics.rs b/src/tests/diagnostics.rs index 90fe7f4c..6e8045fe 100644 --- a/src/tests/diagnostics.rs +++ b/src/tests/diagnostics.rs @@ -17,7 +17,17 @@ endmodule text, ); - let (_result_id, diagnostics) = request_document_diagnostics(&client, uri, 190); + let (_result_id, diagnostics) = request_document_diagnostics_until( + &client, + uri, + 190, + |_result_id, diagnostics| { + diagnostics + .iter() + .any(|diagnostic| diagnostic_option(diagnostic) == Some("port-width-trunc")) + }, + "default semantic diagnostics", + ); assert!( diagnostics @@ -558,92 +568,69 @@ fn workspace_diagnostics_use_multi_file_semantic_context() { let unused_uri = uris[1].clone(); let top_uri = uris[2].clone(); - let request_id = lsp_server::RequestId::from(1); - let request = Request::new( - request_id.clone(), - WorkspaceDiagnosticRequest::METHOD.to_string(), - WorkspaceDiagnosticParams { - identifier: None, - previous_result_ids: Vec::new(), - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ); - client.sender.send(Message::Request(request)).unwrap(); - - let deadline = Instant::now() + LSP_TEST_TIMEOUT; - while Instant::now() < deadline { - let timeout = deadline.saturating_duration_since(Instant::now()); - match client.receiver.recv_timeout(timeout).unwrap() { - Message::Response(response) if response.id == request_id => { - assert!(response.error.is_none(), "{:?}", response.error); - let result = serde_json::from_value::( - response.result.unwrap(), - ) - .unwrap(); - let report = match result { - WorkspaceDiagnosticReportResult::Report(report) => report, - other => panic!("unexpected workspace diagnostic response: {other:?}"), + let report = request_workspace_diagnostic_report_until( + &client, + 1, + |report| { + report.items.iter().any(|item| { + let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item else { + return false; }; - let mut child_diagnostics = None; - let mut unused_diagnostics = None; - let mut top_diagnostics = None; - for item in report.items { - if let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item { - if full.uri == child_uri { - child_diagnostics = Some(full.full_document_diagnostic_report.items); - } else if full.uri == unused_uri { - unused_diagnostics = Some(full.full_document_diagnostic_report.items); - } else if full.uri == top_uri { - top_diagnostics = Some(full.full_document_diagnostic_report.items); - } - } - } - - let child_diagnostics = child_diagnostics.expect("missing child diagnostics"); - let unused_diagnostics = unused_diagnostics.expect("missing unused diagnostics"); - let top_diagnostics = top_diagnostics.expect("missing top diagnostics"); - assert!( - child_diagnostics.is_empty(), - "child.sv should not receive top.sv diagnostics: {child_diagnostics:?}" - ); - assert!( - unused_diagnostics.is_empty(), - "unused.sv should not receive top.sv diagnostics: {unused_diagnostics:?}" - ); - assert!( - top_diagnostics - .iter() - .any(|diag| diag.message.contains("port 'b' has no connection")), - "top.sv should receive semantic diagnostic using child.sv: {top_diagnostics:?}" - ); - assert_eq!( - top_diagnostics - .iter() - .filter(|diag| diag.message.contains("port 'b' has no connection")) - .count(), - 1, - "workspace diagnostics should not duplicate source-root diagnostics: {top_diagnostics:?}" - ); - assert!( - !top_diagnostics + full.uri == top_uri + && full + .full_document_diagnostic_report + .items .iter() - .any(|diag| diag.message.contains("unknown module 'child'")), - "top.sv should resolve child module from child.sv: {top_diagnostics:?}" - ); - shutdown_test_server(&client, server_thread); - return; - } - Message::Notification(notification) - if notification.method == lsp_types::notification::Progress::METHOD => {} - Message::Request(request) => { - panic!("unexpected server request during diagnostics test: {request:?}"); + .any(|diag| diag.message.contains("port 'b' has no connection")) + }) + }, + "multi-file semantic workspace diagnostics", + ); + + let mut child_diagnostics = None; + let mut unused_diagnostics = None; + let mut top_diagnostics = None; + for item in report.items { + if let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item { + if full.uri == child_uri { + child_diagnostics = Some(full.full_document_diagnostic_report.items); + } else if full.uri == unused_uri { + unused_diagnostics = Some(full.full_document_diagnostic_report.items); + } else if full.uri == top_uri { + top_diagnostics = Some(full.full_document_diagnostic_report.items); } - _ => {} } } - panic!("workspaceDiagnostic response not received"); + let child_diagnostics = child_diagnostics.expect("missing child diagnostics"); + let unused_diagnostics = unused_diagnostics.expect("missing unused diagnostics"); + let top_diagnostics = top_diagnostics.expect("missing top diagnostics"); + assert!( + child_diagnostics.is_empty(), + "child.sv should not receive top.sv diagnostics: {child_diagnostics:?}" + ); + assert!( + unused_diagnostics.is_empty(), + "unused.sv should not receive top.sv diagnostics: {unused_diagnostics:?}" + ); + assert!( + top_diagnostics.iter().any(|diag| diag.message.contains("port 'b' has no connection")), + "top.sv should receive semantic diagnostic using child.sv: {top_diagnostics:?}" + ); + assert_eq!( + top_diagnostics + .iter() + .filter(|diag| diag.message.contains("port 'b' has no connection")) + .count(), + 1, + "workspace diagnostics should not duplicate source-root diagnostics: {top_diagnostics:?}" + ); + assert!( + !top_diagnostics.iter().any(|diag| diag.message.contains("unknown module 'child'")), + "top.sv should resolve child module from child.sv: {top_diagnostics:?}" + ); + + shutdown_test_server(&client, server_thread); } #[test] @@ -692,7 +679,28 @@ fn workspace_diagnostics_compute_profile_owner_once_across_source_roots() { let server_thread = spawn_default_test_server(config, server); let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); - let report = request_workspace_diagnostic_report(&client, 1, Vec::new()); + let report = request_workspace_diagnostic_report_until( + &client, + 1, + |report| { + report + .items + .iter() + .filter_map(|item| match item { + lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) + if full.uri == top_uri => + { + Some(&full.full_document_diagnostic_report.items) + } + _ => None, + }) + .flatten() + .filter(|diag| diag.message.contains("port 'b' has no connection")) + .count() + == 1 + }, + "profile semantic workspace diagnostics", + ); let missing_port_count = report .items .into_iter() @@ -1203,109 +1211,48 @@ fn workspace_scan_refreshes_diagnostics_for_unopened_systemverilog_dependency() ))) .unwrap(); - let deadline = Instant::now() + LSP_TEST_TIMEOUT; - while Instant::now() < deadline { - let timeout = deadline.saturating_duration_since(Instant::now()); - match client.receiver.recv_timeout(timeout).unwrap() { - Message::Request(request) - if request.method == lsp_types::request::WorkspaceDiagnosticRefresh::METHOD => - { - client - .sender - .send(Message::Response(lsp_server::Response::new_ok(request.id, ()))) - .unwrap(); - break; - } - Message::Request(request) - if request.method == lsp_types::request::WorkDoneProgressCreate::METHOD => - { - client - .sender - .send(Message::Response(lsp_server::Response::new_ok(request.id, ()))) - .unwrap(); - } - Message::Notification(notification) - if notification.method == lsp_types::notification::Progress::METHOD => {} - Message::Request(request) => { - panic!( - "unexpected server request while waiting for diagnostic refresh: {request:?}" - ); - } - _ => {} - } - } - - let request_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - request_id.clone(), - WorkspaceDiagnosticRequest::METHOD.to_string(), - WorkspaceDiagnosticParams { - identifier: None, - previous_result_ids: Vec::new(), - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - - let deadline = Instant::now() + LSP_TEST_TIMEOUT; - while Instant::now() < deadline { - let timeout = deadline.saturating_duration_since(Instant::now()); - match client.receiver.recv_timeout(timeout).unwrap() { - Message::Response(response) if response.id == request_id => { - assert!(response.error.is_none(), "{:?}", response.error); - let result = serde_json::from_value::( - response.result.unwrap(), - ) - .unwrap(); - let report = match result { - WorkspaceDiagnosticReportResult::Report(report) => report, - other => panic!("unexpected workspace diagnostic response: {other:?}"), + let report = request_workspace_diagnostic_report_until( + &client, + 1, + |report| { + report.items.iter().any(|item| { + let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item else { + return false; }; - let mut top_diagnostics = None; - for item in report.items { - if let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item - && full.uri == top_uri - { - top_diagnostics = Some(full.full_document_diagnostic_report.items); - } - } - let top_diagnostics = top_diagnostics.expect("missing top diagnostics"); - assert!( - !top_diagnostics + full.uri == top_uri + && !full + .full_document_diagnostic_report + .items .iter() - .any(|diag| diag.message.contains("unknown module 'child'")), - "top.v should resolve child module from unopened child.sv: {top_diagnostics:?}" - ); - assert!( - top_diagnostics + .any(|diag| diag.message.contains("unknown module 'child'")) + && full + .full_document_diagnostic_report + .items .iter() - .any(|diag| diag.message.contains("port 'b' has no connection")), - "top.v should use unopened child.sv module definition: {top_diagnostics:?}" - ); - shutdown_test_server(&client, server_thread); - return; - } - Message::Notification(notification) - if notification.method == lsp_types::notification::Progress::METHOD => {} - Message::Request(request) - if request.method == lsp_types::request::WorkspaceDiagnosticRefresh::METHOD => - { - client - .sender - .send(Message::Response(lsp_server::Response::new_ok(request.id, ()))) - .unwrap(); - } - Message::Request(request) => { - panic!("unexpected server request during workspace diagnostics: {request:?}"); + .any(|diag| diag.message.contains("port 'b' has no connection")) + }) + }, + "workspace semantic diagnostics for unopened dependency", + ); + let top_diagnostics = report + .items + .into_iter() + .find_map(|item| match item { + lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) if full.uri == top_uri => { + Some(full.full_document_diagnostic_report.items) } - _ => {} - } - } - - panic!("workspaceDiagnostic response not received"); + _ => None, + }) + .expect("missing top diagnostics"); + assert!( + !top_diagnostics.iter().any(|diag| diag.message.contains("unknown module 'child'")), + "top.v should resolve child module from unopened child.sv: {top_diagnostics:?}" + ); + assert!( + top_diagnostics.iter().any(|diag| diag.message.contains("port 'b' has no connection")), + "top.v should use unopened child.sv module definition: {top_diagnostics:?}" + ); + shutdown_test_server(&client, server_thread); } #[test] @@ -1459,7 +1406,24 @@ fn watched_dependency_change_refreshes_workspace_diagnostics() { let child_uri = to_proto::url_from_abs_path(child_path.as_path()).unwrap(); let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); - let first_report = request_workspace_diagnostic_report(&client, 1, Vec::new()); + let first_report = request_workspace_diagnostic_report_until( + &client, + 1, + |report| { + report.items.iter().any(|item| { + let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item else { + return false; + }; + full.uri == top_uri + && full + .full_document_diagnostic_report + .items + .iter() + .any(|diag| diag.message.contains("port 'b' has no connection")) + }) + }, + "initial watched dependency semantic diagnostics", + ); let mut saw_missing_port = false; for item in first_report.items { if let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item @@ -1549,7 +1513,16 @@ fn document_diagnostic_result_id_tracks_diagnostics_config_revision() { "module top;\nendmodule\n", ); - let (first_result_id, first_items) = request_document_diagnostics(&client, uri.clone(), 1); + let (first_result_id, first_items) = request_document_diagnostics_until( + &client, + uri.clone(), + 1, + |result_id, items| { + items.is_empty() + && result_id.is_some_and(|result_id| result_id.contains("external-slang-semantic")) + }, + "initial empty semantic diagnostics result id", + ); let first_result_id = first_result_id.expect("expected first diagnostic result id"); assert!( first_items.is_empty(), @@ -1559,7 +1532,7 @@ fn document_diagnostic_result_id_tracks_diagnostics_config_revision() { let (second_result_id, second_items) = request_document_diagnostics_with_previous_result_id( &client, uri.clone(), - 2, + 9, Some(first_result_id.clone()), ); assert_eq!( @@ -1585,7 +1558,7 @@ fn document_diagnostic_result_id_tracks_diagnostics_config_revision() { request_document_diagnostics_with_previous_result_id( &client, uri.clone(), - 3, + 10, Some(first_result_id.clone()), ); assert_eq!( @@ -1610,7 +1583,7 @@ fn document_diagnostic_result_id_tracks_diagnostics_config_revision() { let (third_result_id, _third_items) = request_document_diagnostics_with_previous_result_id( &client, uri, - 4, + 11, Some(first_result_id), ); assert_ne!( @@ -1642,22 +1615,15 @@ fn document_diagnostic_result_id_changes_when_dependency_changes() { let child_uri = uris[0].clone(); let top_uri = uris[1].clone(); - let first_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - first_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri: top_uri.clone() }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let (first_result_id, first_items) = recv_document_diagnostics(&client, first_id); + let (first_result_id, first_items) = request_document_diagnostics_until( + &client, + top_uri.clone(), + 1, + |_result_id, items| { + items.iter().any(|diag| diag.message.contains("port 'b' has no connection")) + }, + "initial dependency semantic diagnostics", + ); let first_result_id = first_result_id.expect("expected first diagnostic result id"); assert!(!first_result_id.is_empty(), "diagnostic result id should include open file versions"); assert!( @@ -1680,7 +1646,7 @@ fn document_diagnostic_result_id_changes_when_dependency_changes() { ))) .unwrap(); - let second_id = lsp_server::RequestId::from(2); + let second_id = lsp_server::RequestId::from(9); client .sender .send(Message::Request(Request::new( From 7c9cf81dd91134eb3255e6a624a4098df084aeac Mon Sep 17 00:00:00 2001 From: hongjr03 Date: Sat, 27 Jun 2026 02:09:59 +0800 Subject: [PATCH 2/2] refactor: store semantic diagnostics in domain form --- src/global_state/diagnostics.rs | 17 +++- .../handlers/request/code_action.rs | 86 +------------------ .../handlers/request/diagnostics.rs | 5 +- src/global_state/qihe.rs | 2 +- src/global_state/qihe/tests.rs | 4 +- src/global_state/semantic_compiler.rs | 9 +- src/global_state/snapshot.rs | 26 +++++- 7 files changed, 49 insertions(+), 100 deletions(-) diff --git a/src/global_state/diagnostics.rs b/src/global_state/diagnostics.rs index 95079732..92de4ef1 100644 --- a/src/global_state/diagnostics.rs +++ b/src/global_state/diagnostics.rs @@ -22,11 +22,26 @@ impl DiagnosticCommitFreshness { } pub(crate) trait DiagnosticSource: Send + Sync { + /// Domain diagnostics consumed by IDE features before protocol conversion. fn diagnostics( &self, file_id: FileId, freshness: &DiagnosticCommitFreshness, - ) -> Vec; + ) -> Vec { + let _ = (file_id, freshness); + Vec::new() + } + + /// Protocol-native diagnostics for sources that are produced outside the + /// IDE model. + fn lsp_diagnostics( + &self, + file_id: FileId, + freshness: &DiagnosticCommitFreshness, + ) -> Vec { + let _ = (file_id, freshness); + Vec::new() + } fn external_revision( &self, diff --git a/src/global_state/handlers/request/code_action.rs b/src/global_state/handlers/request/code_action.rs index 869cf3a8..40ebe450 100644 --- a/src/global_state/handlers/request/code_action.rs +++ b/src/global_state/handlers/request/code_action.rs @@ -6,8 +6,6 @@ use ide::{ }, diagnostics as ide_diagnostics, }; -use serde::Deserialize; -use syntax::DiagnosticSeverity; use utils::text_edit::TextRange; use vfs::FileId; @@ -95,7 +93,7 @@ fn server_diagnostics_for_code_action( line_info: &utils::lines::LineInfo, ) -> anyhow::Result> { let mut server_diagnostics = snap.diagnostics(file_id)?; - server_diagnostics.extend(cached_external_diagnostics(snap, file_id, line_info)); + server_diagnostics.extend(snap.external_diagnostics(file_id)); let client_locators = client_diagnostics .iter() .filter_map(|diag| DiagnosticLocator::from_lsp(line_info, diag)) @@ -119,88 +117,6 @@ fn server_diagnostics_for_code_action( Ok(diagnostics) } -fn cached_external_diagnostics( - snap: &GlobalStateSnapshot, - file_id: FileId, - line_info: &utils::lines::LineInfo, -) -> Vec { - let freshness = snap.diagnostic_commit_freshness(); - snap.external_sources - .iter() - .flat_map(|source| source.diagnostics(file_id, &freshness)) - .filter_map(|diag| cached_lsp_diagnostic_to_ide(file_id, line_info, diag)) - .collect() -} - -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct CachedDiagnosticData { - source: String, - subsystem: u16, - code: u16, - name: String, - #[serde(rename = "option")] - option_name: Option, - #[serde(default)] - groups: Vec, - #[serde(default)] - args: Vec, -} - -fn cached_lsp_diagnostic_to_ide( - file_id: FileId, - line_info: &utils::lines::LineInfo, - diag: lsp_types::Diagnostic, -) -> Option { - let data = serde_json::from_value::(diag.data.clone()?).ok()?; - let source = match data.source.as_str() { - "parse" => ide_diagnostics::DiagnosticSource::SlangParse, - "semantic" => ide_diagnostics::DiagnosticSource::SlangSemantic, - _ => return None, - }; - Some(ide_diagnostics::Diagnostic { - file_id, - code: data.code, - subsystem: data.subsystem, - name: data.name, - option_name: data.option_name, - groups: data.groups, - source, - range: from_proto::text_range(line_info, diag.range).ok()?, - severity: lsp_severity_to_ide(diag.severity), - message: diag.message, - args: data.args, - message_key: None, - message_args: Vec::new(), - tags: lsp_tags_to_ide(diag.tags), - }) -} - -fn lsp_severity_to_ide(severity: Option) -> DiagnosticSeverity { - match severity { - Some(lsp_types::DiagnosticSeverity::ERROR) => DiagnosticSeverity::Error, - Some(lsp_types::DiagnosticSeverity::WARNING) => DiagnosticSeverity::Warning, - Some(lsp_types::DiagnosticSeverity::INFORMATION | lsp_types::DiagnosticSeverity::HINT) => { - DiagnosticSeverity::Note - } - _ => DiagnosticSeverity::Error, - } -} - -fn lsp_tags_to_ide( - tags: Option>, -) -> Vec { - tags.unwrap_or_default() - .into_iter() - .filter_map(|tag| match tag { - lsp_types::DiagnosticTag::UNNECESSARY => { - Some(ide_diagnostics::DiagnosticTag::Unnecessary) - } - _ => None, - }) - .collect() -} - #[derive(Debug, Clone, PartialEq, Eq)] struct DiagnosticLocator { range: TextRange, diff --git a/src/global_state/handlers/request/diagnostics.rs b/src/global_state/handlers/request/diagnostics.rs index b938a142..e028f16b 100644 --- a/src/global_state/handlers/request/diagnostics.rs +++ b/src/global_state/handlers/request/diagnostics.rs @@ -67,10 +67,7 @@ pub(crate) fn handle_workspace_diagnostic( .into_iter() .map(|diag| to_proto::diagnostic(snap.config.i18n, &line_info, diag)) .collect::>(); - let freshness = snap.diagnostic_commit_freshness(); - diag_items.extend( - snap.external_sources.iter().flat_map(|source| source.diagnostics(file_id, &freshness)), - ); + diag_items.extend(snap.external_lsp_diagnostics(file_id)?); for target in targets { let uri = target.uri().clone(); diff --git a/src/global_state/qihe.rs b/src/global_state/qihe.rs index f2f14c79..e4fffab6 100644 --- a/src/global_state/qihe.rs +++ b/src/global_state/qihe.rs @@ -88,7 +88,7 @@ impl QiheDiagnostics { } impl DiagnosticSource for QiheDiagnostics { - fn diagnostics( + fn lsp_diagnostics( &self, file_id: FileId, freshness: &DiagnosticCommitFreshness, diff --git a/src/global_state/qihe/tests.rs b/src/global_state/qihe/tests.rs index 77044c65..cbab0585 100644 --- a/src/global_state/qihe/tests.rs +++ b/src/global_state/qihe/tests.rs @@ -314,7 +314,7 @@ fn qihe_diagnostics_are_scoped_to_diagnostic_commit_freshness() { let diagnostics = snapshot .external_sources .iter() - .flat_map(|source| source.diagnostics(file_id, &freshness)) + .flat_map(|source| source.lsp_diagnostics(file_id, &freshness)) .collect::>(); assert_eq!(diagnostics, vec![diagnostic]); @@ -325,7 +325,7 @@ fn qihe_diagnostics_are_scoped_to_diagnostic_commit_freshness() { snapshot .external_sources .iter() - .flat_map(|source| source.diagnostics(file_id, &freshness)) + .flat_map(|source| source.lsp_diagnostics(file_id, &freshness)) .collect::>() .is_empty() ); diff --git a/src/global_state/semantic_compiler.rs b/src/global_state/semantic_compiler.rs index 86af8c01..071655c2 100644 --- a/src/global_state/semantic_compiler.rs +++ b/src/global_state/semantic_compiler.rs @@ -25,13 +25,12 @@ use super::{ snapshot::GlobalStateSnapshot, task::{SemanticCompilerTask, Task}, }; -use crate::lsp_ext::to_proto; const SLANG_SEMANTIC: &str = "slang-semantic"; #[derive(Debug)] pub(crate) struct SemanticCompilerUpdate { - by_file: FxHashMap>, + by_file: FxHashMap>, touched_files: FxHashSet, freshness: DiagnosticCommitFreshness, } @@ -66,7 +65,7 @@ impl DiagnosticSource for SemanticDiagnostics { &self, file_id: FileId, freshness: &DiagnosticCommitFreshness, - ) -> Vec { + ) -> Vec { self.lock() .get(&file_id) .filter(|state| state.freshness == *freshness) @@ -296,7 +295,7 @@ impl SemanticCompiler { struct SemanticDiagnosticState { freshness: DiagnosticCommitFreshness, generation: u64, - diagnostics: Vec, + diagnostics: Vec, } pub(crate) trait SemanticCompilerCtx { @@ -513,8 +512,6 @@ fn collect_semantic_diagnostics( continue; } let file_id = diagnostic.file_id; - let line_info = snapshot.line_info(file_id)?; - let diagnostic = to_proto::diagnostic(snapshot.config.i18n, &line_info, diagnostic); by_file.entry(file_id).or_insert_with(Vec::new).push(diagnostic); } } diff --git a/src/global_state/snapshot.rs b/src/global_state/snapshot.rs index 0ef5f9bc..020d4492 100644 --- a/src/global_state/snapshot.rs +++ b/src/global_state/snapshot.rs @@ -160,9 +160,33 @@ impl GlobalStateSnapshot { .into_iter() .map(|diag| crate::lsp_ext::to_proto::diagnostic(self.config.i18n, &line_info, diag)) .collect::>(); + diagnostics.extend(self.external_lsp_diagnostics(file_id)?); + Ok(diagnostics) + } + + pub(crate) fn external_diagnostics( + &self, + file_id: FileId, + ) -> Vec { + let freshness = self.diagnostic_commit_freshness(); + self.external_sources + .iter() + .flat_map(|source| source.diagnostics(file_id, &freshness)) + .collect() + } + + pub(crate) fn external_lsp_diagnostics( + &self, + file_id: FileId, + ) -> anyhow::Result> { let freshness = self.diagnostic_commit_freshness(); + let mut diagnostics = Vec::new(); for source in &self.external_sources { - diagnostics.extend(source.diagnostics(file_id, &freshness)); + for diagnostic in source.diagnostics(file_id, &freshness) { + let line_info = self.line_info(diagnostic.file_id)?; + diagnostics.push(to_proto::diagnostic(self.config.i18n, &line_info, diagnostic)); + } + diagnostics.extend(source.lsp_diagnostics(file_id, &freshness)); } Ok(diagnostics) }