From c463d217a1e001abe6a812f309d93527e28a70c6 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 13:18:47 +0300 Subject: Add names for diagnostics and add a possibility to disable them --- crates/ra_hir_def/src/diagnostics.rs | 3 +++ crates/ra_hir_expand/src/diagnostics.rs | 18 ++++++++++++++++-- crates/ra_hir_ty/src/diagnostics.rs | 25 +++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 30db48f86..1d28c24e8 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -15,6 +15,9 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { + fn name(&self) -> String { + "unresolved-module".to_string() + } fn message(&self) -> String { "unresolved module".to_string() } diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 84ba97b14..bf9fb081a 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -14,13 +14,14 @@ //! subsystem provides a separate, non-query-based API which can walk all stored //! values and transform them into instances of `Diagnostic`. -use std::{any::Any, fmt}; +use std::{any::Any, collections::HashSet, fmt}; use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn name(&self) -> String; fn message(&self) -> String; fn source(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); @@ -49,10 +50,16 @@ pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, default_callback: Box, + disabled_diagnostics: HashSet, } impl<'a> DiagnosticSink<'a> { pub fn push(&mut self, d: impl Diagnostic) { + if self.disabled_diagnostics.contains(&d.name()) { + // This diagnostic is disabled, ignore it completely. + return; + } + let d: &dyn Diagnostic = &d; self._push(d); } @@ -76,11 +83,12 @@ impl<'a> DiagnosticSink<'a> { pub struct DiagnosticSinkBuilder<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, + disabled_diagnostics: HashSet, } impl<'a> DiagnosticSinkBuilder<'a> { pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new() } + Self { callbacks: Vec::new(), filters: Vec::new(), disabled_diagnostics: HashSet::new() } } pub fn filter bool + 'a>(mut self, cb: F) -> Self { @@ -100,11 +108,17 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } + pub fn disable_diagnostic(mut self, diagnostic: impl Into) -> Self { + self.disabled_diagnostics.insert(diagnostic.into()); + self + } + pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { DiagnosticSink { callbacks: self.callbacks, filters: self.filters, default_callback: Box::new(default_callback), + disabled_diagnostics: self.disabled_diagnostics, } } } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 977c0525b..0b3e16ae7 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -33,6 +33,10 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { + fn name(&self) -> String { + "no-such-field".to_string() + } + fn message(&self) -> String { "no such field".to_string() } @@ -64,6 +68,9 @@ pub struct MissingFields { } impl Diagnostic for MissingFields { + fn name(&self) -> String { + "missing-structure-fields".to_string() + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -97,6 +104,9 @@ pub struct MissingPatFields { } impl Diagnostic for MissingPatFields { + fn name(&self) -> String { + "missing-pat-fields".to_string() + } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); for field in &self.missed_fields { @@ -120,6 +130,9 @@ pub struct MissingMatchArms { } impl Diagnostic for MissingMatchArms { + fn name(&self) -> String { + "missing-match-arm".to_string() + } fn message(&self) -> String { String::from("Missing match arm") } @@ -138,6 +151,9 @@ pub struct MissingOkInTailExpr { } impl Diagnostic for MissingOkInTailExpr { + fn name(&self) -> String { + "missing-ok-in-tail-expr".to_string() + } fn message(&self) -> String { "wrap return expression in Ok".to_string() } @@ -166,6 +182,9 @@ pub struct BreakOutsideOfLoop { } impl Diagnostic for BreakOutsideOfLoop { + fn name(&self) -> String { + "break-outside-of-loop".to_string() + } fn message(&self) -> String { "break outside of loop".to_string() } @@ -194,6 +213,9 @@ pub struct MissingUnsafe { } impl Diagnostic for MissingUnsafe { + fn name(&self) -> String { + "missing-unsafe".to_string() + } fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } @@ -224,6 +246,9 @@ pub struct MismatchedArgCount { } impl Diagnostic for MismatchedArgCount { + fn name(&self) -> String { + "mismatched-arg-count".to_string() + } fn message(&self) -> String { let s = if self.expected == 1 { "" } else { "s" }; format!("Expected {} argument{}, found {}", self.expected, s, self.found) -- cgit v1.2.3 From 90857ff8b08d73945598bac12a841559e86402b1 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:25:55 +0300 Subject: Add an AnalysisConfig structure and use it to configure diagnostics run --- crates/ra_hir_expand/src/diagnostics.rs | 17 ++--------------- crates/ra_ide/src/diagnostics.rs | 24 +++++++++++++++++++++--- crates/ra_ide/src/lib.rs | 28 +++++++++++++++++++++++----- crates/rust-analyzer/src/config.rs | 12 ++++++++++-- crates/rust-analyzer/src/global_state.rs | 2 +- 5 files changed, 57 insertions(+), 26 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index bf9fb081a..ef1d61144 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -14,7 +14,7 @@ //! subsystem provides a separate, non-query-based API which can walk all stored //! values and transform them into instances of `Diagnostic`. -use std::{any::Any, collections::HashSet, fmt}; +use std::{any::Any, fmt}; use ra_syntax::{SyntaxNode, SyntaxNodePtr}; @@ -50,16 +50,10 @@ pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, default_callback: Box, - disabled_diagnostics: HashSet, } impl<'a> DiagnosticSink<'a> { pub fn push(&mut self, d: impl Diagnostic) { - if self.disabled_diagnostics.contains(&d.name()) { - // This diagnostic is disabled, ignore it completely. - return; - } - let d: &dyn Diagnostic = &d; self._push(d); } @@ -83,12 +77,11 @@ impl<'a> DiagnosticSink<'a> { pub struct DiagnosticSinkBuilder<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, - disabled_diagnostics: HashSet, } impl<'a> DiagnosticSinkBuilder<'a> { pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new(), disabled_diagnostics: HashSet::new() } + Self { callbacks: Vec::new(), filters: Vec::new() } } pub fn filter bool + 'a>(mut self, cb: F) -> Self { @@ -108,17 +101,11 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } - pub fn disable_diagnostic(mut self, diagnostic: impl Into) -> Self { - self.disabled_diagnostics.insert(diagnostic.into()); - self - } - pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { DiagnosticSink { callbacks: self.callbacks, filters: self.filters, default_callback: Box::new(default_callback), - disabled_diagnostics: self.disabled_diagnostics, } } } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 73c0b8275..33e4f1743 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -21,7 +21,7 @@ use ra_syntax::{ }; use ra_text_edit::{TextEdit, TextEditBuilder}; -use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +use crate::{AnalysisConfig, Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; #[derive(Debug, Copy, Clone)] pub enum Severity { @@ -33,6 +33,7 @@ pub(crate) fn diagnostics( db: &RootDatabase, file_id: FileId, enable_experimental: bool, + analysis_config: &AnalysisConfig, ) -> Vec { let _p = profile("diagnostics"); let sema = Semantics::new(db); @@ -41,6 +42,7 @@ pub(crate) fn diagnostics( // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { + name: None, range: err.range(), message: format!("Syntax Error: {}", err), severity: Severity::Error, @@ -52,7 +54,7 @@ pub(crate) fn diagnostics( check_struct_shorthand_initialization(&mut res, file_id, &node); } let res = RefCell::new(res); - let mut sink = DiagnosticSinkBuilder::new() + let mut sink_builder = DiagnosticSinkBuilder::new() .on::(|d| { let original_file = d.source().file_id.original_file(db); let fix = Fix::new( @@ -61,6 +63,7 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -95,6 +98,7 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -108,6 +112,7 @@ pub(crate) fn diagnostics( let source_change = SourceFileEdit { file_id, edit }.into(); let fix = Fix::new("Wrap with ok", source_change); res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -116,6 +121,7 @@ pub(crate) fn diagnostics( }) .on::(|d| { res.borrow_mut().push(Diagnostic { + name: Some(d.name()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -123,10 +129,20 @@ pub(crate) fn diagnostics( }) }) // Only collect experimental diagnostics when they're enabled. - .filter(|diag| !diag.is_experimental() || enable_experimental) + .filter(|diag| !diag.is_experimental() || enable_experimental); + + if !analysis_config.disabled_diagnostics.is_empty() { + // Do not collect disabled diagnostics. + sink_builder = sink_builder + .filter(|diag| !analysis_config.disabled_diagnostics.contains(&diag.name())); + } + + // Finalize the `DiagnosticSink` building process. + let mut sink = sink_builder // Diagnostics not handled above get no fix and default treatment. .build(|d| { res.borrow_mut().push(Diagnostic { + name: Some(d.name()), message: d.message(), range: sema.diagnostics_range(d).range, severity: Severity::Error, @@ -234,6 +250,7 @@ fn check_unnecessary_braces_in_use_statement( }); acc.push(Diagnostic { + name: None, range, message: "Unnecessary braces in use statement".to_string(), severity: Severity::WeakWarning, @@ -279,6 +296,7 @@ fn check_struct_shorthand_initialization( let edit = edit_builder.finish(); acc.push(Diagnostic { + name: None, range: record_field.syntax().text_range(), message: "Shorthand struct initialization".to_string(), severity: Severity::WeakWarning, diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0fede0d87..3822b9409 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -45,7 +45,7 @@ mod syntax_highlighting; mod syntax_tree; mod typing; -use std::sync::Arc; +use std::{collections::HashSet, sync::Arc}; use ra_cfg::CfgOptions; use ra_db::{ @@ -100,8 +100,15 @@ pub use ra_text_edit::{Indel, TextEdit}; pub type Cancelable = Result; +/// Configuration parameters for the analysis run. +#[derive(Debug, Default, Clone)] +pub struct AnalysisConfig { + pub disabled_diagnostics: HashSet, +} + #[derive(Debug)] pub struct Diagnostic { + pub name: Option, pub message: String, pub range: TextRange, pub severity: Severity, @@ -139,11 +146,16 @@ impl RangeInfo { #[derive(Debug)] pub struct AnalysisHost { db: RootDatabase, + config: AnalysisConfig, } impl AnalysisHost { - pub fn new(lru_capacity: Option) -> AnalysisHost { - AnalysisHost { db: RootDatabase::new(lru_capacity) } + pub fn new(lru_capacity: Option) -> Self { + Self::with_config(lru_capacity, AnalysisConfig::default()) + } + + pub fn with_config(lru_capacity: Option, config: AnalysisConfig) -> Self { + AnalysisHost { db: RootDatabase::new(lru_capacity), config } } pub fn update_lru_capacity(&mut self, lru_capacity: Option) { @@ -153,7 +165,7 @@ impl AnalysisHost { /// Returns a snapshot of the current state, which you can query for /// semantic information. pub fn analysis(&self) -> Analysis { - Analysis { db: self.db.snapshot() } + Analysis { db: self.db.snapshot(), config: self.config.clone() } } /// Applies changes to the current state of the world. If there are @@ -197,6 +209,7 @@ impl Default for AnalysisHost { #[derive(Debug)] pub struct Analysis { db: salsa::Snapshot, + config: AnalysisConfig, } // As a general design guideline, `Analysis` API are intended to be independent @@ -492,7 +505,7 @@ impl Analysis { file_id: FileId, enable_experimental: bool, ) -> Cancelable> { - self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental)) + self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental, &self.config)) } /// Returns the edit required to rename reference at the position to the new @@ -518,6 +531,11 @@ impl Analysis { }) } + /// Sets the provided config. + pub fn set_config(&mut self, config: AnalysisConfig) { + self.config = config; + } + /// Performs an operation on that may be Canceled. fn with_db T + std::panic::UnwindSafe, T>( &self, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 70b4512d0..2c1db9546 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -7,11 +7,11 @@ //! configure the server itself, feature flags are passed into analysis, and //! tweak things like automatic insertion of `()` in completions. -use std::{ffi::OsString, path::PathBuf}; +use std::{collections::HashSet, ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; use lsp_types::ClientCapabilities; -use ra_ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; +use ra_ide::{AnalysisConfig, AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; use ra_project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; use serde::Deserialize; use vfs::AbsPathBuf; @@ -45,6 +45,8 @@ pub struct Config { pub with_sysroot: bool, pub linked_projects: Vec, pub root_path: AbsPathBuf, + + pub analysis: AnalysisConfig, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -176,6 +178,8 @@ impl Config { hover: HoverConfig::default(), linked_projects: Vec::new(), root_path, + + analysis: AnalysisConfig::default(), } } @@ -293,6 +297,8 @@ impl Config { goto_type_def: data.hoverActions_enable && data.hoverActions_gotoTypeDef, }; + self.analysis = AnalysisConfig { disabled_diagnostics: data.analysis_disabledDiagnostics }; + log::info!("Config::update() = {:#?}", self); } @@ -444,5 +450,7 @@ config_data! { rustfmt_overrideCommand: Option> = None, withSysroot: bool = true, + + analysis_disabledDiagnostics: HashSet = HashSet::new(), } } diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 0e592ac1b..46cb7ebe2 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -108,7 +108,7 @@ impl GlobalState { Handle { handle, receiver } }; - let analysis_host = AnalysisHost::new(config.lru_capacity); + let analysis_host = AnalysisHost::with_config(config.lru_capacity, config.analysis.clone()); let (flycheck_sender, flycheck_receiver) = unbounded(); GlobalState { sender, -- cgit v1.2.3 From 78c1a87797b93e9f6fac9daecb9e5d5d8b7e60fd Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:26:36 +0300 Subject: Add a test for disabled diagnostics --- crates/ra_ide/src/diagnostics.rs | 58 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 33e4f1743..4abf602ad 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -316,7 +316,10 @@ mod tests { use stdx::trim_indent; use test_utils::assert_eq_text; - use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis}; + use crate::{ + mock_analysis::{analysis_and_position, single_file, MockAnalysis}, + AnalysisConfig, + }; use expect::{expect, Expect}; /// Takes a multi-file input fixture with annotated cursor positions, @@ -380,6 +383,54 @@ mod tests { assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); } + /// 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: impl IntoIterator, + ) { + let disabled_diagnostics: std::collections::HashSet<_> = + disabled_diagnostics.into_iter().collect(); + + let mock = MockAnalysis::with_files(ra_fixture); + let files = mock.files().map(|(it, _)| it).collect::>(); + let mut analysis = mock.analysis(); + analysis.set_config(AnalysisConfig { disabled_diagnostics: disabled_diagnostics.clone() }); + + let diagnostics = files + .clone() + .into_iter() + .flat_map(|file_id| analysis.diagnostics(file_id, true).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); + } + } + + // Then, we must reset the config and repeat the check, so that we'll be sure that without + // config these diagnostics are emitted. + // This is required for tests to not become outdated if e.g. diagnostics name changes: + // without this additional run the test will pass simply because a diagnostic with an old name + // will no longer exist. + analysis.set_config(AnalysisConfig { disabled_diagnostics: Default::default() }); + + let diagnostics = files + .into_iter() + .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .collect::>(); + + assert!( + diagnostics + .into_iter() + .filter_map(|diag| diag.name) + .any(|name| disabled_diagnostics.contains(&name)), + "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).unwrap(); @@ -814,4 +865,9 @@ struct Foo { ", ) } + + #[test] + fn test_disabled_diagnostics() { + check_disabled_diagnostics(r#"mod foo;"#, vec!["unresolved-module".to_string()]); + } } -- cgit v1.2.3 From c51fb7aca531b98e01a8a71a30bb35d1376efe02 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Fri, 7 Aug 2020 14:35:43 +0300 Subject: Fix failing test --- crates/ra_ide/src/diagnostics.rs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'crates') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 4abf602ad..8e011a40d 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -693,6 +693,9 @@ fn test_fn() { expect![[r#" [ Diagnostic { + name: Some( + "unresolved-module", + ), message: "unresolved module", range: 0..8, severity: Error, -- cgit v1.2.3 From 831e3d58b32ad64329f0c84ac93b7b97c7d6c268 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 12 Aug 2020 15:33:07 +0300 Subject: Replace String with &'static str --- crates/ra_hir_def/src/diagnostics.rs | 4 ++-- crates/ra_hir_expand/src/diagnostics.rs | 2 +- crates/ra_hir_ty/src/diagnostics.rs | 32 ++++++++++++++++---------------- crates/ra_ide/src/diagnostics.rs | 26 ++++++++++++-------------- 4 files changed, 31 insertions(+), 33 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 1d28c24e8..481b13a87 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -15,8 +15,8 @@ pub struct UnresolvedModule { } impl Diagnostic for UnresolvedModule { - fn name(&self) -> String { - "unresolved-module".to_string() + fn name(&self) -> &'static str { + "unresolved-module" } fn message(&self) -> String { "unresolved module".to_string() diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index ef1d61144..507132a13 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -21,7 +21,7 @@ use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn name(&self) -> String; + fn name(&self) -> &'static str; fn message(&self) -> String; fn source(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 0b3e16ae7..56acd3bbf 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -33,8 +33,8 @@ pub struct NoSuchField { } impl Diagnostic for NoSuchField { - fn name(&self) -> String { - "no-such-field".to_string() + fn name(&self) -> &'static str { + "no-such-field" } fn message(&self) -> String { @@ -68,8 +68,8 @@ pub struct MissingFields { } impl Diagnostic for MissingFields { - fn name(&self) -> String { - "missing-structure-fields".to_string() + fn name(&self) -> &'static str { + "missing-structure-fields" } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); @@ -104,8 +104,8 @@ pub struct MissingPatFields { } impl Diagnostic for MissingPatFields { - fn name(&self) -> String { - "missing-pat-fields".to_string() + fn name(&self) -> &'static str { + "missing-pat-fields" } fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); @@ -130,8 +130,8 @@ pub struct MissingMatchArms { } impl Diagnostic for MissingMatchArms { - fn name(&self) -> String { - "missing-match-arm".to_string() + fn name(&self) -> &'static str { + "missing-match-arm" } fn message(&self) -> String { String::from("Missing match arm") @@ -151,8 +151,8 @@ pub struct MissingOkInTailExpr { } impl Diagnostic for MissingOkInTailExpr { - fn name(&self) -> String { - "missing-ok-in-tail-expr".to_string() + fn name(&self) -> &'static str { + "missing-ok-in-tail-expr" } fn message(&self) -> String { "wrap return expression in Ok".to_string() @@ -182,8 +182,8 @@ pub struct BreakOutsideOfLoop { } impl Diagnostic for BreakOutsideOfLoop { - fn name(&self) -> String { - "break-outside-of-loop".to_string() + fn name(&self) -> &'static str { + "break-outside-of-loop" } fn message(&self) -> String { "break outside of loop".to_string() @@ -213,8 +213,8 @@ pub struct MissingUnsafe { } impl Diagnostic for MissingUnsafe { - fn name(&self) -> String { - "missing-unsafe".to_string() + fn name(&self) -> &'static str { + "missing-unsafe" } fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") @@ -246,8 +246,8 @@ pub struct MismatchedArgCount { } impl Diagnostic for MismatchedArgCount { - fn name(&self) -> String { - "mismatched-arg-count".to_string() + fn name(&self) -> &'static str { + "mismatched-arg-count" } fn message(&self) -> String { let s = if self.expected == 1 { "" } else { "s" }; diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 8e011a40d..d97bde939 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -63,7 +63,7 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -98,7 +98,7 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -112,7 +112,7 @@ pub(crate) fn diagnostics( let source_change = SourceFileEdit { file_id, edit }.into(); let fix = Fix::new("Wrap with ok", source_change); res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -121,7 +121,7 @@ pub(crate) fn diagnostics( }) .on::(|d| { res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, @@ -133,8 +133,8 @@ pub(crate) fn diagnostics( if !analysis_config.disabled_diagnostics.is_empty() { // Do not collect disabled diagnostics. - sink_builder = sink_builder - .filter(|diag| !analysis_config.disabled_diagnostics.contains(&diag.name())); + sink_builder = + sink_builder.filter(|diag| !analysis_config.disabled_diagnostics.contains(diag.name())); } // Finalize the `DiagnosticSink` building process. @@ -142,7 +142,7 @@ pub(crate) fn diagnostics( // Diagnostics not handled above get no fix and default treatment. .build(|d| { res.borrow_mut().push(Diagnostic { - name: Some(d.name()), + name: Some(d.name().into()), message: d.message(), range: sema.diagnostics_range(d).range, severity: Severity::Error, @@ -313,6 +313,7 @@ fn check_struct_shorthand_initialization( #[cfg(test)] mod tests { + use std::collections::HashSet; use stdx::trim_indent; use test_utils::assert_eq_text; @@ -385,12 +386,9 @@ 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: impl IntoIterator, - ) { - let disabled_diagnostics: std::collections::HashSet<_> = - disabled_diagnostics.into_iter().collect(); + 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 mock = MockAnalysis::with_files(ra_fixture); let files = mock.files().map(|(it, _)| it).collect::>(); @@ -871,6 +869,6 @@ struct Foo { #[test] fn test_disabled_diagnostics() { - check_disabled_diagnostics(r#"mod foo;"#, vec!["unresolved-module".to_string()]); + check_disabled_diagnostics(r#"mod foo;"#, &vec!["unresolved-module"]); } } -- cgit v1.2.3 From ae0b2477fe5cb8b496b41bd37cb06d1a585bc66b Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Wed, 12 Aug 2020 15:48:56 +0300 Subject: Update crates/ra_ide/src/diagnostics.rs Co-authored-by: Jonas Schievink --- crates/ra_ide/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index d97bde939..5ce900bf4 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -869,6 +869,6 @@ struct Foo { #[test] fn test_disabled_diagnostics() { - check_disabled_diagnostics(r#"mod foo;"#, &vec!["unresolved-module"]); + check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]); } } -- cgit v1.2.3 From b56cfcca10994ec2bf1878f222afdb375459f8d3 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Tue, 18 Aug 2020 13:32:29 +0300 Subject: Make disabled diagnostics an argument of corresponding function --- crates/ide/src/diagnostics.rs | 36 ++++++++++++-------------- crates/ide/src/lib.rs | 20 +++++--------- crates/rust-analyzer/src/cli/analysis_bench.rs | 2 +- crates/rust-analyzer/src/cli/diagnostics.rs | 2 +- crates/rust-analyzer/src/config.rs | 8 ++++++ crates/rust-analyzer/src/global_state.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 12 +++++++-- 7 files changed, 43 insertions(+), 39 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index a54df37db..606a6064b 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -4,7 +4,7 @@ //! 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; +use std::{cell::RefCell, collections::HashSet}; use base_db::SourceDatabase; use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; @@ -16,7 +16,7 @@ use syntax::{ }; use text_edit::TextEdit; -use crate::{AnalysisConfig, Diagnostic, FileId, Fix, SourceFileEdit}; +use crate::{Diagnostic, FileId, Fix, SourceFileEdit}; mod diagnostics_with_fix; use diagnostics_with_fix::DiagnosticWithFix; @@ -31,7 +31,7 @@ pub(crate) fn diagnostics( db: &RootDatabase, file_id: FileId, enable_experimental: bool, - analysis_config: &AnalysisConfig, + disabled_diagnostics: Option>, ) -> Vec { let _p = profile::span("diagnostics"); let sema = Semantics::new(db); @@ -68,10 +68,9 @@ pub(crate) fn diagnostics( // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental); - if !analysis_config.disabled_diagnostics.is_empty() { + if let Some(disabled_diagnostics) = disabled_diagnostics { // Do not collect disabled diagnostics. - sink_builder = - sink_builder.filter(|diag| !analysis_config.disabled_diagnostics.contains(diag.name())); + sink_builder = sink_builder.filter(move |diag| !disabled_diagnostics.contains(diag.name())); } // Finalize the `DiagnosticSink` building process. @@ -192,10 +191,7 @@ mod tests { use stdx::trim_indent; use test_utils::assert_eq_text; - use crate::{ - mock_analysis::{analysis_and_position, single_file, MockAnalysis}, - AnalysisConfig, - }; + use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis}; use expect::{expect, Expect}; /// Takes a multi-file input fixture with annotated cursor positions, @@ -207,7 +203,8 @@ 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).unwrap().pop().unwrap(); + let diagnostic = + analysis.diagnostics(file_position.file_id, true, None).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(); @@ -233,7 +230,7 @@ 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).unwrap().pop().unwrap(); + let diagnostic = analysis.diagnostics(current_file_id, true, None).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; @@ -254,7 +251,7 @@ mod tests { let analysis = mock.analysis(); let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) .collect::>(); assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); } @@ -267,13 +264,14 @@ mod tests { let mock = MockAnalysis::with_files(ra_fixture); let files = mock.files().map(|(it, _)| it).collect::>(); - let mut analysis = mock.analysis(); - analysis.set_config(AnalysisConfig { disabled_diagnostics: disabled_diagnostics.clone() }); + let analysis = mock.analysis(); let diagnostics = files .clone() .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| { + analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap() + }) .collect::>(); // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. @@ -288,11 +286,9 @@ mod tests { // This is required for tests to not become outdated if e.g. diagnostics name changes: // without this additional run the test will pass simply because a diagnostic with an old name // will no longer exist. - analysis.set_config(AnalysisConfig { disabled_diagnostics: Default::default() }); - let diagnostics = files .into_iter() - .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) + .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) .collect::>(); assert!( @@ -306,7 +302,7 @@ mod tests { fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = single_file(ra_fixture); - let diagnostics = analysis.diagnostics(file_id, true).unwrap(); + let diagnostics = analysis.diagnostics(file_id, true, None).unwrap(); expect.assert_debug_eq(&diagnostics) } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index b762c994e..a19a379c6 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -151,16 +151,11 @@ impl RangeInfo { #[derive(Debug)] pub struct AnalysisHost { db: RootDatabase, - config: AnalysisConfig, } impl AnalysisHost { pub fn new(lru_capacity: Option) -> Self { - Self::with_config(lru_capacity, AnalysisConfig::default()) - } - - pub fn with_config(lru_capacity: Option, config: AnalysisConfig) -> Self { - AnalysisHost { db: RootDatabase::new(lru_capacity), config } + AnalysisHost { db: RootDatabase::new(lru_capacity) } } pub fn update_lru_capacity(&mut self, lru_capacity: Option) { @@ -170,7 +165,7 @@ impl AnalysisHost { /// Returns a snapshot of the current state, which you can query for /// semantic information. pub fn analysis(&self) -> Analysis { - Analysis { db: self.db.snapshot(), config: self.config.clone() } + Analysis { db: self.db.snapshot() } } /// Applies changes to the current state of the world. If there are @@ -214,7 +209,6 @@ impl Default for AnalysisHost { #[derive(Debug)] pub struct Analysis { db: salsa::Snapshot, - config: AnalysisConfig, } // As a general design guideline, `Analysis` API are intended to be independent @@ -509,8 +503,11 @@ impl Analysis { &self, file_id: FileId, enable_experimental: bool, + disabled_diagnostics: Option>, ) -> Cancelable> { - self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental, &self.config)) + self.with_db(|db| { + diagnostics::diagnostics(db, file_id, enable_experimental, disabled_diagnostics) + }) } /// Returns the edit required to rename reference at the position to the new @@ -539,11 +536,6 @@ impl Analysis { }) } - /// Sets the provided config. - pub fn set_config(&mut self, config: AnalysisConfig) { - self.config = config; - } - /// Performs an operation on that may be Canceled. fn with_db(&self, f: F) -> Cancelable where diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs index 0f614f9e0..43f0196af 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/crates/rust-analyzer/src/cli/analysis_bench.rs @@ -71,7 +71,7 @@ impl BenchCmd { match &self.what { BenchWhat::Highlight { .. } => { let res = do_work(&mut host, file_id, |analysis| { - analysis.diagnostics(file_id, true).unwrap(); + analysis.diagnostics(file_id, true, None).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 3371c4fd3..31eb7ff3f 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/crates/rust-analyzer/src/cli/diagnostics.rs @@ -47,7 +47,7 @@ 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).unwrap() { + for diagnostic in analysis.diagnostics(file_id, true, None).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 949824479..4e3ab05b2 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -363,6 +363,14 @@ 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)] diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 8fa31f59c..212f98a30 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -108,7 +108,7 @@ impl GlobalState { Handle { handle, receiver } }; - let analysis_host = AnalysisHost::with_config(config.lru_capacity, config.analysis.clone()); + let analysis_host = AnalysisHost::new(config.lru_capacity); let (flycheck_sender, flycheck_receiver) = unbounded(); GlobalState { sender, diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 74f73655a..067f5ff66 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -770,7 +770,11 @@ fn handle_fixes( None => {} }; - let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?; + let diagnostics = snap.analysis.diagnostics( + file_id, + snap.config.experimental_diagnostics, + snap.config.disabled_diagnostics(), + )?; for fix in diagnostics .into_iter() @@ -1044,7 +1048,11 @@ pub(crate) fn publish_diagnostics( let line_index = snap.analysis.file_line_index(file_id)?; let diagnostics: Vec = snap .analysis - .diagnostics(file_id, snap.config.experimental_diagnostics)? + .diagnostics( + file_id, + snap.config.experimental_diagnostics, + snap.config.disabled_diagnostics(), + )? .into_iter() .map(|d| Diagnostic { range: to_proto::range(&line_index, d.range), -- cgit v1.2.3 From 34847c8d96ddf98c40117ebacaecec38b9b758fc Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Tue, 18 Aug 2020 13:35:36 +0300 Subject: Move analysis config structure to the config.rs --- crates/ide/src/lib.rs | 6 ------ crates/rust-analyzer/src/config.rs | 8 +++++++- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index a19a379c6..4b797f374 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -99,12 +99,6 @@ pub use text_edit::{Indel, TextEdit}; pub type Cancelable = Result; -/// Configuration parameters for the analysis run. -#[derive(Debug, Default, Clone)] -pub struct AnalysisConfig { - pub disabled_diagnostics: HashSet, -} - #[derive(Debug)] pub struct Diagnostic { pub name: Option, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 4e3ab05b2..44fd7c286 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -10,7 +10,7 @@ use std::{collections::HashSet, ffi::OsString, path::PathBuf}; use flycheck::FlycheckConfig; -use ide::{AnalysisConfig, AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; +use ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig}; use lsp_types::ClientCapabilities; use project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest}; use serde::Deserialize; @@ -49,6 +49,12 @@ pub struct Config { 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)] pub enum LinkedProject { ProjectManifest(ProjectManifest), -- cgit v1.2.3