Mirrors/rust-analyzer
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer synced 2024-12-27 05:23:24 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #18729 from Veykril/push-kyxtnozqzwkn

Browse source 
Clear flycheck diagnostics more granularly
This commit is contained in:
Lukas Wirth 2024-12-20 13:23:31 +00:00 committed by GitHub
commit f4cafbb678
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 140 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
crates/rust-analyzer/src/diagnostics.rs
View file

@ -3,6 +3,7 @@ pub(crate) mod to_proto;
use std::mem;
use cargo_metadata::PackageId;
use ide::FileId;
use ide_db::FxHashMap;
use itertools::Itertools;
@ -13,7 +14,8 @@ use triomphe::Arc;
use crate::{global_state::GlobalStateSnapshot, lsp, lsp_ext, main_loop::DiagnosticsTaskKind};
pub(crate) type CheckFixes = Arc<IntMap<usize, IntMap<FileId, Vec<Fix>>>>;
pub(crate) type CheckFixes =
Arc<IntMap<usize, FxHashMap<Option<Arc<PackageId>>, IntMap<FileId, Vec<Fix>>>>>;
#[derive(Debug, Default, Clone)]
pub struct DiagnosticsMapConfig {
@ -31,7 +33,10 @@ pub(crate) struct DiagnosticCollection {
pub(crate) native_syntax: IntMap<FileId, (DiagnosticsGeneration, Vec<lsp_types::Diagnostic>)>,
pub(crate) native_semantic: IntMap<FileId, (DiagnosticsGeneration, Vec<lsp_types::Diagnostic>)>,
// FIXME: should be Vec<flycheck::Diagnostic>
pub(crate) check: IntMap<usize, IntMap<FileId, Vec<lsp_types::Diagnostic>>>,
pub(crate) check: IntMap<
usize,
FxHashMap<Option<Arc<PackageId>>, IntMap<FileId, Vec<lsp_types::Diagnostic>>>,
>,
pub(crate) check_fixes: CheckFixes,
changes: IntSet<FileId>,
/// Counter for supplying a new generation number for diagnostics.
@ -50,18 +55,19 @@ pub(crate) struct Fix {
impl DiagnosticCollection {
pub(crate) fn clear_check(&mut self, flycheck_id: usize) {
if let Some(it) = Arc::make_mut(&mut self.check_fixes).get_mut(&flycheck_id) {
if let Some(it) = self.check.get_mut(&flycheck_id) {
it.clear();
}
if let Some(it) = self.check.get_mut(&flycheck_id) {
self.changes.extend(it.drain().map(|(key, _value)| key));
if let Some(fixes) = Arc::make_mut(&mut self.check_fixes).get_mut(&flycheck_id) {
fixes.clear();
}
}
pub(crate) fn clear_check_all(&mut self) {
Arc::make_mut(&mut self.check_fixes).clear();
self.changes
.extend(self.check.values_mut().flat_map(|it| it.drain().map(|(key, _value)| key)))
self.changes.extend(
self.check.values_mut().flat_map(|it| it.drain().flat_map(|(_, v)| v.into_keys())),
)
}
pub(crate) fn clear_native_for(&mut self, file_id: FileId) {
@ -70,14 +76,33 @@ impl DiagnosticCollection {
self.changes.insert(file_id);
}
pub(crate) fn clear_check_for_package(
&mut self,
flycheck_id: usize,
package_id: Arc<PackageId>,
) {
let Some(check) = self.check.get_mut(&flycheck_id) else {
return;
};
check.remove(&Some(package_id));
}
pub(crate) fn add_check_diagnostic(
&mut self,
flycheck_id: usize,
package_id: &Option<Arc<PackageId>>,
file_id: FileId,
diagnostic: lsp_types::Diagnostic,
fix: Option<Box<Fix>>,
) {
let diagnostics = self.check.entry(flycheck_id).or_default().entry(file_id).or_default();
let diagnostics = self
.check
.entry(flycheck_id)
.or_default()
.entry(package_id.clone())
.or_default()
.entry(file_id)
.or_default();
for existing_diagnostic in diagnostics.iter() {
if are_diagnostics_equal(existing_diagnostic, &diagnostic) {
return;
@ -86,7 +111,14 @@ impl DiagnosticCollection {
if let Some(fix) = fix {
let check_fixes = Arc::make_mut(&mut self.check_fixes);
check_fixes.entry(flycheck_id).or_default().entry(file_id).or_default().push(*fix);
check_fixes
.entry(flycheck_id)
.or_default()
.entry(package_id.clone())
.or_default()
.entry(file_id)
.or_default()
.push(*fix);
}
diagnostics.push(diagnostic);
self.changes.insert(file_id);
@ -135,7 +167,12 @@ impl DiagnosticCollection {
) -> impl Iterator<Item = &lsp_types::Diagnostic> {
let native_syntax = self.native_syntax.get(&file_id).into_iter().flat_map(|(_, d)| d);
let native_semantic = self.native_semantic.get(&file_id).into_iter().flat_map(|(_, d)| d);
let check = self.check.values().filter_map(move |it| it.get(&file_id)).flatten();
let check = self
.check
.values()
.flat_map(|it| it.values())
.filter_map(move |it| it.get(&file_id))
.flatten();
native_syntax.chain(native_semantic).chain(check)
}

115
crates/rust-analyzer/src/flycheck.rs
View file

@ -1,8 +1,9 @@
//! Flycheck provides the functionality needed to run `cargo check` to provide
//! LSP diagnostics based on the output of the command.
use std::{fmt, io, process::Command, time::Duration};
use std::{fmt, io, mem, process::Command, time::Duration};
use cargo_metadata::PackageId;
use crossbeam_channel::{select_biased, unbounded, Receiver, Sender};
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
use rustc_hash::FxHashMap;
@ -13,6 +14,7 @@ pub(crate) use cargo_metadata::diagnostic::{
Applicability, Diagnostic, DiagnosticCode, DiagnosticLevel, DiagnosticSpan,
};
use toolchain::Tool;
use triomphe::Arc;
use crate::command::{CommandHandle, ParseFromLine};
@ -151,10 +153,19 @@ impl FlycheckHandle {
pub(crate) enum FlycheckMessage {
/// Request adding a diagnostic with fixes included to a file
AddDiagnostic { id: usize, workspace_root: AbsPathBuf, diagnostic: Diagnostic },
AddDiagnostic {
id: usize,
workspace_root: Arc<AbsPathBuf>,
diagnostic: Diagnostic,
package_id: Option<Arc<PackageId>>,
},
/// Request clearing all previous diagnostics
ClearDiagnostics { id: usize },
/// Request clearing all outdated diagnostics.
ClearDiagnostics {
id: usize,
/// The package whose diagnostics to clear, or if unspecified, all diagnostics.
package_id: Option<Arc<PackageId>>,
},
/// Request check progress notification to client
Progress {
@ -167,15 +178,18 @@ pub(crate) enum FlycheckMessage {
impl fmt::Debug for FlycheckMessage {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic } => f
FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic, package_id } => f
.debug_struct("AddDiagnostic")
.field("id", id)
.field("workspace_root", workspace_root)
.field("package_id", package_id)
.field("diagnostic_code", &diagnostic.code.as_ref().map(|it| &it.code))
.finish(),
FlycheckMessage::ClearDiagnostics { id } => {
f.debug_struct("ClearDiagnostics").field("id", id).finish()
}
FlycheckMessage::ClearDiagnostics { id, package_id } => f
.debug_struct("ClearDiagnostics")
.field("id", id)
.field("package_id", package_id)
.finish(),
FlycheckMessage::Progress { id, progress } => {
f.debug_struct("Progress").field("id", id).field("progress", progress).finish()
}
@ -201,12 +215,13 @@ enum StateChange {
struct FlycheckActor {
/// The workspace id of this flycheck instance.
id: usize,
sender: Sender<FlycheckMessage>,
config: FlycheckConfig,
manifest_path: Option<AbsPathBuf>,
/// Either the workspace root of the workspace we are flychecking,
/// or the project root of the project.
root: AbsPathBuf,
root: Arc<AbsPathBuf>,
sysroot_root: Option<AbsPathBuf>,
/// CargoHandle exists to wrap around the communication needed to be able to
/// run `cargo check` without blocking. Currently the Rust standard library
@ -216,8 +231,13 @@ struct FlycheckActor {
command_handle: Option<CommandHandle<CargoCheckMessage>>,
/// The receiver side of the channel mentioned above.
command_receiver: Option<Receiver<CargoCheckMessage>>,
package_status: FxHashMap<Arc<PackageId>, DiagnosticReceived>,
}
status: FlycheckStatus,
#[derive(PartialEq, Eq, Copy, Clone, Debug)]
enum DiagnosticReceived {
Yes,
No,
}
#[allow(clippy::large_enum_variant)]
@ -226,13 +246,6 @@ enum Event {
CheckEvent(Option<CargoCheckMessage>),
}
#[derive(PartialEq)]
enum FlycheckStatus {
Started,
DiagnosticSent,
Finished,
}
pub(crate) const SAVED_FILE_PLACEHOLDER: &str = "$saved_file";
impl FlycheckActor {
@ -250,11 +263,11 @@ impl FlycheckActor {
sender,
config,
sysroot_root,
root: workspace_root,
root: Arc::new(workspace_root),
manifest_path,
command_handle: None,
command_receiver: None,
status: FlycheckStatus::Finished,
package_status: FxHashMap::default(),
}
}
@ -307,13 +320,11 @@ impl FlycheckActor {
self.command_handle = Some(command_handle);
self.command_receiver = Some(receiver);
self.report_progress(Progress::DidStart);
self.status = FlycheckStatus::Started;
}
Err(error) => {
self.report_progress(Progress::DidFailToRestart(format!(
"Failed to run the following command: {formatted_command} error={error}"
)));
self.status = FlycheckStatus::Finished;
}
}
}
@ -333,11 +344,25 @@ impl FlycheckActor {
error
);
}
if self.status == FlycheckStatus::Started {
self.send(FlycheckMessage::ClearDiagnostics { id: self.id });
if self.package_status.is_empty() {
// We finished without receiving any diagnostics.
// That means all of them are stale.
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
package_id: None,
});
} else {
for (package_id, status) in mem::take(&mut self.package_status) {
if let DiagnosticReceived::No = status {
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
package_id: Some(package_id),
});
}
}
}
self.report_progress(Progress::DidFinish(res));
self.status = FlycheckStatus::Finished;
}
Event::CheckEvent(Some(message)) => match message {
CargoCheckMessage::CompilerArtifact(msg) => {
@ -347,23 +372,32 @@ impl FlycheckActor {
"artifact received"
);
self.report_progress(Progress::DidCheckCrate(msg.target.name));
self.package_status
.entry(Arc::new(msg.package_id))
.or_insert(DiagnosticReceived::No);
}
CargoCheckMessage::Diagnostic(msg) => {
CargoCheckMessage::Diagnostic { diagnostic, package_id } => {
tracing::trace!(
flycheck_id = self.id,
message = msg.message,
message = diagnostic.message,
"diagnostic received"
);
if self.status == FlycheckStatus::Started {
self.send(FlycheckMessage::ClearDiagnostics { id: self.id });
if let Some(package_id) = &package_id {
if !self.package_status.contains_key(package_id) {
self.package_status
.insert(package_id.clone(), DiagnosticReceived::Yes);
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
package_id: Some(package_id.clone()),
});
}
}
self.send(FlycheckMessage::AddDiagnostic {
id: self.id,
package_id,
workspace_root: self.root.clone(),
diagnostic: msg,
diagnostic,
});
self.status = FlycheckStatus::DiagnosticSent;
}
},
}
@ -381,7 +415,7 @@ impl FlycheckActor {
command_handle.cancel();
self.command_receiver.take();
self.report_progress(Progress::DidCancel);
self.status = FlycheckStatus::Finished;
self.package_status.clear();
}
}
@ -401,7 +435,7 @@ impl FlycheckActor {
cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(sysroot_root));
}
cmd.arg(command);
cmd.current_dir(&self.root);
cmd.current_dir(&*self.root);
match package {
Some(pkg) => cmd.arg("-p").arg(pkg),
@ -443,11 +477,11 @@ impl FlycheckActor {
match invocation_strategy {
InvocationStrategy::Once => {
cmd.current_dir(&self.root);
cmd.current_dir(&*self.root);
}
InvocationStrategy::PerWorkspace => {
// FIXME: cmd.current_dir(&affected_workspace);
cmd.current_dir(&self.root);
cmd.current_dir(&*self.root);
}
}
@ -487,7 +521,7 @@ impl FlycheckActor {
#[allow(clippy::large_enum_variant)]
enum CargoCheckMessage {
CompilerArtifact(cargo_metadata::Artifact),
Diagnostic(Diagnostic),
Diagnostic { diagnostic: Diagnostic, package_id: Option<Arc<PackageId>> },
}
impl ParseFromLine for CargoCheckMessage {
@ -502,11 +536,16 @@ impl ParseFromLine for CargoCheckMessage {
Some(CargoCheckMessage::CompilerArtifact(artifact))
}
cargo_metadata::Message::CompilerMessage(msg) => {
Some(CargoCheckMessage::Diagnostic(msg.message))
Some(CargoCheckMessage::Diagnostic {
diagnostic: msg.message,
package_id: Some(Arc::new(msg.package_id)),
})
}
_ => None,
},
JsonMessage::Rustc(message) => Some(CargoCheckMessage::Diagnostic(message)),
JsonMessage::Rustc(message) => {
Some(CargoCheckMessage::Diagnostic { diagnostic: message, package_id: None })
}
};
}

8
crates/rust-analyzer/src/handlers/request.rs
View file

@ -1445,7 +1445,13 @@ pub(crate) fn handle_code_action(
}
// Fixes from `cargo check`.
for fix in snap.check_fixes.values().filter_map(|it| it.get(&frange.file_id)).flatten() {
for fix in snap
.check_fixes
.values()
.flat_map(|it| it.values())
.filter_map(|it| it.get(&frange.file_id))
.flatten()
{
// FIXME: this mapping is awkward and shouldn't exist. Refactor
// `snap.check_fixes` to not convert to LSP prematurely.
let intersect_fix_range = fix

13
crates/rust-analyzer/src/main_loop.rs
View file

@ -715,6 +715,7 @@ impl GlobalState {
error!("FetchWorkspaceError: {e}");
}
self.wants_to_switch = Some("fetched workspace".to_owned());
self.diagnostics.clear_check_all();
(Progress::End, None)
}
};
@ -956,7 +957,7 @@ impl GlobalState {
fn handle_flycheck_msg(&mut self, message: FlycheckMessage) {
match message {
FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic } => {
FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic, package_id } => {
let snap = self.snapshot();
let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
&self.config.diagnostics_map(None),
@ -968,6 +969,7 @@ impl GlobalState {
match url_to_file_id(&self.vfs.read().0, &diag.url) {
Ok(file_id) => self.diagnostics.add_check_diagnostic(
id,
&package_id,
file_id,
diag.diagnostic,
diag.fix,
@ -981,9 +983,12 @@ impl GlobalState {
};
}
}
FlycheckMessage::ClearDiagnostics { id } => self.diagnostics.clear_check(id),
FlycheckMessage::ClearDiagnostics { id, package_id: None } => {
self.diagnostics.clear_check(id)
}
FlycheckMessage::ClearDiagnostics { id, package_id: Some(package_id) } => {
self.diagnostics.clear_check_for_package(id, package_id)
}
FlycheckMessage::Progress { id, progress } => {
let (state, message) = match progress {
flycheck::Progress::DidStart => (Progress::Begin, None),