From 0866b1be894b9148cf69897a1e1aa70e3c416e29 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 18 Aug 2020 16:03:15 +0200 Subject: Align diagnostics config with the rest of rust-analyzer --- crates/ide/src/diagnostics.rs | 74 +++++++++++++++--------- crates/ide/src/lib.rs | 13 ++--- crates/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 +- crates/rust-analyzer/src/diagnostics/to_proto.rs | 18 +++--- crates/rust-analyzer/src/handlers.rs | 13 +---- crates/rust-analyzer/src/main_loop.rs | 2 +- 9 files changed, 84 insertions(+), 89 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 606a6064b..89ff55490 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 4b797f374..2a73abba2 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 43f0196af..c312e0a2e 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 31eb7ff3f..c424aa6e2 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 44fd7c286..99f7751ac 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 108df3eb0..ee6f2a867 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 6d5408156..df5583897 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 4f77b1b4d..69ab1b3b1 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 66e04653a..f039cdc31 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, ); -- cgit v1.2.3