From 6d104de15aee6a24a442871c59528c39d410c161 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 16:42:34 +0300 Subject: internal: refactor unresolved import diagnostic --- crates/hir/src/diagnostics.rs | 27 +------- crates/hir/src/lib.rs | 5 +- crates/hir_def/src/nameres/tests/diagnostics.rs | 37 ---------- crates/ide/src/diagnostics.rs | 59 ++++++++-------- crates/ide/src/diagnostics/unresolved_import.rs | 90 +++++++++++++++++++++++++ crates/ide/src/diagnostics/unresolved_module.rs | 1 + 6 files changed, 127 insertions(+), 92 deletions(-) create mode 100644 crates/ide/src/diagnostics/unresolved_import.rs (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index ec0a8fe41..70a4d000d 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -32,7 +32,7 @@ macro_rules! diagnostics { }; } -diagnostics![UnresolvedModule, UnresolvedExternCrate, MissingFields]; +diagnostics![UnresolvedModule, UnresolvedExternCrate, UnresolvedImport, MissingFields]; #[derive(Debug)] pub struct UnresolvedModule { @@ -47,30 +47,7 @@ pub struct UnresolvedExternCrate { #[derive(Debug)] pub struct UnresolvedImport { - pub file: HirFileId, - pub node: AstPtr, -} - -impl Diagnostic for UnresolvedImport { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("unresolved-import") - } - fn message(&self) -> String { - "unresolved import".to_string() - } - fn display_source(&self) -> InFile { - InFile::new(self.file, self.node.clone().into()) - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - fn is_experimental(&self) -> bool { - // This currently results in false positives in the following cases: - // - `cfg_if!`-generated code in libstd (we don't load the sysroot correctly) - // - `core::arch` (we don't handle `#[path = "../"]` correctly) - // - proc macros and/or proc macro generated code - true - } + pub decl: InFile>, } // Diagnostic: unresolved-macro-call diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index f7883c469..d32246709 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -498,7 +498,10 @@ impl Module { let import = &item_tree[id.value]; let use_tree = import.use_tree_to_ast(db.upcast(), file_id, *index); - sink.push(UnresolvedImport { file: file_id, node: AstPtr::new(&use_tree) }); + acc.push( + UnresolvedImport { decl: InFile::new(file_id, AstPtr::new(&use_tree)) } + .into(), + ); } DefDiagnosticKind::UnconfiguredCode { ast, cfg, opts } => { diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index 84d6fdc93..7b0f42004 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -12,43 +12,6 @@ fn check_no_diagnostics(ra_fixture: &str) { db.check_no_diagnostics(); } -#[test] -fn unresolved_import() { - check_diagnostics( - r" - use does_exist; - use does_not_exist; - //^^^^^^^^^^^^^^^^^^^ UnresolvedImport - - mod does_exist {} - ", - ); -} - -#[test] -fn dedup_unresolved_import_from_unresolved_crate() { - check_diagnostics( - r" - //- /main.rs crate:main - mod a { - extern crate doesnotexist; - //^^^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedExternCrate - - // Should not error, since we already errored for the missing crate. - use doesnotexist::{self, bla, *}; - - use crate::doesnotexist; - //^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedImport - } - - mod m { - use super::doesnotexist; - //^^^^^^^^^^^^^^^^^^^^^^^^ UnresolvedImport - } - ", - ); -} - #[test] fn inactive_item() { // Additional tests in `cfg` crate. This only tests disabled cfgs. diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 1fbb7131d..3d05dd093 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -6,6 +6,7 @@ mod unresolved_module; mod unresolved_extern_crate; +mod unresolved_import; mod missing_fields; mod fixes; @@ -43,17 +44,39 @@ pub struct Diagnostic { pub fixes: Option>, pub unused: bool, pub code: Option, + pub experimental: bool, } impl Diagnostic { fn new(code: &'static str, message: impl Into, range: TextRange) -> Diagnostic { let message = message.into(); let code = Some(DiagnosticCode(code)); - Self { message, range, severity: Severity::Error, fixes: None, unused: false, code } + Self { + message, + range, + severity: Severity::Error, + fixes: None, + unused: false, + code, + experimental: false, + } + } + + fn experimental(mut self) -> Diagnostic { + self.experimental = true; + self } fn error(range: TextRange, message: String) -> Self { - Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None } + Self { + message, + range, + severity: Severity::Error, + fixes: None, + unused: false, + code: None, + experimental: false, + } } fn hint(range: TextRange, message: String) -> Self { @@ -64,6 +87,7 @@ impl Diagnostic { fixes: None, unused: false, code: None, + experimental: false, } } @@ -234,6 +258,7 @@ pub(crate) fn diagnostics( let d = match diag { AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), + AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), }; if let Some(code) = d.code { @@ -241,6 +266,9 @@ pub(crate) fn diagnostics( continue; } } + if ctx.config.disable_experimental && d.experimental { + continue; + } res.push(d) } @@ -462,33 +490,6 @@ foo::bar!(92); ); } - #[test] - fn unresolved_import_in_use_tree() { - // Only the relevant part of a nested `use` item should be highlighted. - check_diagnostics( - r#" -use does_exist::{Exists, DoesntExist}; - //^^^^^^^^^^^ unresolved import - -use {does_not_exist::*, does_exist}; - //^^^^^^^^^^^^^^^^^ unresolved import - -use does_not_exist::{ - a, - //^ unresolved import - b, - //^ unresolved import - c, - //^ unresolved import -}; - -mod does_exist { - pub struct Exists; -} -"#, - ); - } - #[test] fn range_mapping_out_of_macros() { // FIXME: this is very wrong, but somewhat tricky to fix. diff --git a/crates/ide/src/diagnostics/unresolved_import.rs b/crates/ide/src/diagnostics/unresolved_import.rs new file mode 100644 index 000000000..1cbf96ba1 --- /dev/null +++ b/crates/ide/src/diagnostics/unresolved_import.rs @@ -0,0 +1,90 @@ +use crate::diagnostics::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-import +// +// This diagnostic is triggered if rust-analyzer is unable to resolve a path in +// a `use` declaration. +pub(super) fn unresolved_import( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedImport, +) -> Diagnostic { + Diagnostic::new( + "unresolved-import", + "unresolved import", + ctx.sema.diagnostics_display_range(d.decl.clone().map(|it| it.into())).range, + ) + // This currently results in false positives in the following cases: + // - `cfg_if!`-generated code in libstd (we don't load the sysroot correctly) + // - `core::arch` (we don't handle `#[path = "../"]` correctly) + // - proc macros and/or proc macro generated code + .experimental() +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_diagnostics; + + #[test] + fn unresolved_import() { + check_diagnostics( + r#" +use does_exist; +use does_not_exist; + //^^^^^^^^^^^^^^ unresolved import + +mod does_exist {} +"#, + ); + } + + #[test] + fn unresolved_import_in_use_tree() { + // Only the relevant part of a nested `use` item should be highlighted. + check_diagnostics( + r#" +use does_exist::{Exists, DoesntExist}; + //^^^^^^^^^^^ unresolved import + +use {does_not_exist::*, does_exist}; + //^^^^^^^^^^^^^^^^^ unresolved import + +use does_not_exist::{ + a, + //^ unresolved import + b, + //^ unresolved import + c, + //^ unresolved import +}; + +mod does_exist { + pub struct Exists; +} +"#, + ); + } + + #[test] + fn dedup_unresolved_import_from_unresolved_crate() { + check_diagnostics( + r#" +//- /main.rs crate:main +mod a { + extern crate doesnotexist; + //^^^^^^^^^^^^^^^^^^^^^^^^^^ unresolved extern crate + + // Should not error, since we already errored for the missing crate. + use doesnotexist::{self, bla, *}; + + use crate::doesnotexist; + //^^^^^^^^^^^^^^^^^^^ unresolved import +} + +mod m { + use super::doesnotexist; + //^^^^^^^^^^^^^^^^^^^ unresolved import +} +"#, + ); + } +} diff --git a/crates/ide/src/diagnostics/unresolved_module.rs b/crates/ide/src/diagnostics/unresolved_module.rs index 4c8c74ff7..b1da8f0e1 100644 --- a/crates/ide/src/diagnostics/unresolved_module.rs +++ b/crates/ide/src/diagnostics/unresolved_module.rs @@ -104,6 +104,7 @@ mod baz {} "unresolved-module", ), ), + experimental: false, }, ] "#]], -- cgit v1.2.3 From fa9ed4e0ce633e51d1411951bf044719e6837457 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 17:06:36 +0300 Subject: internal: refactor unresolved macro call diagnostic --- crates/hir/src/diagnostics.rs | 35 +++-------- crates/hir/src/lib.rs | 22 +++---- crates/hir_def/src/nameres/tests/diagnostics.rs | 31 ---------- crates/ide/src/diagnostics.rs | 29 +-------- .../ide/src/diagnostics/unresolved_macro_call.rs | 72 ++++++++++++++++++++++ 5 files changed, 94 insertions(+), 95 deletions(-) create mode 100644 crates/ide/src/diagnostics/unresolved_macro_call.rs (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 70a4d000d..718c86b3a 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -17,7 +17,7 @@ pub use crate::diagnostics_sink::{ }; macro_rules! diagnostics { - ($($diag:ident),*) => { + ($($diag:ident,)*) => { pub enum AnyDiagnostic {$( $diag(Box<$diag>), )*} @@ -32,7 +32,13 @@ macro_rules! diagnostics { }; } -diagnostics![UnresolvedModule, UnresolvedExternCrate, UnresolvedImport, MissingFields]; +diagnostics![ + UnresolvedModule, + UnresolvedExternCrate, + UnresolvedImport, + UnresolvedMacroCall, + MissingFields, +]; #[derive(Debug)] pub struct UnresolvedModule { @@ -50,35 +56,12 @@ pub struct UnresolvedImport { pub decl: InFile>, } -// Diagnostic: unresolved-macro-call -// -// This diagnostic is triggered if rust-analyzer is unable to resolve the path to a -// macro in a macro invocation. #[derive(Debug, Clone, Eq, PartialEq)] pub struct UnresolvedMacroCall { - pub file: HirFileId, - pub node: AstPtr, + pub macro_call: InFile>, pub path: ModPath, } -impl Diagnostic for UnresolvedMacroCall { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("unresolved-macro-call") - } - fn message(&self) -> String { - format!("unresolved macro `{}!`", self.path) - } - fn display_source(&self) -> InFile { - InFile::new(self.file, self.node.clone().into()) - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - fn is_experimental(&self) -> bool { - true - } -} - // Diagnostic: inactive-code // // This diagnostic is shown for code with inactive `#[cfg]` attributes. diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d32246709..0a9414013 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -580,11 +580,13 @@ impl Module { DefDiagnosticKind::UnresolvedMacroCall { ast, path } => { let node = ast.to_node(db.upcast()); - sink.push(UnresolvedMacroCall { - file: ast.file_id, - node: AstPtr::new(&node), - path: path.clone(), - }); + acc.push( + UnresolvedMacroCall { + macro_call: InFile::new(ast.file_id, AstPtr::new(&node)), + path: path.clone(), + } + .into(), + ); } DefDiagnosticKind::MacroError { ast, message } => { @@ -1060,13 +1062,9 @@ impl Function { precise_location: None, macro_name: None, }), - BodyDiagnostic::UnresolvedMacroCall { node, path } => { - sink.push(UnresolvedMacroCall { - file: node.file_id, - node: node.value.clone(), - path: path.clone(), - }) - } + BodyDiagnostic::UnresolvedMacroCall { node, path } => acc.push( + UnresolvedMacroCall { macro_call: node.clone(), path: path.clone() }.into(), + ), } } diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index 7b0f42004..5a088b6e5 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -54,37 +54,6 @@ fn inactive_via_cfg_attr() { ); } -#[test] -fn unresolved_legacy_scope_macro() { - check_diagnostics( - r#" - //- /lib.rs - macro_rules! m { () => {} } - - m!(); - m2!(); - //^^^^^^ UnresolvedMacroCall - "#, - ); -} - -#[test] -fn unresolved_module_scope_macro() { - check_diagnostics( - r#" - //- /lib.rs - mod mac { - #[macro_export] - macro_rules! m { () => {} } - } - - self::m!(); - self::m2!(); - //^^^^^^^^^^^^ UnresolvedMacroCall - "#, - ); -} - #[test] fn builtin_macro_fails_expansion() { check_diagnostics( diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 3d05dd093..1a4800832 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -7,6 +7,7 @@ mod unresolved_module; mod unresolved_extern_crate; mod unresolved_import; +mod unresolved_macro_call; mod missing_fields; mod fixes; @@ -16,9 +17,8 @@ mod unlinked_file; use std::cell::RefCell; use hir::{ - db::AstDatabase, diagnostics::{AnyDiagnostic, Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder}, - InFile, Semantics, + Semantics, }; use ide_assists::AssistResolveStrategy; use ide_db::{base_db::SourceDatabase, RootDatabase}; @@ -203,20 +203,6 @@ pub(crate) fn diagnostics( res.borrow_mut() .push(Diagnostic::hint(display_range, d.message()).with_code(Some(d.code()))); }) - .on::(|d| { - let last_path_segment = sema.db.parse_or_expand(d.file).and_then(|root| { - d.node - .to_node(&root) - .path() - .and_then(|it| it.segment()) - .and_then(|it| it.name_ref()) - .map(|it| InFile::new(d.file, SyntaxNodePtr::new(it.syntax()))) - }); - let diagnostics = last_path_segment.unwrap_or_else(|| d.display_source()); - let display_range = sema.diagnostics_display_range(diagnostics).range; - res.borrow_mut() - .push(Diagnostic::error(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() @@ -259,6 +245,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), 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::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), }; if let Some(code) = d.code { @@ -480,16 +467,6 @@ mod tests { assert_eq!(expected, actual); } - #[test] - fn test_unresolved_macro_range() { - check_diagnostics( - r#" -foo::bar!(92); - //^^^ unresolved macro `foo::bar!` -"#, - ); - } - #[test] fn range_mapping_out_of_macros() { // FIXME: this is very wrong, but somewhat tricky to fix. diff --git a/crates/ide/src/diagnostics/unresolved_macro_call.rs b/crates/ide/src/diagnostics/unresolved_macro_call.rs new file mode 100644 index 000000000..a3af332a4 --- /dev/null +++ b/crates/ide/src/diagnostics/unresolved_macro_call.rs @@ -0,0 +1,72 @@ +use hir::{db::AstDatabase, InFile}; +use syntax::{AstNode, SyntaxNodePtr}; + +use crate::diagnostics::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-macro-call +// +// This diagnostic is triggered if rust-analyzer is unable to resolve the path +// to a macro in a macro invocation. +pub(super) fn unresolved_macro_call( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedMacroCall, +) -> Diagnostic { + let last_path_segment = ctx.sema.db.parse_or_expand(d.macro_call.file_id).and_then(|root| { + d.macro_call + .value + .to_node(&root) + .path() + .and_then(|it| it.segment()) + .and_then(|it| it.name_ref()) + .map(|it| InFile::new(d.macro_call.file_id, SyntaxNodePtr::new(it.syntax()))) + }); + let diagnostics = last_path_segment.unwrap_or_else(|| d.macro_call.clone().map(|it| it.into())); + + Diagnostic::new( + "unresolved-macro-call", + format!("unresolved macro `{}!`", d.path), + ctx.sema.diagnostics_display_range(diagnostics).range, + ) + .experimental() +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_diagnostics; + + #[test] + fn test_unresolved_macro_range() { + check_diagnostics( + r#" +foo::bar!(92); + //^^^ unresolved macro `foo::bar!` +"#, + ); + } + + #[test] + fn unresolved_legacy_scope_macro() { + check_diagnostics( + r#" +macro_rules! m { () => {} } + +m!(); m2!(); + //^^ unresolved macro `self::m2!` +"#, + ); + } + + #[test] + fn unresolved_module_scope_macro() { + check_diagnostics( + r#" +mod mac { +#[macro_export] +macro_rules! m { () => {} } } + +self::m!(); self::m2!(); + //^^ unresolved macro `self::m2!` +"#, + ); + } +} -- cgit v1.2.3