9248: internal: refactor unresolved macro call diagnostic r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2021-06-13 14:09:41 +00:00 committed by GitHub
commit 3d8df2aef8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 220 additions and 186 deletions

View File

@ -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, MissingFields];
diagnostics![
UnresolvedModule,
UnresolvedExternCrate,
UnresolvedImport,
UnresolvedMacroCall,
MissingFields,
];
#[derive(Debug)]
pub struct UnresolvedModule {
@ -47,61 +53,15 @@ pub struct UnresolvedExternCrate {
#[derive(Debug)]
pub struct UnresolvedImport {
pub file: HirFileId,
pub node: AstPtr<ast::UseTree>,
pub decl: InFile<AstPtr<ast::UseTree>>,
}
impl Diagnostic for UnresolvedImport {
fn code(&self) -> DiagnosticCode {
DiagnosticCode("unresolved-import")
}
fn message(&self) -> String {
"unresolved import".to_string()
}
fn display_source(&self) -> InFile<SyntaxNodePtr> {
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 = "../<path>"]` correctly)
// - proc macros and/or proc macro generated code
true
}
}
// 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<ast::MacroCall>,
pub macro_call: InFile<AstPtr<ast::MacroCall>>,
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<SyntaxNodePtr> {
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.

View File

@ -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 } => {
@ -577,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 } => {
@ -1057,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(),
),
}
}

View File

@ -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.
@ -91,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(

View File

@ -6,6 +6,8 @@
mod unresolved_module;
mod unresolved_extern_crate;
mod unresolved_import;
mod unresolved_macro_call;
mod missing_fields;
mod fixes;
@ -15,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};
@ -43,17 +44,39 @@ pub struct Diagnostic {
pub fixes: Option<Vec<Assist>>,
pub unused: bool,
pub code: Option<DiagnosticCode>,
pub experimental: bool,
}
impl Diagnostic {
fn new(code: &'static str, message: impl Into<String>, 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,
}
}
@ -179,20 +203,6 @@ pub(crate) fn diagnostics(
res.borrow_mut()
.push(Diagnostic::hint(display_range, d.message()).with_code(Some(d.code())));
})
.on::<hir::diagnostics::UnresolvedMacroCall, _>(|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::<hir::diagnostics::UnimplementedBuiltinMacro, _>(|d| {
let display_range = sema.diagnostics_display_range(d.display_source()).range;
res.borrow_mut()
@ -234,6 +244,8 @@ 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::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 {
@ -241,6 +253,9 @@ pub(crate) fn diagnostics(
continue;
}
}
if ctx.config.disable_experimental && d.experimental {
continue;
}
res.push(d)
}
@ -452,43 +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 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.

View File

@ -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 = "../<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
}
"#,
);
}
}

View File

@ -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!`
"#,
);
}
}

View File

@ -104,6 +104,7 @@ mod baz {}
"unresolved-module",
),
),
experimental: false,
},
]
"#]],