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 (limited to 'crates') 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 From 1e4aaee7bbc1d56698e70158aa35f578422623d9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 17:51:44 +0300 Subject: internal: refactor unresolved proc macro diagnostic --- crates/hir/src/diagnostics.rs | 34 ++-------------------- crates/hir/src/lib.rs | 33 ++++++++++----------- crates/ide/src/diagnostics.rs | 17 +++++------ crates/ide/src/diagnostics/inactive_code.rs | 6 +++- .../ide/src/diagnostics/unresolved_proc_macro.rs | 30 +++++++++++++++++++ 5 files changed, 59 insertions(+), 61 deletions(-) create mode 100644 crates/ide/src/diagnostics/unresolved_proc_macro.rs (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 03e7f5e84..2039d2b43 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -36,6 +36,7 @@ diagnostics![ UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, + UnresolvedProcMacro, MissingFields, InactiveCode, ]; @@ -69,46 +70,15 @@ pub struct InactiveCode { pub opts: CfgOptions, } -// Diagnostic: unresolved-proc-macro -// -// This diagnostic is shown when a procedural macro can not be found. This usually means that -// procedural macro support is simply disabled (and hence is only a weak hint instead of an error), -// but can also indicate project setup problems. -// -// If you are seeing a lot of "proc macro not expanded" warnings, you can add this option to the -// `rust-analyzer.diagnostics.disabled` list to prevent them from showing. Alternatively you can -// enable support for procedural macros (see `rust-analyzer.procMacro.enable`). #[derive(Debug, Clone, Eq, PartialEq)] pub struct UnresolvedProcMacro { - pub file: HirFileId, - pub node: SyntaxNodePtr, + pub node: InFile, /// If the diagnostic can be pinpointed more accurately than via `node`, this is the `TextRange` /// to use instead. pub precise_location: Option, pub macro_name: Option, } -impl Diagnostic for UnresolvedProcMacro { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("unresolved-proc-macro") - } - - fn message(&self) -> String { - match &self.macro_name { - Some(name) => format!("proc macro `{}` not expanded", name), - None => "proc macro not expanded".to_string(), - } - } - - fn display_source(&self) -> InFile { - InFile::new(self.file, self.node.clone()) - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - // Diagnostic: macro-error // // This diagnostic is shown for macro expansion errors. diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d59b52b25..87a3db946 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -518,10 +518,10 @@ impl Module { DefDiagnosticKind::UnresolvedProcMacro { ast } => { let mut precise_location = None; - let (file, ast, name) = match ast { + let (node, name) = match ast { MacroCallKind::FnLike { ast_id, .. } => { let node = ast_id.to_node(db.upcast()); - (ast_id.file_id, SyntaxNodePtr::from(AstPtr::new(&node)), None) + (ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), None) } MacroCallKind::Derive { ast_id, derive_name, .. } => { let node = ast_id.to_node(db.upcast()); @@ -554,8 +554,7 @@ impl Module { } ( - ast_id.file_id, - SyntaxNodePtr::from(AstPtr::new(&node)), + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))), Some(derive_name.clone()), ) } @@ -566,18 +565,14 @@ impl Module { || panic!("cannot find attribute #{}", invoc_attr_index), ); ( - ast_id.file_id, - SyntaxNodePtr::from(AstPtr::new(&attr)), + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))), Some(attr_name.clone()), ) } }; - sink.push(UnresolvedProcMacro { - file, - node: ast, - precise_location, - macro_name: name, - }); + acc.push( + UnresolvedProcMacro { node, precise_location, macro_name: name }.into(), + ); } DefDiagnosticKind::UnresolvedMacroCall { ast, path } => { @@ -1056,12 +1051,14 @@ impl Function { node: node.value.clone().into(), message: message.to_string(), }), - BodyDiagnostic::UnresolvedProcMacro { node } => sink.push(UnresolvedProcMacro { - file: node.file_id, - node: node.value.clone().into(), - precise_location: None, - macro_name: None, - }), + BodyDiagnostic::UnresolvedProcMacro { node } => acc.push( + UnresolvedProcMacro { + node: node.clone().map(|it| it.into()), + precise_location: None, + macro_name: None, + } + .into(), + ), BodyDiagnostic::UnresolvedMacroCall { node, path } => acc.push( UnresolvedMacroCall { macro_call: node.clone(), path: path.clone() }.into(), ), diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 634e6a043..f7965326d 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 unresolved_proc_macro; mod inactive_code; mod missing_fields; @@ -68,6 +69,11 @@ impl Diagnostic { self } + fn severity(mut self, severity: Severity) -> Diagnostic { + self.severity = severity; + self + } + fn error(range: TextRange, message: String) -> Self { Self { message, @@ -178,16 +184,6 @@ pub(crate) fn diagnostics( .with_code(Some(d.code())), ); }) - .on::(|d| { - // Use more accurate position if available. - let display_range = d - .precise_location - .unwrap_or_else(|| sema.diagnostics_display_range(d.display_source()).range); - - // FIXME: it would be nice to tell the user whether proc macros are currently disabled - res.borrow_mut() - .push(Diagnostic::hint(display_range, d.message()).with_code(Some(d.code()))); - }) .on::(|d| { let display_range = sema.diagnostics_display_range(d.display_source()).range; res.borrow_mut() @@ -231,6 +227,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), + AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { diff --git a/crates/ide/src/diagnostics/inactive_code.rs b/crates/ide/src/diagnostics/inactive_code.rs index 52f97cb4c..afe333204 100644 --- a/crates/ide/src/diagnostics/inactive_code.rs +++ b/crates/ide/src/diagnostics/inactive_code.rs @@ -1,7 +1,10 @@ use cfg::DnfExpr; use stdx::format_to; -use crate::diagnostics::{Diagnostic, DiagnosticsContext}; +use crate::{ + diagnostics::{Diagnostic, DiagnosticsContext}, + Severity, +}; // Diagnostic: inactive-code // @@ -27,6 +30,7 @@ pub(super) fn inactive_code( message, ctx.sema.diagnostics_display_range(d.node.clone()).range, ) + .severity(Severity::WeakWarning) .with_unused(true); Some(res) } diff --git a/crates/ide/src/diagnostics/unresolved_proc_macro.rs b/crates/ide/src/diagnostics/unresolved_proc_macro.rs new file mode 100644 index 000000000..3dc6ab451 --- /dev/null +++ b/crates/ide/src/diagnostics/unresolved_proc_macro.rs @@ -0,0 +1,30 @@ +use crate::{ + diagnostics::{Diagnostic, DiagnosticsContext}, + Severity, +}; + +// Diagnostic: unresolved-proc-macro +// +// This diagnostic is shown when a procedural macro can not be found. This usually means that +// procedural macro support is simply disabled (and hence is only a weak hint instead of an error), +// but can also indicate project setup problems. +// +// If you are seeing a lot of "proc macro not expanded" warnings, you can add this option to the +// `rust-analyzer.diagnostics.disabled` list to prevent them from showing. Alternatively you can +// enable support for procedural macros (see `rust-analyzer.procMacro.enable`). +pub(super) fn unresolved_proc_macro( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedProcMacro, +) -> Diagnostic { + // Use more accurate position if available. + let display_range = d + .precise_location + .unwrap_or_else(|| ctx.sema.diagnostics_display_range(d.node.clone()).range); + // FIXME: it would be nice to tell the user whether proc macros are currently disabled + let message = match &d.macro_name { + Some(name) => format!("proc macro `{}` not expanded", name), + None => "proc macro not expanded".to_string(), + }; + + Diagnostic::new("unresolved-proc-macro", message, display_range).severity(Severity::WeakWarning) +} -- cgit v1.2.3 From 00303284b5cc3a82e32dc3ecbbcfeb2f99de6818 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 18:41:04 +0300 Subject: internal: refactor macro error --- crates/hir/src/diagnostics.rs | 26 +--- crates/hir/src/lib.rs | 20 +-- crates/hir_def/src/body/tests.rs | 88 ------------- crates/hir_def/src/nameres/tests.rs | 1 - crates/hir_def/src/nameres/tests/diagnostics.rs | 76 ----------- crates/ide/src/diagnostics.rs | 2 + crates/ide/src/diagnostics/macro_error.rs | 163 ++++++++++++++++++++++++ 7 files changed, 178 insertions(+), 198 deletions(-) delete mode 100644 crates/hir_def/src/nameres/tests/diagnostics.rs create mode 100644 crates/ide/src/diagnostics/macro_error.rs (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 2039d2b43..28580eeb4 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -37,6 +37,7 @@ diagnostics![ UnresolvedImport, UnresolvedMacroCall, UnresolvedProcMacro, + MacroError, MissingFields, InactiveCode, ]; @@ -79,35 +80,12 @@ pub struct UnresolvedProcMacro { pub macro_name: Option, } -// Diagnostic: macro-error -// -// This diagnostic is shown for macro expansion errors. #[derive(Debug, Clone, Eq, PartialEq)] pub struct MacroError { - pub file: HirFileId, - pub node: SyntaxNodePtr, + pub node: InFile, pub message: String, } -impl Diagnostic for MacroError { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("macro-error") - } - fn message(&self) -> String { - self.message.clone() - } - fn display_source(&self) -> InFile { - InFile::new(self.file, self.node.clone()) - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - fn is_experimental(&self) -> bool { - // Newly added and not very well-tested, might contain false positives. - true - } -} - #[derive(Debug)] pub struct UnimplementedBuiltinMacro { pub file: HirFileId, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 87a3db946..d891d0ec1 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -587,19 +587,19 @@ impl Module { } DefDiagnosticKind::MacroError { ast, message } => { - let (file, ast) = match ast { + let node = match ast { MacroCallKind::FnLike { ast_id, .. } => { let node = ast_id.to_node(db.upcast()); - (ast_id.file_id, SyntaxNodePtr::from(AstPtr::new(&node))) + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))) } MacroCallKind::Derive { ast_id, .. } | MacroCallKind::Attr { ast_id, .. } => { // FIXME: point to the attribute instead, this creates very large diagnostics let node = ast_id.to_node(db.upcast()); - (ast_id.file_id, SyntaxNodePtr::from(AstPtr::new(&node))) + ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node))) } }; - sink.push(MacroError { file, node: ast, message: message.clone() }); + acc.push(MacroError { node, message: message.clone() }.into()); } DefDiagnosticKind::UnimplementedBuiltinMacro { ast } => { @@ -1046,11 +1046,13 @@ impl Function { 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(), - message: message.to_string(), - }), + BodyDiagnostic::MacroError { node, message } => acc.push( + MacroError { + node: node.clone().map(|it| it.into()), + message: message.to_string(), + } + .into(), + ), BodyDiagnostic::UnresolvedProcMacro { node } => acc.push( UnresolvedProcMacro { node: node.clone().map(|it| it.into()), diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs index 075dcc6d2..0dccabcfd 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -88,67 +88,6 @@ mod m { ); } -#[test] -fn macro_diag_builtin() { - check_diagnostics( - r#" -#[rustc_builtin_macro] -macro_rules! env {} - -#[rustc_builtin_macro] -macro_rules! include {} - -#[rustc_builtin_macro] -macro_rules! compile_error {} - -#[rustc_builtin_macro] -macro_rules! format_args { - () => {} -} - -fn f() { - // Test a handful of built-in (eager) macros: - - include!(invalid); - //^^^^^^^^^^^^^^^^^ could not convert tokens - include!("does not exist"); - //^^^^^^^^^^^^^^^^^^^^^^^^^^ failed to load file `does not exist` - - env!(invalid); - //^^^^^^^^^^^^^ could not convert tokens - - env!("OUT_DIR"); - //^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "run build scripts" to fix - - compile_error!("compile_error works"); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ compile_error works - - // Lazy: - - format_args!(); - //^^^^^^^^^^^^^^ no rule matches input tokens -} - "#, - ); -} - -#[test] -fn macro_rules_diag() { - check_diagnostics( - r#" -macro_rules! m { - () => {}; -} -fn f() { - m!(); - - m!(hi); - //^^^^^^ leftover tokens -} - "#, - ); -} - #[test] fn unresolved_macro_diag() { check_diagnostics( @@ -161,30 +100,3 @@ fn f() { ); } -#[test] -fn dollar_crate_in_builtin_macro() { - check_diagnostics( - r#" -#[macro_export] -#[rustc_builtin_macro] -macro_rules! format_args {} - -#[macro_export] -macro_rules! arg { - () => {} -} - -#[macro_export] -macro_rules! outer { - () => { - $crate::format_args!( "", $crate::arg!(1) ) - }; -} - -fn f() { - outer!(); - //^^^^^^^^ leftover tokens -} - "#, - ) -} diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs index 58c01354a..cf43f2a96 100644 --- a/crates/hir_def/src/nameres/tests.rs +++ b/crates/hir_def/src/nameres/tests.rs @@ -2,7 +2,6 @@ mod globs; mod incremental; mod macros; mod mod_resolution; -mod diagnostics; mod primitives; use std::sync::Arc; diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs deleted file mode 100644 index f1ee03d4d..000000000 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ /dev/null @@ -1,76 +0,0 @@ -use base_db::fixture::WithFixture; - -use crate::test_db::TestDB; - -fn check_diagnostics(ra_fixture: &str) { - let db: TestDB = TestDB::with_files(ra_fixture); - db.check_diagnostics(); -} - -fn check_no_diagnostics(ra_fixture: &str) { - let db: TestDB = TestDB::with_files(ra_fixture); - db.check_no_diagnostics(); -} - -#[test] -fn builtin_macro_fails_expansion() { - check_diagnostics( - r#" - //- /lib.rs - #[rustc_builtin_macro] - macro_rules! include { () => {} } - - include!("doesntexist"); - //^^^^^^^^^^^^^^^^^^^^^^^^ failed to load file `doesntexist` - "#, - ); -} - -#[test] -fn include_macro_should_allow_empty_content() { - check_no_diagnostics( - r#" - //- /lib.rs - #[rustc_builtin_macro] - macro_rules! include { () => {} } - - include!("bar.rs"); - //- /bar.rs - // empty - "#, - ); -} - -#[test] -fn good_out_dir_diagnostic() { - check_diagnostics( - r#" - #[rustc_builtin_macro] - macro_rules! include { () => {} } - #[rustc_builtin_macro] - macro_rules! env { () => {} } - #[rustc_builtin_macro] - macro_rules! concat { () => {} } - - include!(concat!(env!("OUT_DIR"), "/out.rs")); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "run build scripts" to fix - "#, - ); -} - -#[test] -fn register_attr_and_tool() { - cov_mark::check!(register_attr); - cov_mark::check!(register_tool); - check_no_diagnostics( - r#" -#![register_tool(tool)] -#![register_attr(attr)] - -#[tool::path] -#[attr] -struct S; - "#, - ); - // NB: we don't currently emit diagnostics here -} diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index f7965326d..c257ea8e7 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -9,6 +9,7 @@ mod unresolved_extern_crate; mod unresolved_import; mod unresolved_macro_call; mod unresolved_proc_macro; +mod macro_error; mod inactive_code; mod missing_fields; @@ -229,6 +230,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), + AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { Some(it) => it, diff --git a/crates/ide/src/diagnostics/macro_error.rs b/crates/ide/src/diagnostics/macro_error.rs new file mode 100644 index 000000000..8cc8cfb48 --- /dev/null +++ b/crates/ide/src/diagnostics/macro_error.rs @@ -0,0 +1,163 @@ +use crate::diagnostics::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: macro-error +// +// This diagnostic is shown for macro expansion errors. +pub(super) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> Diagnostic { + Diagnostic::new( + "macro-error", + d.message.clone(), + ctx.sema.diagnostics_display_range(d.node.clone()).range, + ) + .experimental() +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_diagnostics, check_no_diagnostics}; + + #[test] + fn builtin_macro_fails_expansion() { + check_diagnostics( + r#" +#[rustc_builtin_macro] +macro_rules! include { () => {} } + + include!("doesntexist"); +//^^^^^^^^^^^^^^^^^^^^^^^^ failed to load file `doesntexist` + "#, + ); + } + + #[test] + fn include_macro_should_allow_empty_content() { + check_diagnostics( + r#" +//- /lib.rs +#[rustc_builtin_macro] +macro_rules! include { () => {} } + +include!("foo/bar.rs"); +//- /foo/bar.rs +// empty +"#, + ); + } + + #[test] + fn good_out_dir_diagnostic() { + check_diagnostics( + r#" +#[rustc_builtin_macro] +macro_rules! include { () => {} } +#[rustc_builtin_macro] +macro_rules! env { () => {} } +#[rustc_builtin_macro] +macro_rules! concat { () => {} } + + include!(concat!(env!("OUT_DIR"), "/out.rs")); +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "run build scripts" to fix +"#, + ); + } + + #[test] + fn register_attr_and_tool() { + cov_mark::check!(register_attr); + cov_mark::check!(register_tool); + check_no_diagnostics( + r#" +#![register_tool(tool)] +#![register_attr(attr)] + +#[tool::path] +#[attr] +struct S; +"#, + ); + // NB: we don't currently emit diagnostics here + } + + #[test] + fn macro_diag_builtin() { + check_diagnostics( + r#" +#[rustc_builtin_macro] +macro_rules! env {} + +#[rustc_builtin_macro] +macro_rules! include {} + +#[rustc_builtin_macro] +macro_rules! compile_error {} + +#[rustc_builtin_macro] +macro_rules! format_args { () => {} } + +fn main() { + // Test a handful of built-in (eager) macros: + + include!(invalid); + //^^^^^^^^^^^^^^^^^ could not convert tokens + include!("does not exist"); + //^^^^^^^^^^^^^^^^^^^^^^^^^^ failed to load file `does not exist` + + env!(invalid); + //^^^^^^^^^^^^^ could not convert tokens + + env!("OUT_DIR"); + //^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "run build scripts" to fix + + compile_error!("compile_error works"); + //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ compile_error works + + // Lazy: + + format_args!(); + //^^^^^^^^^^^^^^ no rule matches input tokens +} +"#, + ); + } + + #[test] + fn macro_rules_diag() { + check_diagnostics( + r#" +macro_rules! m { + () => {}; +} +fn f() { + m!(); + + m!(hi); + //^^^^^^ leftover tokens +} + "#, + ); + } + #[test] + fn dollar_crate_in_builtin_macro() { + check_diagnostics( + r#" +#[macro_export] +#[rustc_builtin_macro] +macro_rules! format_args {} + +#[macro_export] +macro_rules! arg { () => {} } + +#[macro_export] +macro_rules! outer { + () => { + $crate::format_args!( "", $crate::arg!(1) ) + }; +} + +fn f() { + outer!(); +} //^^^^^^^^ leftover tokens +"#, + ) + } +} -- cgit v1.2.3 From 4af7a35197a1cb159458694e69e17bd83dc9edff Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 18:45:38 +0300 Subject: internal: remove def-level diagnostics tests --- crates/hir_def/src/body/tests.rs | 28 +--- crates/hir_def/src/test_db.rs | 152 +-------------------- .../ide/src/diagnostics/unresolved_macro_call.rs | 12 ++ 3 files changed, 21 insertions(+), 171 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs index 0dccabcfd..27d837d47 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -3,7 +3,7 @@ mod block; use base_db::{fixture::WithFixture, SourceDatabase}; use expect_test::Expect; -use crate::{test_db::TestDB, ModuleDefId}; +use crate::ModuleDefId; use super::*; @@ -28,11 +28,6 @@ fn lower(ra_fixture: &str) -> Arc { db.body(fn_def.unwrap().into()) } -fn check_diagnostics(ra_fixture: &str) { - let db: TestDB = TestDB::with_files(ra_fixture); - db.check_diagnostics(); -} - fn block_def_map_at(ra_fixture: &str) -> String { let (db, position) = crate::test_db::TestDB::with_position(ra_fixture); @@ -57,7 +52,7 @@ fn check_at(ra_fixture: &str, expect: Expect) { fn your_stack_belongs_to_me() { cov_mark::check!(your_stack_belongs_to_me); lower( - " + r#" macro_rules! n_nuple { ($e:tt) => (); ($($rest:tt)*) => {{ @@ -65,7 +60,7 @@ macro_rules! n_nuple { }}; } fn main() { n_nuple!(1,2,3); } -", +"#, ); } @@ -73,7 +68,7 @@ fn main() { n_nuple!(1,2,3); } fn macro_resolve() { // Regression test for a path resolution bug introduced with inner item handling. lower( - r" + r#" macro_rules! vec { () => { () }; ($elem:expr; $n:expr) => { () }; @@ -84,19 +79,6 @@ mod m { let _ = vec![FileSet::default(); self.len()]; } } - ", +"#, ); } - -#[test] -fn unresolved_macro_diag() { - check_diagnostics( - r#" -fn f() { - m!(); - //^^^^ UnresolvedMacroCall -} - "#, - ); -} - diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index a16203fdb..2635b556e 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -6,19 +6,16 @@ use std::{ }; use base_db::{ - salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, FilePosition, FileRange, Upcast, + salsa, AnchoredPath, CrateId, FileId, FileLoader, FileLoaderDelegate, FilePosition, + SourceDatabase, Upcast, }; -use base_db::{AnchoredPath, SourceDatabase}; use hir_expand::{db::AstDatabase, InFile}; -use rustc_hash::FxHashMap; use rustc_hash::FxHashSet; -use syntax::{algo, ast, AstNode, SyntaxNode, SyntaxNodePtr, TextRange, TextSize}; -use test_utils::extract_annotations; +use syntax::{algo, ast, AstNode}; use crate::{ - body::BodyDiagnostic, db::DefDatabase, - nameres::{diagnostics::DefDiagnosticKind, DefMap, ModuleSource}, + nameres::{DefMap, ModuleSource}, src::HasSource, LocalModuleId, Lookup, ModuleDefId, ModuleId, }; @@ -245,145 +242,4 @@ impl TestDB { }) .collect() } - - pub(crate) fn extract_annotations(&self) -> FxHashMap> { - let mut files = Vec::new(); - let crate_graph = self.crate_graph(); - for krate in crate_graph.iter() { - let crate_def_map = self.crate_def_map(krate); - for (module_id, _) in crate_def_map.modules() { - let file_id = crate_def_map[module_id].origin.file_id(); - files.extend(file_id) - } - } - assert!(!files.is_empty()); - files - .into_iter() - .filter_map(|file_id| { - let text = self.file_text(file_id); - let annotations = extract_annotations(&text); - if annotations.is_empty() { - return None; - } - Some((file_id, annotations)) - }) - .collect() - } - - pub(crate) fn diagnostics(&self, cb: &mut dyn FnMut(FileRange, String)) { - let crate_graph = self.crate_graph(); - for krate in crate_graph.iter() { - let crate_def_map = self.crate_def_map(krate); - - for diag in crate_def_map.diagnostics() { - let (node, message): (InFile, &str) = match &diag.kind { - DefDiagnosticKind::UnresolvedModule { ast, .. } => { - let node = ast.to_node(self.upcast()); - (InFile::new(ast.file_id, node.syntax().clone()), "UnresolvedModule") - } - DefDiagnosticKind::UnresolvedExternCrate { ast, .. } => { - let node = ast.to_node(self.upcast()); - (InFile::new(ast.file_id, node.syntax().clone()), "UnresolvedExternCrate") - } - DefDiagnosticKind::UnresolvedImport { id, .. } => { - let item_tree = id.item_tree(self.upcast()); - let import = &item_tree[id.value]; - let node = InFile::new(id.file_id(), import.ast_id).to_node(self.upcast()); - (InFile::new(id.file_id(), node.syntax().clone()), "UnresolvedImport") - } - DefDiagnosticKind::UnconfiguredCode { ast, .. } => { - let node = ast.to_node(self.upcast()); - (InFile::new(ast.file_id, node.syntax().clone()), "UnconfiguredCode") - } - DefDiagnosticKind::UnresolvedProcMacro { ast, .. } => { - (ast.to_node(self.upcast()), "UnresolvedProcMacro") - } - DefDiagnosticKind::UnresolvedMacroCall { ast, .. } => { - let node = ast.to_node(self.upcast()); - (InFile::new(ast.file_id, node.syntax().clone()), "UnresolvedMacroCall") - } - DefDiagnosticKind::MacroError { ast, message } => { - (ast.to_node(self.upcast()), message.as_str()) - } - DefDiagnosticKind::UnimplementedBuiltinMacro { ast } => { - let node = ast.to_node(self.upcast()); - ( - InFile::new(ast.file_id, node.syntax().clone()), - "UnimplementedBuiltinMacro", - ) - } - }; - - let frange = node.as_ref().original_file_range(self); - cb(frange, message.to_string()) - } - - for (_module_id, module) in crate_def_map.modules() { - for decl in module.scope.declarations() { - if let ModuleDefId::FunctionId(it) = decl { - let source_map = self.body_with_source_map(it.into()).1; - for diag in source_map.diagnostics() { - let (ptr, message): (InFile, &str) = match diag { - BodyDiagnostic::InactiveCode { node, .. } => { - (node.clone().map(|it| it), "InactiveCode") - } - BodyDiagnostic::MacroError { node, message } => { - (node.clone().map(|it| it.into()), message.as_str()) - } - BodyDiagnostic::UnresolvedProcMacro { node } => { - (node.clone().map(|it| it.into()), "UnresolvedProcMacro") - } - BodyDiagnostic::UnresolvedMacroCall { node, .. } => { - (node.clone().map(|it| it.into()), "UnresolvedMacroCall") - } - }; - - let root = self.parse_or_expand(ptr.file_id).unwrap(); - let node = ptr.map(|ptr| ptr.to_node(&root)); - let frange = node.as_ref().original_file_range(self); - cb(frange, message.to_string()) - } - } - } - } - } - } - - pub(crate) fn check_diagnostics(&self) { - let db: &TestDB = self; - let annotations = db.extract_annotations(); - assert!(!annotations.is_empty()); - - let mut actual: FxHashMap> = FxHashMap::default(); - db.diagnostics(&mut |frange, message| { - actual.entry(frange.file_id).or_default().push((frange.range, message)); - }); - - for (file_id, diags) in actual.iter_mut() { - diags.sort_by_key(|it| it.0.start()); - let text = db.file_text(*file_id); - // For multiline spans, place them on line start - for (range, content) in diags { - if text[*range].contains('\n') { - *range = TextRange::new(range.start(), range.start() + TextSize::from(1)); - *content = format!("... {}", content); - } - } - } - - assert_eq!(annotations, actual); - } - - pub(crate) fn check_no_diagnostics(&self) { - let db: &TestDB = self; - let annotations = db.extract_annotations(); - assert!(annotations.is_empty()); - - let mut has_diagnostics = false; - db.diagnostics(&mut |_, _| { - has_diagnostics = true; - }); - - assert!(!has_diagnostics); - } } diff --git a/crates/ide/src/diagnostics/unresolved_macro_call.rs b/crates/ide/src/diagnostics/unresolved_macro_call.rs index a3af332a4..15b6a2730 100644 --- a/crates/ide/src/diagnostics/unresolved_macro_call.rs +++ b/crates/ide/src/diagnostics/unresolved_macro_call.rs @@ -34,6 +34,18 @@ pub(super) fn unresolved_macro_call( mod tests { use crate::diagnostics::tests::check_diagnostics; + #[test] + fn unresolved_macro_diag() { + check_diagnostics( + r#" +fn f() { + m!(); +} //^ unresolved macro `m!` + +"#, + ); + } + #[test] fn test_unresolved_macro_range() { check_diagnostics( -- cgit v1.2.3