From 0866b1be894b9148cf69897a1e1aa70e3c416e29 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 16:03:15 +0200 Subject: [PATCH] Align diagnostics config with the rest of rust-analyzer --- crates/ide/src/diagnostics.rs | 74 ++++++++++++------- crates/ide/src/lib.rs | 13 ++-- .../rust-analyzer/src/cli/analysis_bench.rs | 7 +- crates/rust-analyzer/src/cli/diagnostics.rs | 5 +- crates/rust-analyzer/src/config.rs | 39 +++------- crates/rust-analyzer/src/diagnostics.rs | 2 +- .../rust-analyzer/src/diagnostics/to_proto.rs | 18 ++--- crates/rust-analyzer/src/handlers.rs | 13 +--- crates/rust-analyzer/src/main_loop.rs | 2 +- editors/code/package.json | 18 ++--- 10 files changed, 93 insertions(+), 98 deletions(-) diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 606a6064b4..89ff554908 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -4,12 +4,15 @@ //! macro-expanded files, but we need to present them to the users in terms of //! original files. So we need to map the ranges. -use std::{cell::RefCell, collections::HashSet}; +mod diagnostics_with_fix; + +use std::cell::RefCell; use base_db::SourceDatabase; use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; use ide_db::RootDatabase; use itertools::Itertools; +use rustc_hash::FxHashSet; use syntax::{ ast::{self, AstNode}, SyntaxNode, TextRange, T, @@ -18,8 +21,7 @@ use text_edit::TextEdit; use crate::{Diagnostic, FileId, Fix, SourceFileEdit}; -mod diagnostics_with_fix; -use diagnostics_with_fix::DiagnosticWithFix; +use self::diagnostics_with_fix::DiagnosticWithFix; #[derive(Debug, Copy, Clone)] pub enum Severity { @@ -27,11 +29,16 @@ pub enum Severity { WeakWarning, } +#[derive(Default, Debug, Clone)] +pub struct DiagnosticsConfig { + pub disable_experimental: bool, + pub disabled: FxHashSet, +} + pub(crate) fn diagnostics( db: &RootDatabase, + config: &DiagnosticsConfig, file_id: FileId, - enable_experimental: bool, - disabled_diagnostics: Option>, ) -> Vec { let _p = profile::span("diagnostics"); let sema = Semantics::new(db); @@ -52,7 +59,7 @@ pub(crate) fn diagnostics( check_struct_shorthand_initialization(&mut res, file_id, &node); } let res = RefCell::new(res); - let mut sink_builder = DiagnosticSinkBuilder::new() + let sink_builder = DiagnosticSinkBuilder::new() .on::(|d| { res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) @@ -66,12 +73,8 @@ pub(crate) fn diagnostics( res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) // Only collect experimental diagnostics when they're enabled. - .filter(|diag| !diag.is_experimental() || enable_experimental); - - if let Some(disabled_diagnostics) = disabled_diagnostics { - // Do not collect disabled diagnostics. - sink_builder = sink_builder.filter(move |diag| !disabled_diagnostics.contains(diag.name())); - } + .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) + .filter(|diag| !config.disabled.contains(diag.name())); // Finalize the `DiagnosticSink` building process. let mut sink = sink_builder @@ -187,12 +190,14 @@ fn check_struct_shorthand_initialization( #[cfg(test)] mod tests { - use std::collections::HashSet; + use expect::{expect, Expect}; use stdx::trim_indent; use test_utils::assert_eq_text; - use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis}; - use expect::{expect, Expect}; + use crate::{ + mock_analysis::{analysis_and_position, single_file, MockAnalysis}, + DiagnosticsConfig, + }; /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: @@ -203,8 +208,11 @@ mod tests { let after = trim_indent(ra_fixture_after); let (analysis, file_position) = analysis_and_position(ra_fixture_before); - let diagnostic = - analysis.diagnostics(file_position.file_id, true, None).unwrap().pop().unwrap(); + let diagnostic = analysis + .diagnostics(&DiagnosticsConfig::default(), file_position.file_id) + .unwrap() + .pop() + .unwrap(); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); @@ -230,7 +238,11 @@ mod tests { let ra_fixture_after = &trim_indent(ra_fixture_after); let (analysis, file_pos) = analysis_and_position(ra_fixture_before); let current_file_id = file_pos.file_id; - let diagnostic = analysis.diagnostics(current_file_id, true, None).unwrap().pop().unwrap(); + let diagnostic = analysis + .diagnostics(&DiagnosticsConfig::default(), current_file_id) + .unwrap() + .pop() + .unwrap(); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap(); let changed_file_id = edit.file_id; @@ -251,7 +263,9 @@ mod tests { let analysis = mock.analysis(); let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) + .flat_map(|file_id| { + analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() + }) .collect::>(); assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); } @@ -259,8 +273,8 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics, /// and checks that provided diagnostics aren't spawned during analysis. fn check_disabled_diagnostics(ra_fixture: &str, disabled_diagnostics: &[&'static str]) { - let disabled_diagnostics: HashSet<_> = - disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect(); + let mut config = DiagnosticsConfig::default(); + config.disabled = disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect(); let mock = MockAnalysis::with_files(ra_fixture); let files = mock.files().map(|(it, _)| it).collect::>(); @@ -269,15 +283,17 @@ mod tests { let diagnostics = files .clone() .into_iter() - .flat_map(|file_id| { - analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap() - }) + .flat_map(|file_id| analysis.diagnostics(&config, file_id).unwrap()) .collect::>(); // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. for diagnostic in diagnostics { if let Some(name) = diagnostic.name { - assert!(!disabled_diagnostics.contains(&name), "Diagnostic {} is disabled", name); + assert!( + !disabled_diagnostics.contains(&name.as_str()), + "Diagnostic {} is disabled", + name + ); } } @@ -288,21 +304,23 @@ mod tests { // will no longer exist. let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) + .flat_map(|file_id| { + analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() + }) .collect::>(); assert!( diagnostics .into_iter() .filter_map(|diag| diag.name) - .any(|name| disabled_diagnostics.contains(&name)), + .any(|name| disabled_diagnostics.contains(&name.as_str())), "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?" ); } fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = single_file(ra_fixture); - let diagnostics = analysis.diagnostics(file_id, true, None).unwrap(); + let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); expect.assert_debug_eq(&diagnostics) } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 4b797f374c..2a73abba27 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -44,7 +44,7 @@ mod syntax_highlighting; mod syntax_tree; mod typing; -use std::{collections::HashSet, sync::Arc}; +use std::sync::Arc; use base_db::{ salsa::{self, ParallelDatabase}, @@ -65,7 +65,7 @@ pub use crate::{ completion::{ CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat, }, - diagnostics::Severity, + diagnostics::{DiagnosticsConfig, Severity}, display::NavigationTarget, expand_macro::ExpandedMacro, file_structure::StructureNode, @@ -148,7 +148,7 @@ pub struct AnalysisHost { } impl AnalysisHost { - pub fn new(lru_capacity: Option) -> Self { + pub fn new(lru_capacity: Option) -> AnalysisHost { AnalysisHost { db: RootDatabase::new(lru_capacity) } } @@ -495,13 +495,10 @@ impl Analysis { /// Computes the set of diagnostics for the given file. pub fn diagnostics( &self, + config: &DiagnosticsConfig, file_id: FileId, - enable_experimental: bool, - disabled_diagnostics: Option>, ) -> Cancelable> { - self.with_db(|db| { - diagnostics::diagnostics(db, file_id, enable_experimental, disabled_diagnostics) - }) + self.with_db(|db| diagnostics::diagnostics(db, config, file_id)) } /// Returns the edit required to rename reference at the position to the new diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 43f0196afc..c312e0a2e8 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/crates/rust-analyzer/src/cli/analysis_bench.rs @@ -7,7 +7,10 @@ use base_db::{ salsa::{Database, Durability}, FileId, }; -use ide::{Analysis, AnalysisChange, AnalysisHost, CompletionConfig, FilePosition, LineCol}; +use ide::{ + Analysis, AnalysisChange, AnalysisHost, CompletionConfig, DiagnosticsConfig, FilePosition, + LineCol, +}; use vfs::AbsPathBuf; use crate::{ @@ -71,7 +74,7 @@ impl BenchCmd { match &self.what { BenchWhat::Highlight { .. } => { let res = do_work(&mut host, file_id, |analysis| { - analysis.diagnostics(file_id, true, None).unwrap(); + analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); analysis.highlight_as_html(file_id, false).unwrap() }); if verbosity.is_verbose() { diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs index 31eb7ff3f8..c424aa6e28 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/crates/rust-analyzer/src/cli/diagnostics.rs @@ -8,7 +8,7 @@ use rustc_hash::FxHashSet; use base_db::SourceDatabaseExt; use hir::Crate; -use ide::Severity; +use ide::{DiagnosticsConfig, Severity}; use crate::cli::{load_cargo::load_cargo, Result}; @@ -47,7 +47,8 @@ pub fn diagnostics( String::from("unknown") }; println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id)); - for diagnostic in analysis.diagnostics(file_id, true, None).unwrap() { + for diagnostic in analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() + { if matches!(diagnostic.severity, Severity::Error) { found_error = true; } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 44fd7c286f..99f7751acd 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -7,24 +7,25 @@ //! configure the server itself, feature flags are passed into analysis, and //! tweak things like automatic insertion of `()` in completions. -use std::{collections::HashSet, ffi::OsString, path::PathBuf}; +use std::{ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; -use ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; +use ide::{AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig}; use lsp_types::ClientCapabilities; use project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; +use rustc_hash::FxHashSet; use serde::Deserialize; use vfs::AbsPathBuf; -use crate::diagnostics::DiagnosticsConfig; +use crate::diagnostics::DiagnosticsMapConfig; #[derive(Debug, Clone)] pub struct Config { pub client_caps: ClientCapsConfig, pub publish_diagnostics: bool, - pub experimental_diagnostics: bool, pub diagnostics: DiagnosticsConfig, + pub diagnostics_map: DiagnosticsMapConfig, pub lru_capacity: Option, pub proc_macro_srv: Option<(PathBuf, Vec)>, pub files: FilesConfig, @@ -45,14 +46,6 @@ pub struct Config { pub with_sysroot: bool, pub linked_projects: Vec, pub root_path: AbsPathBuf, - - pub analysis: AnalysisConfig, -} - -/// Configuration parameters for the analysis run. -#[derive(Debug, Default, Clone)] -pub struct AnalysisConfig { - pub disabled_diagnostics: HashSet, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -146,8 +139,8 @@ impl Config { with_sysroot: true, publish_diagnostics: true, - experimental_diagnostics: true, diagnostics: DiagnosticsConfig::default(), + diagnostics_map: DiagnosticsMapConfig::default(), lru_capacity: None, proc_macro_srv: None, files: FilesConfig { watcher: FilesWatcher::Notify, exclude: Vec::new() }, @@ -184,8 +177,6 @@ impl Config { hover: HoverConfig::default(), linked_projects: Vec::new(), root_path, - - analysis: AnalysisConfig::default(), } } @@ -200,8 +191,11 @@ impl Config { self.with_sysroot = data.withSysroot; self.publish_diagnostics = data.diagnostics_enable; - self.experimental_diagnostics = data.diagnostics_enableExperimental; self.diagnostics = DiagnosticsConfig { + disable_experimental: !data.diagnostics_enableExperimental, + disabled: data.diagnostics_disabled, + }; + self.diagnostics_map = DiagnosticsMapConfig { warnings_as_info: data.diagnostics_warningsAsInfo, warnings_as_hint: data.diagnostics_warningsAsHint, }; @@ -303,8 +297,6 @@ impl Config { goto_type_def: data.hoverActions_enable && data.hoverActions_gotoTypeDef, }; - self.analysis = AnalysisConfig { disabled_diagnostics: data.analysis_disabledDiagnostics }; - log::info!("Config::update() = {:#?}", self); } @@ -369,14 +361,6 @@ impl Config { self.client_caps.status_notification = get_bool("statusNotification"); } } - - pub fn disabled_diagnostics(&self) -> Option> { - if self.analysis.disabled_diagnostics.is_empty() { - None - } else { - Some(self.analysis.disabled_diagnostics.clone()) - } - } } #[derive(Deserialize)] @@ -434,6 +418,7 @@ config_data! { diagnostics_enable: bool = true, diagnostics_enableExperimental: bool = true, + diagnostics_disabled: FxHashSet = FxHashSet::default(), diagnostics_warningsAsHint: Vec = Vec::new(), diagnostics_warningsAsInfo: Vec = Vec::new(), @@ -464,7 +449,5 @@ config_data! { rustfmt_overrideCommand: Option> = None, withSysroot: bool = true, - - analysis_disabledDiagnostics: HashSet = HashSet::new(), } } diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs index 108df3eb04..ee6f2a867b 100644 --- a/crates/rust-analyzer/src/diagnostics.rs +++ b/crates/rust-analyzer/src/diagnostics.rs @@ -11,7 +11,7 @@ use crate::lsp_ext; pub(crate) type CheckFixes = Arc>>; #[derive(Debug, Default, Clone)] -pub struct DiagnosticsConfig { +pub struct DiagnosticsMapConfig { pub warnings_as_info: Vec, pub warnings_as_hint: Vec, } diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index 6d54081560..df55838973 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs @@ -7,11 +7,11 @@ use stdx::format_to; use crate::{lsp_ext, to_proto::url_from_abs_path}; -use super::DiagnosticsConfig; +use super::DiagnosticsMapConfig; /// Determines the LSP severity from a diagnostic fn diagnostic_severity( - config: &DiagnosticsConfig, + config: &DiagnosticsMapConfig, level: flycheck::DiagnosticLevel, code: Option, ) -> Option { @@ -141,7 +141,7 @@ pub(crate) struct MappedRustDiagnostic { /// /// If the diagnostic has no primary span this will return `None` pub(crate) fn map_rust_diagnostic_to_lsp( - config: &DiagnosticsConfig, + config: &DiagnosticsMapConfig, rd: &flycheck::Diagnostic, workspace_root: &Path, ) -> Vec { @@ -259,10 +259,10 @@ mod tests { use expect::{expect_file, ExpectFile}; fn check(diagnostics_json: &str, expect: ExpectFile) { - check_with_config(DiagnosticsConfig::default(), diagnostics_json, expect) + check_with_config(DiagnosticsMapConfig::default(), diagnostics_json, expect) } - fn check_with_config(config: DiagnosticsConfig, diagnostics_json: &str, expect: ExpectFile) { + fn check_with_config(config: DiagnosticsMapConfig, diagnostics_json: &str, expect: ExpectFile) { let diagnostic: flycheck::Diagnostic = serde_json::from_str(diagnostics_json).unwrap(); let workspace_root = Path::new("/test/"); let actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root); @@ -402,9 +402,9 @@ mod tests { #[cfg(not(windows))] fn rustc_unused_variable_as_info() { check_with_config( - DiagnosticsConfig { + DiagnosticsMapConfig { warnings_as_info: vec!["unused_variables".to_string()], - ..DiagnosticsConfig::default() + ..DiagnosticsMapConfig::default() }, r##"{ "message": "unused variable: `foo`", @@ -486,9 +486,9 @@ mod tests { #[cfg(not(windows))] fn rustc_unused_variable_as_hint() { check_with_config( - DiagnosticsConfig { + DiagnosticsMapConfig { warnings_as_hint: vec!["unused_variables".to_string()], - ..DiagnosticsConfig::default() + ..DiagnosticsMapConfig::default() }, r##"{ "message": "unused variable: `foo`", diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 4f77b1b4d2..69ab1b3b1f 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -775,11 +775,7 @@ fn handle_fixes( None => {} }; - let diagnostics = snap.analysis.diagnostics( - file_id, - snap.config.experimental_diagnostics, - snap.config.disabled_diagnostics(), - )?; + let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics, file_id)?; for fix in diagnostics .into_iter() @@ -1051,13 +1047,10 @@ pub(crate) fn publish_diagnostics( ) -> Result> { let _p = profile::span("publish_diagnostics"); let line_index = snap.analysis.file_line_index(file_id)?; + let diagnostics: Vec = snap .analysis - .diagnostics( - file_id, - snap.config.experimental_diagnostics, - snap.config.disabled_diagnostics(), - )? + .diagnostics(&snap.config.diagnostics, file_id)? .into_iter() .map(|d| Diagnostic { range: to_proto::range(&line_index, d.range), diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 66e04653a3..f039cdc310 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -248,7 +248,7 @@ impl GlobalState { Event::Flycheck(task) => match task { flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => { let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp( - &self.config.diagnostics, + &self.config.diagnostics_map, &diagnostic, &workspace_root, ); diff --git a/editors/code/package.json b/editors/code/package.json index 429ff5def4..f079f73b80 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -592,6 +592,15 @@ "default": true, "markdownDescription": "Whether to show experimental rust-analyzer diagnostics that might have more false positives than usual." }, + "rust-analyzer.diagnostics.disabled": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "string" + }, + "description": "List of rust-analyzer diagnostics to disable", + "default": [] + }, "rust-analyzer.diagnostics.warningsAsInfo": { "type": "array", "uniqueItems": true, @@ -609,15 +618,6 @@ }, "description": "List of warnings that should be displayed with hint severity.\nThe warnings will be indicated by faded text or three dots in code and will not show up in the problems panel.", "default": [] - }, - "rust-analyzer.analysis.disabledDiagnostics": { - "type": "array", - "uniqueItems": true, - "items": { - "type": "string" - }, - "description": "List of rust-analyzer diagnostics to disable", - "default": [] } } },