From f85e383b94376d55bb5ee6be375ef3dc0006590f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 17:29:25 +0300 Subject: internal: refactor inactive code diagnostics --- crates/hir/src/diagnostics.rs | 32 +------ crates/hir/src/lib.rs | 24 ++--- crates/hir_def/src/body/tests.rs | 33 ------- crates/hir_def/src/nameres/tests/diagnostics.rs | 42 --------- crates/ide/src/diagnostics.rs | 30 +++---- crates/ide/src/diagnostics/inactive_code.rs | 113 ++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 133 deletions(-) create mode 100644 crates/ide/src/diagnostics/inactive_code.rs diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 718c86b3a..03e7f5e84 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -5,11 +5,10 @@ //! be expressed in terms of hir types themselves. use std::any::Any; -use cfg::{CfgExpr, CfgOptions, DnfExpr}; +use cfg::{CfgExpr, CfgOptions}; use either::Either; use hir_def::path::ModPath; use hir_expand::{name::Name, HirFileId, InFile}; -use stdx::format_to; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; pub use crate::diagnostics_sink::{ @@ -38,6 +37,7 @@ diagnostics![ UnresolvedImport, UnresolvedMacroCall, MissingFields, + InactiveCode, ]; #[derive(Debug)] @@ -62,39 +62,13 @@ pub struct UnresolvedMacroCall { pub path: ModPath, } -// Diagnostic: inactive-code -// -// This diagnostic is shown for code with inactive `#[cfg]` attributes. #[derive(Debug, Clone, Eq, PartialEq)] pub struct InactiveCode { - pub file: HirFileId, - pub node: SyntaxNodePtr, + pub node: InFile, pub cfg: CfgExpr, pub opts: CfgOptions, } -impl Diagnostic for InactiveCode { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("inactive-code") - } - fn message(&self) -> String { - let inactive = DnfExpr::new(self.cfg.clone()).why_inactive(&self.opts); - let mut buf = "code is inactive due to #[cfg] directives".to_string(); - - if let Some(inactive) = inactive { - format_to!(buf, ": {}", inactive); - } - - buf - } - fn display_source(&self) -> InFile { - InFile::new(self.file, self.node.clone()) - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - // Diagnostic: unresolved-proc-macro // // This diagnostic is shown when a procedural macro can not be found. This usually means that diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 0a9414013..d59b52b25 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -506,12 +506,14 @@ impl Module { DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } => { let item = ast.to_node(db.upcast()); - sink.push(InactiveCode { - file: ast.file_id, - node: AstPtr::new(&item).into(), - cfg: cfg.clone(), - opts: opts.clone(), - }); + acc.push( + InactiveCode { + node: ast.with_value(AstPtr::new(&item).into()), + cfg: cfg.clone(), + opts: opts.clone(), + } + .into(), + ); } DefDiagnosticKind::UnresolvedProcMacro { ast } => { @@ -1045,12 +1047,10 @@ impl Function { let source_map = db.body_with_source_map(self.id.into()).1; for diag in source_map.diagnostics() { match diag { - BodyDiagnostic::InactiveCode { node, cfg, opts } => sink.push(InactiveCode { - file: node.file_id, - node: node.value.clone(), - cfg: cfg.clone(), - opts: opts.clone(), - }), + BodyDiagnostic::InactiveCode { node, cfg, opts } => acc.push( + InactiveCode { node: node.clone(), cfg: cfg.clone(), opts: opts.clone() } + .into(), + ), BodyDiagnostic::MacroError { node, message } => sink.push(MacroError { file: node.file_id, node: node.value.clone().into(), diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs index d4fae05a6..075dcc6d2 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -88,39 +88,6 @@ mod m { ); } -#[test] -fn cfg_diagnostics() { - check_diagnostics( - r" -fn f() { - // The three g̶e̶n̶d̶e̶r̶s̶ statements: - - #[cfg(a)] fn f() {} // Item statement - //^^^^^^^^^^^^^^^^^^^ InactiveCode - #[cfg(a)] {} // Expression statement - //^^^^^^^^^^^^ InactiveCode - #[cfg(a)] let x = 0; // let statement - //^^^^^^^^^^^^^^^^^^^^ InactiveCode - - abc(#[cfg(a)] 0); - //^^^^^^^^^^^ InactiveCode - let x = Struct { - #[cfg(a)] f: 0, - //^^^^^^^^^^^^^^ InactiveCode - }; - match () { - () => (), - #[cfg(a)] () => (), - //^^^^^^^^^^^^^^^^^^ InactiveCode - } - - #[cfg(a)] 0 // Trailing expression of block - //^^^^^^^^^^^ InactiveCode -} - ", - ); -} - #[test] fn macro_diag_builtin() { check_diagnostics( diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index 5a088b6e5..f1ee03d4d 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -12,48 +12,6 @@ fn check_no_diagnostics(ra_fixture: &str) { db.check_no_diagnostics(); } -#[test] -fn inactive_item() { - // Additional tests in `cfg` crate. This only tests disabled cfgs. - - check_diagnostics( - r#" - //- /lib.rs - #[cfg(no)] pub fn f() {} - //^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - - #[cfg(no)] #[cfg(no2)] mod m; - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - - #[cfg(all(not(a), b))] enum E {} - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - - #[cfg(feature = "std")] use std; - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - "#, - ); -} - -/// Tests that `cfg` attributes behind `cfg_attr` is handled properly. -#[test] -fn inactive_via_cfg_attr() { - cov_mark::check!(cfg_attr_active); - check_diagnostics( - r#" - //- /lib.rs - #[cfg_attr(not(never), cfg(no))] fn f() {} - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - - #[cfg_attr(not(never), cfg(not(no)))] fn f() {} - - #[cfg_attr(never, cfg(no))] fn g() {} - - #[cfg_attr(not(never), inline, cfg(no))] fn h() {} - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UnconfiguredCode - "#, - ); -} - #[test] fn builtin_macro_fails_expansion() { check_diagnostics( diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 1a4800832..634e6a043 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -8,6 +8,7 @@ mod unresolved_module; mod unresolved_extern_crate; mod unresolved_import; mod unresolved_macro_call; +mod inactive_code; mod missing_fields; mod fixes; @@ -164,22 +165,6 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) - .on::(|d| { - // If there's inactive code somewhere in a macro, don't propagate to the call-site. - if d.display_source().file_id.expansion_info(db).is_some() { - return; - } - - // Override severity and mark as unused. - res.borrow_mut().push( - Diagnostic::hint( - sema.diagnostics_display_range(d.display_source()).range, - d.message(), - ) - .with_unused(true) - .with_code(Some(d.code())), - ); - }) .on::(|d| { // Limit diagnostic to the first few characters in the file. This matches how VS Code // renders it with the full span, but on other editors, and is less invasive. @@ -247,6 +232,11 @@ pub(crate) fn diagnostics( AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), + + AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { + Some(it) => it, + None => continue, + } }; if let Some(code) = d.code { if ctx.config.disabled.contains(code.as_str()) { @@ -451,7 +441,13 @@ mod tests { expect.assert_debug_eq(&diagnostics) } + #[track_caller] pub(crate) fn check_diagnostics(ra_fixture: &str) { + check_diagnostics_with_inactive_code(ra_fixture, false) + } + + #[track_caller] + pub(crate) fn check_diagnostics_with_inactive_code(ra_fixture: &str, with_inactive_code: bool) { let (analysis, file_id) = fixture::file(ra_fixture); let diagnostics = analysis .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) @@ -460,7 +456,7 @@ mod tests { let expected = extract_annotations(&*analysis.file_text(file_id).unwrap()); let mut actual = diagnostics .into_iter() - .filter(|d| d.code != Some(DiagnosticCode("inactive-code"))) + .filter(|d| d.code != Some(DiagnosticCode("inactive-code")) || with_inactive_code) .map(|d| (d.range, d.message)) .collect::>(); actual.sort_by_key(|(range, _)| range.start()); diff --git a/crates/ide/src/diagnostics/inactive_code.rs b/crates/ide/src/diagnostics/inactive_code.rs new file mode 100644 index 000000000..52f97cb4c --- /dev/null +++ b/crates/ide/src/diagnostics/inactive_code.rs @@ -0,0 +1,113 @@ +use cfg::DnfExpr; +use stdx::format_to; + +use crate::diagnostics::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: inactive-code +// +// This diagnostic is shown for code with inactive `#[cfg]` attributes. +pub(super) fn inactive_code( + ctx: &DiagnosticsContext<'_>, + d: &hir::InactiveCode, +) -> Option { + // If there's inactive code somewhere in a macro, don't propagate to the call-site. + if d.node.file_id.expansion_info(ctx.sema.db).is_some() { + return None; + } + + let inactive = DnfExpr::new(d.cfg.clone()).why_inactive(&d.opts); + let mut message = "code is inactive due to #[cfg] directives".to_string(); + + if let Some(inactive) = inactive { + format_to!(message, ": {}", inactive); + } + + let res = Diagnostic::new( + "inactive-code", + message, + ctx.sema.diagnostics_display_range(d.node.clone()).range, + ) + .with_unused(true); + Some(res) +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_diagnostics_with_inactive_code; + + #[test] + fn cfg_diagnostics() { + check_diagnostics_with_inactive_code( + r#" +fn f() { + // The three g̶e̶n̶d̶e̶r̶s̶ statements: + + #[cfg(a)] fn f() {} // Item statement + //^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + #[cfg(a)] {} // Expression statement + //^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + #[cfg(a)] let x = 0; // let statement + //^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + + abc(#[cfg(a)] 0); + //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + let x = Struct { + #[cfg(a)] f: 0, + //^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + }; + match () { + () => (), + #[cfg(a)] () => (), + //^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + } + + #[cfg(a)] 0 // Trailing expression of block + //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled +} + "#, + true, + ); + } + + #[test] + fn inactive_item() { + // Additional tests in `cfg` crate. This only tests disabled cfgs. + + check_diagnostics_with_inactive_code( + r#" + #[cfg(no)] pub fn f() {} + //^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled + + #[cfg(no)] #[cfg(no2)] mod m; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no and no2 are disabled + + #[cfg(all(not(a), b))] enum E {} + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: b is disabled + + #[cfg(feature = "std")] use std; + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: feature = "std" is disabled +"#, + true, + ); + } + + /// Tests that `cfg` attributes behind `cfg_attr` is handled properly. + #[test] + fn inactive_via_cfg_attr() { + cov_mark::check!(cfg_attr_active); + check_diagnostics_with_inactive_code( + r#" + #[cfg_attr(not(never), cfg(no))] fn f() {} + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled + + #[cfg_attr(not(never), cfg(not(no)))] fn f() {} + + #[cfg_attr(never, cfg(no))] fn g() {} + + #[cfg_attr(not(never), inline, cfg(no))] fn h() {} + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled +"#, + true, + ); + } +} -- cgit v1.2.3