Auto merge of #16058 - Veykril:macro-diagnostics, r=Veykril

fix: Smaller spans for unresolved field and method diagnostics
rust-lang · Dec 8, 2023 · 0395328 · 0395328
2 parents 9e82ab5 + b1a8f83
commit 0395328
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 18 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/hir-expand/src/files.rs b/crates/hir-expand/src/files.rs
@@ -314,6 +314,21 @@ impl InFile<TextRange> {
         }
     }
 
+    pub fn original_node_file_range_rooted(self, db: &dyn db::ExpandDatabase) -> FileRange {
+        match self.file_id.repr() {
+            HirFileIdRepr::FileId(file_id) => FileRange { file_id, range: self.value },
+            HirFileIdRepr::MacroFile(mac_file) => {
+                match ExpansionInfo::new(db, mac_file).map_node_range_up(db, self.value) {
+                    Some((it, SyntaxContextId::ROOT)) => it,
+                    _ => {
+                        let loc = db.lookup_intern_macro_call(mac_file.macro_call_id);
+                        loc.kind.original_call_range(db)
+                    }
+                }
+            }
+        }
+    }
+
     pub fn original_node_file_range_opt(
         self,
         db: &dyn db::ExpandDatabase,

diff --git a/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/crates/ide-diagnostics/src/handlers/unresolved_field.rs
@@ -8,7 +8,7 @@ use ide_db::{
 use syntax::{ast, AstNode, AstPtr};
 use text_edit::TextEdit;
 
-use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
+use crate::{adjusted_display_range_new, Diagnostic, DiagnosticCode, DiagnosticsContext};
 
 // Diagnostic: unresolved-field
 //
@@ -22,15 +22,24 @@ pub(crate) fn unresolved_field(
     } else {
         ""
     };
-    Diagnostic::new_with_syntax_node_ptr(
-        ctx,
+    Diagnostic::new(
         DiagnosticCode::RustcHardError("E0559"),
         format!(
             "no field `{}` on type `{}`{method_suffix}",
             d.name.display(ctx.sema.db),
             d.receiver.display(ctx.sema.db)
         ),
-        d.expr.clone().map(|it| it.into()),
+        adjusted_display_range_new(ctx, d.expr, &|expr| {
+            Some(
+                match expr {
+                    ast::Expr::MethodCallExpr(it) => it.name_ref(),
+                    ast::Expr::FieldExpr(it) => it.name_ref(),
+                    _ => None,
+                }?
+                .syntax()
+                .text_range(),
+            )
+        }),
     )
     .with_fixes(fixes(ctx, d))
     .experimental()
@@ -79,7 +88,7 @@ mod tests {
             r#"
 fn main() {
     ().foo;
- // ^^^^^^ error: no field `foo` on type `()`
+    // ^^^ error: no field `foo` on type `()`
 }
 "#,
         );
@@ -95,7 +104,7 @@ impl Foo {
 }
 fn foo() {
     Foo.bar;
- // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
+     // ^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
 }
 "#,
         );
@@ -112,7 +121,7 @@ trait Bar {
 impl Bar for Foo {}
 fn foo() {
     Foo.bar;
- // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
+     // ^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
 }
 "#,
         );
@@ -131,7 +140,7 @@ impl Bar for Foo {
 }
 fn foo() {
     Foo.bar;
- // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
+     // ^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists
 }
 "#,
         );

diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs
@@ -8,7 +8,7 @@ use ide_db::{
 use syntax::{ast, AstNode, TextRange};
 use text_edit::TextEdit;
 
-use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
+use crate::{adjusted_display_range_new, Diagnostic, DiagnosticCode, DiagnosticsContext};
 
 // Diagnostic: unresolved-method
 //
@@ -22,15 +22,24 @@ pub(crate) fn unresolved_method(
     } else {
         ""
     };
-    Diagnostic::new_with_syntax_node_ptr(
-        ctx,
+    Diagnostic::new(
         DiagnosticCode::RustcHardError("E0599"),
         format!(
             "no method `{}` on type `{}`{field_suffix}",
             d.name.display(ctx.sema.db),
             d.receiver.display(ctx.sema.db)
         ),
-        d.expr.clone().map(|it| it.into()),
+        adjusted_display_range_new(ctx, d.expr, &|expr| {
+            Some(
+                match expr {
+                    ast::Expr::MethodCallExpr(it) => it.name_ref(),
+                    ast::Expr::FieldExpr(it) => it.name_ref(),
+                    _ => None,
+                }?
+                .syntax()
+                .text_range(),
+            )
+        }),
     )
     .with_fixes(fixes(ctx, d))
     .experimental()
@@ -92,7 +101,41 @@ mod tests {
             r#"
 fn main() {
     ().foo();
- // ^^^^^^^^ error: no method `foo` on type `()`
+    // ^^^ error: no method `foo` on type `()`
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn smoke_test_in_macro_def_site() {
+        check_diagnostics(
+            r#"
+macro_rules! m {
+    ($rcv:expr) => {
+        $rcv.foo()
+    }
+}
+fn main() {
+    m!(());
+ // ^^^^^^ error: no method `foo` on type `()`
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn smoke_test_in_macro_call_site() {
+        check_diagnostics(
+            r#"
+macro_rules! m {
+    ($ident:ident) => {
+        ().$ident()
+    }
+}
+fn main() {
+    m!(foo);
+    // ^^^ error: no method `foo` on type `()`
 }
 "#,
         );
@@ -105,7 +148,7 @@ fn main() {
 struct Foo { bar: i32 }
 fn foo() {
     Foo { bar: i32 }.bar();
- // ^^^^^^^^^^^^^^^^^^^^^^ error: no method `bar` on type `Foo`, but a field with a similar name exists
+                  // ^^^ error: no method `bar` on type `Foo`, but a field with a similar name exists
 }
 "#,
         );

diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
@@ -90,7 +90,7 @@ use stdx::never;
 use syntax::{
     algo::find_node_at_range,
     ast::{self, AstNode},
-    SyntaxNode, SyntaxNodePtr, TextRange,
+    AstPtr, SyntaxNode, SyntaxNodePtr, TextRange,
 };
 
 // FIXME: Make this an enum
@@ -584,3 +584,16 @@ fn adjusted_display_range<N: AstNode>(
             .unwrap_or(range),
     }
 }
+
+// FIXME Replace the one above with this one?
+fn adjusted_display_range_new<N: AstNode>(
+    ctx: &DiagnosticsContext<'_>,
+    diag_ptr: InFile<AstPtr<N>>,
+    adj: &dyn Fn(N) -> Option<TextRange>,
+) -> FileRange {
+    let source_file = ctx.sema.parse_or_expand(diag_ptr.file_id);
+    let node = diag_ptr.value.to_node(&source_file);
+    diag_ptr
+        .with_value(adj(node).unwrap_or_else(|| diag_ptr.value.text_range()))
+        .original_node_file_range_rooted(ctx.sema.db)
+}
diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml
@@ -16,7 +16,7 @@ doctest = false
 cov-mark = "2.0.0-pre.1"
 either.workspace = true
 itertools.workspace = true
-rowan = "0.15.11"
+rowan = "0.15.15"
 rustc-hash = "1.1.0"
 once_cell = "1.17.0"
 indexmap.workspace = true

diff --git a/crates/syntax/src/ptr.rs b/crates/syntax/src/ptr.rs
@@ -33,6 +33,7 @@ impl<N: AstNode + std::fmt::Debug> std::fmt::Debug for AstPtr<N> {
     }
 }
 
+impl<N: AstNode> Copy for AstPtr<N> {}
 impl<N: AstNode> Clone for AstPtr<N> {
     fn clone(&self) -> AstPtr<N> {
         AstPtr { raw: self.raw.clone(), _ty: PhantomData }