Repository: rust-analyzer/rust-analyzer
Pull request #6698 (2020-12-02 13:25:39)

Attach macro expansion errors to the right file
Previously it attached them to the result of the macro expansion (or, if no result was produced, to the file containing the invocation). Always use the file containing the invocation.

This doesn't seem to have any observable difference, but seems better in theory.

bors r+


Patch:
diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index 2c41c00059..689a3274c7 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -560,6 +560,9 @@ impl ExprCollector<'_> {
                     // FIXME: do we still need to allocate this as missing ?
                     self.alloc_expr(Expr::Missing, syntax_ptr)
                 } else {
+                    // File containing the macro call. Expansion errors will be attached here.
+                    let outer_file = self.expander.current_file_id;
+
                     let macro_call = self.expander.to_source(AstPtr::new(&e));
                     let res = self.expander.enter_expand(self.db, Some(&self.body.item_scope), e);
 
@@ -567,7 +570,7 @@ impl ExprCollector<'_> {
                         Some(ExpandError::UnresolvedProcMacro) => {
                             self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro(
                                 UnresolvedProcMacro {
-                                    file: self.expander.current_file_id,
+                                    file: outer_file,
                                     node: syntax_ptr.clone().into(),
                                     precise_location: None,
                                     macro_name: None,
@@ -577,7 +580,7 @@ impl ExprCollector<'_> {
                         Some(err) => {
                             self.source_map.diagnostics.push(BodyDiagnostic::MacroError(
                                 MacroError {
-                                    file: self.expander.current_file_id,
+                                    file: outer_file,
                                     node: syntax_ptr.clone().into(),
                                     message: err.to_string(),
                                 },
diff --git a/crates/hir_def/src/path/lower/lower_use.rs b/crates/hir_def/src/path/lower/lower_use.rs
index ba0d1f0e7a..53cecb05fb 100644
--- a/crates/hir_def/src/path/lower/lower_use.rs
+++ b/crates/hir_def/src/path/lower/lower_use.rs
@@ -76,7 +76,7 @@ fn convert_path(prefix: Option<ModPath>, path: ast::Path, hygiene: &Hygiene) ->
                 Either::Left(name) => {
                     // no type args in use
                     let mut res = prefix.unwrap_or_else(|| ModPath {
-                        kind: segment.coloncolon_token().map_or(PathKind::Plain, |_| PathKind::Abs),
+                        kind: PathKind::Plain,
                         segments: Vec::with_capacity(1),
                     });
                     res.segments.push(name);
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 462f5c2b8b..94ad800a08 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -295,6 +295,7 @@ fn definition_owner_name(db: &RootDatabase, def: &Definition) -> Option<String>
             ModuleDef::EnumVariant(e) => Some(e.parent_enum(db).name(db)),
             _ => None,
         },
+        Definition::SelfType(i) => i.target_ty(db).as_adt().map(|adt| adt.name(db)),
         _ => None,
     }
     .map(|name| name.to_string())
@@ -356,14 +357,7 @@ fn hover_for_definition(db: &RootDatabase, def: Definition) -> Option<Markup> {
             ModuleDef::BuiltinType(it) => return Some(it.to_string().into()),
         },
         Definition::Local(it) => return Some(Markup::fenced_block(&it.ty(db).display(db))),
-        Definition::SelfType(impl_def) => {
-            impl_def.target_ty(db).as_adt().and_then(|adt| match adt {
-                Adt::Struct(it) => from_def_source(db, it, mod_path),
-                Adt::Union(it) => from_def_source(db, it, mod_path),
-                Adt::Enum(it) => from_def_source(db, it, mod_path),
-            })
-        }
-        Definition::TypeParam(_) => {
+        Definition::TypeParam(_) | Definition::SelfType(_) => {
             // FIXME: Hover for generic param
             None
         }
@@ -1031,75 +1025,52 @@ impl Thing {
 }
 "#,
             expect![[r#"
-                *Self*
-
-                ```rust
-                test
-                ```
-
-                ```rust
-                struct Thing
-                ```
-            "#]],
-        );
-        check(
-            r#"
-struct Thing { x: u32 }
-impl Thing {
-    fn new() -> Self<|> { Self { x: 0 } }
-}
-"#,
-            expect![[r#"
-                *Self*
-
-                ```rust
-                test
-                ```
-
+                *Self { x: 0 }*
                 ```rust
-                struct Thing
-                ```
-            "#]],
-        );
-        check(
-            r#"
-enum Thing { A }
-impl Thing {
-    pub fn new() -> Self<|> { Thing::A }
-}
-"#,
-            expect![[r#"
-                *Self*
-
-                ```rust
-                test
-                ```
-
-                ```rust
-                enum Thing
-                ```
-            "#]],
-        );
-        check(
-            r#"
-        enum Thing { A }
-        impl Thing {
-            pub fn thing(a: Self<|>) {}
-        }
-        "#,
-            expect![[r#"
-                *Self*
-
-                ```rust
-                test
-                ```
-
-                ```rust
-                enum Thing
+                Thing
                 ```
             "#]],
-        );
-    }
+        )
+    } /* FIXME: revive these tests
+              let (analysis, position) = fixture::position(
+                  "
+                  struct Thing { x: u32 }
+                  impl Thing {
+                      fn new() -> Self<|> {
+                          Self { x: 0 }
+                      }
+                  }
+                  ",
+              );
+
+              let hover = analysis.hover(position).unwrap().unwrap();
+              assert_eq!(trim_markup(&hover.info.markup.as_str()), ("Thing"));
+
+              let (analysis, position) = fixture::position(
+                  "
+                  enum Thing { A }
+                  impl Thing {
+                      pub fn new() -> Self<|> {
+                          Thing::A
+                      }
+                  }
+                  ",
+              );
+              let hover = analysis.hover(position).unwrap().unwrap();
+              assert_eq!(trim_markup(&hover.info.markup.as_str()), ("enum Thing"));
+
+              let (analysis, position) = fixture::position(
+                  "
+                  enum Thing { A }
+                  impl Thing {
+                      pub fn thing(a: Self<|>) {
+                      }
+                  }
+                  ",
+              );
+              let hover = analysis.hover(position).unwrap().unwrap();
+              assert_eq!(trim_markup(&hover.info.markup.as_str()), ("enum Thing"));
+      */
 
     #[test]
     fn test_hover_shadowing_pat() {
diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs
index fa40e64e8e..b985861f22 100644
--- a/crates/syntax/src/ast/token_ext.rs
+++ b/crates/syntax/src/ast/token_ext.rs
@@ -543,6 +543,11 @@ impl HasFormatSpecifier for ast::String {
 }
 
 impl ast::IntNumber {
+    const SUFFIXES: &'static [&'static str] = &[
+        "u8", "u16", "u32", "u64", "u128", "usize", // Unsigned.
+        "i8", "i16", "i32", "i64", "i128", "isize", // Signed.
+    ];
+
     pub fn radix(&self) -> Radix {
         match self.text().get(..2).unwrap_or_default() {
             "0b" => Radix::Binary,
@@ -575,30 +580,29 @@ impl ast::IntNumber {
 
     pub fn suffix(&self) -> Option<&str> {
         let text = self.text();
-        let radix = self.radix();
-        let mut indices = text.char_indices();
-        if radix != Radix::Decimal {
-            indices.next()?;
-            indices.next()?;
-        }
-        let is_suffix_start: fn(&(usize, char)) -> bool = match radix {
-            Radix::Hexadecimal => |(_, c)| matches!(c, 'g'..='z' | 'G'..='Z'),
-            _ => |(_, c)| c.is_ascii_alphabetic(),
-        };
-        let (suffix_start, _) = indices.find(is_suffix_start)?;
-        Some(&text[suffix_start..])
+        // FIXME: don't check a fixed set of suffixes, `1_0_1_l_o_l` is valid
+        // syntax, suffix is `l_o_l`.
+        ast::IntNumber::SUFFIXES.iter().chain(ast::FloatNumber::SUFFIXES.iter()).find_map(
+            |suffix| {
+                if text.ends_with(suffix) {
+                    return Some(&text[text.len() - suffix.len()..]);
+                }
+                None
+            },
+        )
     }
 }
 
 impl ast::FloatNumber {
+    const SUFFIXES: &'static [&'static str] = &["f32", "f64"];
     pub fn suffix(&self) -> Option<&str> {
         let text = self.text();
-        let mut indices = text.char_indices();
-        let (mut suffix_start, c) = indices.by_ref().find(|(_, c)| c.is_ascii_alphabetic())?;
-        if c == 'e' || c == 'E' {
-            suffix_start = indices.find(|(_, c)| c.is_ascii_alphabetic())?.0;
-        }
-        Some(&text[suffix_start..])
+        ast::FloatNumber::SUFFIXES.iter().find_map(|suffix| {
+            if text.ends_with(suffix) {
+                return Some(&text[text.len() - suffix.len()..]);
+            }
+            None
+        })
     }
 }
 
@@ -621,40 +625,3 @@ impl Radix {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::ast::{make, FloatNumber, IntNumber};
-
-    fn check_float_suffix<'a>(lit: &str, expected: impl Into<Option<&'a str>>) {
-        assert_eq!(FloatNumber { syntax: make::tokens::literal(lit) }.suffix(), expected.into());
-    }
-
-    fn check_int_suffix<'a>(lit: &str, expected: impl Into<Option<&'a str>>) {
-        assert_eq!(IntNumber { syntax: make::tokens::literal(lit) }.suffix(), expected.into());
-    }
-
-    #[test]
-    fn test_float_number_suffix() {
-        check_float_suffix("123.0", None);
-        check_float_suffix("123f32", "f32");
-        check_float_suffix("123.0e", None);
-        check_float_suffix("123.0e4", None);
-        check_float_suffix("123.0ef32", "f32");
-        check_float_suffix("123.0E4f32", "f32");
-        check_float_suffix("1_2_3.0_f32", "f32");
-    }
-
-    #[test]
-    fn test_int_number_suffix() {
-        check_int_suffix("123", None);
-        check_int_suffix("123i32", "i32");
-        check_int_suffix("1_0_1_l_o_l", "l_o_l");
-        check_int_suffix("0b11", None);
-        check_int_suffix("0o11", None);
-        check_int_suffix("0xff", None);
-        check_int_suffix("0b11u32", "u32");
-        check_int_suffix("0o11u32", "u32");
-        check_int_suffix("0xffu32", "u32");
-    }
-}
diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs
index 9093fbd979..4aa2d6526b 100644
--- a/crates/vfs/src/file_set.rs
+++ b/crates/vfs/src/file_set.rs
@@ -158,4 +158,53 @@ impl fst::Automaton for PrefixOf<'_> {
 }
 
 #[cfg(test)]
-mod tests;
+mod tests {
+    use super::*;
+
+    #[test]
+    fn path_prefix() {
+        let mut file_set = FileSetConfig::builder();
+        file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo".into())]);
+        file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo/bar/baz".into())]);
+        let file_set = file_set.build();
+
+        let mut vfs = Vfs::default();
+        vfs.set_file_contents(
+            VfsPath::new_virtual_path("/foo/src/lib.rs".into()),
+            Some(Vec::new()),
+        );
+        vfs.set_file_contents(
+            VfsPath::new_virtual_path("/foo/src/bar/baz/lib.rs".into()),
+            Some(Vec::new()),
+        );
+        vfs.set_file_contents(
+            VfsPath::new_virtual_path("/foo/bar/baz/lib.rs".into()),
+            Some(Vec::new()),
+        );
+        vfs.set_file_contents(VfsPath::new_virtual_path("/quux/lib.rs".into()), Some(Vec::new()));
+
+        let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();
+        assert_eq!(partition, vec![2, 1, 1]);
+    }
+
+    #[test]
+    fn name_prefix() {
+        let mut file_set = FileSetConfig::builder();
+        file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo".into())]);
+        file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo-things".into())]);
+        let file_set = file_set.build();
+
+        let mut vfs = Vfs::default();
+        vfs.set_file_contents(
+            VfsPath::new_virtual_path("/foo/src/lib.rs".into()),
+            Some(Vec::new()),
+        );
+        vfs.set_file_contents(
+            VfsPath::new_virtual_path("/foo-things/src/lib.rs".into()),
+            Some(Vec::new()),
+        );
+
+        let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();
+        assert_eq!(partition, vec![1, 1, 0]);
+    }
+}
diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs
index bd14911c96..815697597e 100644
--- a/crates/vfs/src/vfs_path.rs
+++ b/crates/vfs/src/vfs_path.rs
@@ -311,4 +311,35 @@ impl VirtualPath {
 }
 
 #[cfg(test)]
-mod tests;
+mod tests {
+    use super::*;
+
+    #[test]
+    fn virtual_path_extensions() {
+        assert_eq!(VirtualPath("/".to_string()).name_and_extension(), None);
+        assert_eq!(
+            VirtualPath("/directory".to_string()).name_and_extension(),
+            Some(("directory", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/".to_string()).name_and_extension(),
+            Some(("directory", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/file".to_string()).name_and_extension(),
+            Some(("file", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/.file".to_string()).name_and_extension(),
+            Some((".file", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/.file.rs".to_string()).name_and_extension(),
+            Some((".file", Some("rs")))
+        );
+        assert_eq!(
+            VirtualPath("/directory/file.rs".to_string()).name_and_extension(),
+            Some(("file", Some("rs")))
+        );
+    }
+}


Test patch:
diff --git a/crates/hir_def/src/nameres/tests/mod_resolution.rs b/crates/hir_def/src/nameres/tests/mod_resolution.rs
index ef6f85e153..ba295fd9e0 100644
--- a/crates/hir_def/src/nameres/tests/mod_resolution.rs
+++ b/crates/hir_def/src/nameres/tests/mod_resolution.rs
@@ -798,24 +798,3 @@ mod foo;
 "#,
     );
 }
-
-#[test]
-fn abs_path_ignores_local() {
-    check(
-        r#"
-//- /main.rs crate:main deps:core
-pub use ::core::hash::Hash;
-pub mod core {}
-
-//- /lib.rs crate:core
-pub mod hash { pub trait Hash {} }
-"#,
-        expect![[r#"
-            crate
-            Hash: t
-            core: t
-
-            crate::core
-        "#]],
-    );
-}
diff --git a/crates/vfs/src/file_set/tests.rs b/crates/vfs/src/file_set/tests.rs
deleted file mode 100644
index 2146df185d..0000000000
--- a/crates/vfs/src/file_set/tests.rs
+++ /dev/null
@@ -1,42 +0,0 @@
-use super::*;
-
-#[test]
-fn path_prefix() {
-    let mut file_set = FileSetConfig::builder();
-    file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo".into())]);
-    file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo/bar/baz".into())]);
-    let file_set = file_set.build();
-
-    let mut vfs = Vfs::default();
-    vfs.set_file_contents(VfsPath::new_virtual_path("/foo/src/lib.rs".into()), Some(Vec::new()));
-    vfs.set_file_contents(
-        VfsPath::new_virtual_path("/foo/src/bar/baz/lib.rs".into()),
-        Some(Vec::new()),
-    );
-    vfs.set_file_contents(
-        VfsPath::new_virtual_path("/foo/bar/baz/lib.rs".into()),
-        Some(Vec::new()),
-    );
-    vfs.set_file_contents(VfsPath::new_virtual_path("/quux/lib.rs".into()), Some(Vec::new()));
-
-    let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();
-    assert_eq!(partition, vec![2, 1, 1]);
-}
-
-#[test]
-fn name_prefix() {
-    let mut file_set = FileSetConfig::builder();
-    file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo".into())]);
-    file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo-things".into())]);
-    let file_set = file_set.build();
-
-    let mut vfs = Vfs::default();
-    vfs.set_file_contents(VfsPath::new_virtual_path("/foo/src/lib.rs".into()), Some(Vec::new()));
-    vfs.set_file_contents(
-        VfsPath::new_virtual_path("/foo-things/src/lib.rs".into()),
-        Some(Vec::new()),
-    );
-
-    let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();
-    assert_eq!(partition, vec![1, 1, 0]);
-}
diff --git a/crates/vfs/src/vfs_path/tests.rs b/crates/vfs/src/vfs_path/tests.rs
deleted file mode 100644
index 510e021e89..0000000000
--- a/crates/vfs/src/vfs_path/tests.rs
+++ /dev/null
@@ -1,30 +0,0 @@
-use super::*;
-
-#[test]
-fn virtual_path_extensions() {
-    assert_eq!(VirtualPath("/".to_string()).name_and_extension(), None);
-    assert_eq!(
-        VirtualPath("/directory".to_string()).name_and_extension(),
-        Some(("directory", None))
-    );
-    assert_eq!(
-        VirtualPath("/directory/".to_string()).name_and_extension(),
-        Some(("directory", None))
-    );
-    assert_eq!(
-        VirtualPath("/directory/file".to_string()).name_and_extension(),
-        Some(("file", None))
-    );
-    assert_eq!(
-        VirtualPath("/directory/.file".to_string()).name_and_extension(),
-        Some((".file", None))
-    );
-    assert_eq!(
-        VirtualPath("/directory/.file.rs".to_string()).name_and_extension(),
-        Some((".file", Some("rs")))
-    );
-    assert_eq!(
-        VirtualPath("/directory/file.rs".to_string()).name_and_extension(),
-        Some(("file", Some("rs")))
-    );
-}