Repository: rust-lang/rust-analyzer
Pull request #13604 (2022-11-11 12:39:55)

fix: Fix hover in attributed items not preferring similar kinded tokens


Patch:
diff --git a/crates/hir-expand/src/fixup.rs b/crates/hir-expand/src/fixup.rs
index a4abe75626..893e6fe4b8 100644
--- a/crates/hir-expand/src/fixup.rs
+++ b/crates/hir-expand/src/fixup.rs
@@ -4,7 +4,6 @@ use std::mem;
 
 use mbe::{SyntheticToken, SyntheticTokenId, TokenMap};
 use rustc_hash::FxHashMap;
-use smallvec::SmallVec;
 use syntax::{
     ast::{self, AstNode, HasLoopBody},
     match_ast, SyntaxElement, SyntaxKind, SyntaxNode, TextRange,
@@ -293,34 +292,25 @@ pub(crate) fn reverse_fixups(
     token_map: &TokenMap,
     undo_info: &SyntaxFixupUndoInfo,
 ) {
-    let tts = std::mem::take(&mut tt.token_trees);
-    tt.token_trees = tts
-        .into_iter()
-        .filter(|tt| match tt {
-            tt::TokenTree::Leaf(leaf) => token_map.synthetic_token_id(leaf.id()) != Some(EMPTY_ID),
-            tt::TokenTree::Subtree(st) => {
-                st.delimiter.map_or(true, |d| token_map.synthetic_token_id(d.id) != Some(EMPTY_ID))
-            }
-        })
-        .flat_map(|tt| match tt {
-            tt::TokenTree::Subtree(mut tt) => {
-                reverse_fixups(&mut tt, token_map, undo_info);
-                SmallVec::from_const([tt.into()])
-            }
-            tt::TokenTree::Leaf(leaf) => {
-                if let Some(id) = token_map.synthetic_token_id(leaf.id()) {
-                    let original = undo_info.original[id.0 as usize].clone();
-                    if original.delimiter.is_none() {
-                        original.token_trees.into()
-                    } else {
-                        SmallVec::from_const([original.into()])
-                    }
-                } else {
-                    SmallVec::from_const([leaf.into()])
-                }
+    tt.token_trees.retain(|tt| match tt {
+        tt::TokenTree::Leaf(leaf) => {
+            token_map.synthetic_token_id(leaf.id()).is_none()
+                || token_map.synthetic_token_id(leaf.id()) != Some(EMPTY_ID)
+        }
+        tt::TokenTree::Subtree(st) => st.delimiter.map_or(true, |d| {
+            token_map.synthetic_token_id(d.id).is_none()
+                || token_map.synthetic_token_id(d.id) != Some(EMPTY_ID)
+        }),
+    });
+    tt.token_trees.iter_mut().for_each(|tt| match tt {
+        tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map, undo_info),
+        tt::TokenTree::Leaf(leaf) => {
+            if let Some(id) = token_map.synthetic_token_id(leaf.id()) {
+                let original = &undo_info.original[id.0 as usize];
+                *tt = tt::TokenTree::Subtree(original.clone());
             }
-        })
-        .collect();
+        }
+    });
 }
 
 #[cfg(test)]
@@ -329,31 +319,6 @@ mod tests {
 
     use super::reverse_fixups;
 
-    // The following three functions are only meant to check partial structural equivalence of
-    // `TokenTree`s, see the last assertion in `check()`.
-    fn check_leaf_eq(a: &tt::Leaf, b: &tt::Leaf) -> bool {
-        match (a, b) {
-            (tt::Leaf::Literal(a), tt::Leaf::Literal(b)) => a.text == b.text,
-            (tt::Leaf::Punct(a), tt::Leaf::Punct(b)) => a.char == b.char,
-            (tt::Leaf::Ident(a), tt::Leaf::Ident(b)) => a.text == b.text,
-            _ => false,
-        }
-    }
-
-    fn check_subtree_eq(a: &tt::Subtree, b: &tt::Subtree) -> bool {
-        a.delimiter.map(|it| it.kind) == b.delimiter.map(|it| it.kind)
-            && a.token_trees.len() == b.token_trees.len()
-            && a.token_trees.iter().zip(&b.token_trees).all(|(a, b)| check_tt_eq(a, b))
-    }
-
-    fn check_tt_eq(a: &tt::TokenTree, b: &tt::TokenTree) -> bool {
-        match (a, b) {
-            (tt::TokenTree::Leaf(a), tt::TokenTree::Leaf(b)) => check_leaf_eq(a, b),
-            (tt::TokenTree::Subtree(a), tt::TokenTree::Subtree(b)) => check_subtree_eq(a, b),
-            _ => false,
-        }
-    }
-
     #[track_caller]
     fn check(ra_fixture: &str, mut expect: Expect) {
         let parsed = syntax::SourceFile::parse(ra_fixture);
@@ -366,15 +331,17 @@ mod tests {
             fixups.append,
         );
 
-        let actual = format!("{}\n", tt);
+        let mut actual = tt.to_string();
+        actual.push('\n');
 
         expect.indent(false);
         expect.assert_eq(&actual);
 
         // the fixed-up tree should be syntactically valid
         let (parse, _) = mbe::token_tree_to_syntax_node(&tt, ::mbe::TopEntryPoint::MacroItems);
-        assert!(
-            parse.errors().is_empty(),
+        assert_eq!(
+            parse.errors(),
+            &[],
             "parse has syntax errors. parse tree:\n{:#?}",
             parse.syntax_node()
         );
@@ -382,12 +349,9 @@ mod tests {
         reverse_fixups(&mut tt, &tmap, &fixups.undo_info);
 
         // the fixed-up + reversed version should be equivalent to the original input
-        // modulo token IDs and `Punct`s' spacing.
+        // (but token IDs don't matter)
         let (original_as_tt, _) = mbe::syntax_node_to_token_tree(&parsed.syntax_node());
-        assert!(
-            check_subtree_eq(&tt, &original_as_tt),
-            "different token tree: {tt:?}, {original_as_tt:?}"
-        );
+        assert_eq!(tt.to_string(), original_as_tt.to_string());
     }
 
     #[test]
@@ -504,7 +468,7 @@ fn foo() {
 }
 "#,
             expect![[r#"
-fn foo () {a . __ra_fixup}
+fn foo () {a .__ra_fixup}
 "#]],
         )
     }
@@ -518,7 +482,7 @@ fn foo() {
 }
 "#,
             expect![[r#"
-fn foo () {a . __ra_fixup ;}
+fn foo () {a .__ra_fixup ;}
 "#]],
         )
     }
@@ -533,7 +497,7 @@ fn foo() {
 }
 "#,
             expect![[r#"
-fn foo () {a . __ra_fixup ; bar () ;}
+fn foo () {a .__ra_fixup ; bar () ;}
 "#]],
         )
     }
@@ -561,7 +525,7 @@ fn foo() {
 }
 "#,
             expect![[r#"
-fn foo () {let x = a . __ra_fixup ;}
+fn foo () {let x = a .__ra_fixup ;}
 "#]],
         )
     }
@@ -577,7 +541,7 @@ fn foo() {
 }
 "#,
             expect![[r#"
-fn foo () {a . b ; bar () ;}
+fn foo () {a .b ; bar () ;}
 "#]],
         )
     }
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 3687b597fc..838fb18c3d 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -119,7 +119,14 @@ pub(crate) fn hover(
         });
     }
 
-    let in_attr = matches!(original_token.parent().and_then(ast::TokenTree::cast), Some(tt) if tt.syntax().ancestors().any(|it| ast::Meta::can_cast(it.kind())));
+    let in_attr = original_token
+        .parent_ancestors()
+        .filter_map(ast::Item::cast)
+        .any(|item| sema.is_attr_macro_call(&item))
+        && !matches!(
+            original_token.parent().and_then(ast::TokenTree::cast),
+            Some(tt) if tt.syntax().ancestors().any(|it| ast::Meta::can_cast(it.kind()))
+        );
     // prefer descending the same token kind in attribute expansions, in normal macros text
     // equivalency is more important
     let descended = if in_attr {
diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs
index cf53c16726..e4c56565b9 100644
--- a/crates/mbe/src/syntax_bridge.rs
+++ b/crates/mbe/src/syntax_bridge.rs
@@ -12,9 +12,6 @@ use tt::buffer::{Cursor, TokenBuffer};
 
 use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, TokenMap};
 
-#[cfg(test)]
-mod tests;
-
 /// Convert the syntax node to a `TokenTree` (what macro
 /// will consume).
 pub fn syntax_node_to_token_tree(node: &SyntaxNode) -> (tt::Subtree, TokenMap) {
@@ -38,7 +35,7 @@ pub fn syntax_node_to_token_tree_with_modifications(
     append: FxHashMap<SyntaxElement, Vec<SyntheticToken>>,
 ) -> (tt::Subtree, TokenMap, u32) {
     let global_offset = node.text_range().start();
-    let mut c = Converter::new(node, global_offset, existing_token_map, next_id, replace, append);
+    let mut c = Convertor::new(node, global_offset, existing_token_map, next_id, replace, append);
     let subtree = convert_tokens(&mut c);
     c.id_alloc.map.shrink_to_fit();
     always!(c.replace.is_empty(), "replace: {:?}", c.replace);
@@ -103,7 +100,7 @@ pub fn parse_to_token_tree(text: &str) -> Option<(tt::Subtree, TokenMap)> {
         return None;
     }
 
-    let mut conv = RawConverter {
+    let mut conv = RawConvertor {
         lexed,
         pos: 0,
         id_alloc: TokenIdAlloc {
@@ -151,7 +148,7 @@ pub fn parse_exprs_with_sep(tt: &tt::Subtree, sep: char) -> Vec<tt::Subtree> {
     res
 }
 
-fn convert_tokens<C: TokenConverter>(conv: &mut C) -> tt::Subtree {
+fn convert_tokens<C: TokenConvertor>(conv: &mut C) -> tt::Subtree {
     struct StackEntry {
         subtree: tt::Subtree,
         idx: usize,
@@ -231,7 +228,7 @@ fn convert_tokens<C: TokenConverter>(conv: &mut C) -> tt::Subtree {
             }
 
             let spacing = match conv.peek().map(|next| next.kind(conv)) {
-                Some(kind) if is_single_token_op(kind) => tt::Spacing::Joint,
+                Some(kind) if !kind.is_trivia() => tt::Spacing::Joint,
                 _ => tt::Spacing::Alone,
             };
             let char = match token.to_char(conv) {
@@ -310,35 +307,6 @@ fn convert_tokens<C: TokenConverter>(conv: &mut C) -> tt::Subtree {
     }
 }
 
-fn is_single_token_op(kind: SyntaxKind) -> bool {
-    matches!(
-        kind,
-        EQ | L_ANGLE
-            | R_ANGLE
-            | BANG
-            | AMP
-            | PIPE
-            | TILDE
-            | AT
-            | DOT
-            | COMMA
-            | SEMICOLON
-            | COLON
-            | POUND
-            | DOLLAR
-            | QUESTION
-            | PLUS
-            | MINUS
-            | STAR
-            | SLASH
-            | PERCENT
-            | CARET
-            // LIFETIME_IDENT will be split into a sequence of `'` (a single quote) and an
-            // identifier.
-            | LIFETIME_IDENT
-    )
-}
-
 /// Returns the textual content of a doc comment block as a quoted string
 /// That is, strips leading `///` (or `/**`, etc)
 /// and strips the ending `*/`
@@ -457,8 +425,8 @@ impl TokenIdAlloc {
     }
 }
 
-/// A raw token (straight from lexer) converter
-struct RawConverter<'a> {
+/// A raw token (straight from lexer) convertor
+struct RawConvertor<'a> {
     lexed: parser::LexedStr<'a>,
     pos: usize,
     id_alloc: TokenIdAlloc,
@@ -474,7 +442,7 @@ trait SrcToken<Ctx>: std::fmt::Debug {
     fn synthetic_id(&self, ctx: &Ctx) -> Option<SyntheticTokenId>;
 }
 
-trait TokenConverter: Sized {
+trait TokenConvertor: Sized {
     type Token: SrcToken<Self>;
 
     fn convert_doc_comment(&self, token: &Self::Token) -> Option<Vec<tt::TokenTree>>;
@@ -486,25 +454,25 @@ trait TokenConverter: Sized {
     fn id_alloc(&mut self) -> &mut TokenIdAlloc;
 }
 
-impl<'a> SrcToken<RawConverter<'a>> for usize {
-    fn kind(&self, ctx: &RawConverter<'a>) -> SyntaxKind {
+impl<'a> SrcToken<RawConvertor<'a>> for usize {
+    fn kind(&self, ctx: &RawConvertor<'a>) -> SyntaxKind {
         ctx.lexed.kind(*self)
     }
 
-    fn to_char(&self, ctx: &RawConverter<'a>) -> Option<char> {
+    fn to_char(&self, ctx: &RawConvertor<'a>) -> Option<char> {
         ctx.lexed.text(*self).chars().next()
     }
 
-    fn to_text(&self, ctx: &RawConverter<'_>) -> SmolStr {
+    fn to_text(&self, ctx: &RawConvertor<'_>) -> SmolStr {
         ctx.lexed.text(*self).into()
     }
 
-    fn synthetic_id(&self, _ctx: &RawConverter<'a>) -> Option<SyntheticTokenId> {
+    fn synthetic_id(&self, _ctx: &RawConvertor<'a>) -> Option<SyntheticTokenId> {
         None
     }
 }
 
-impl<'a> TokenConverter for RawConverter<'a> {
+impl<'a> TokenConvertor for RawConvertor<'a> {
     type Token = usize;
 
     fn convert_doc_comment(&self, &token: &usize) -> Option<Vec<tt::TokenTree>> {
@@ -536,7 +504,7 @@ impl<'a> TokenConverter for RawConverter<'a> {
     }
 }
 
-struct Converter {
+struct Convertor {
     id_alloc: TokenIdAlloc,
     current: Option<SyntaxToken>,
     current_synthetic: Vec<SyntheticToken>,
@@ -547,7 +515,7 @@ struct Converter {
     punct_offset: Option<(SyntaxToken, TextSize)>,
 }
 
-impl Converter {
+impl Convertor {
     fn new(
         node: &SyntaxNode,
         global_offset: TextSize,
@@ -555,11 +523,11 @@ impl Converter {
         next_id: u32,
         mut replace: FxHashMap<SyntaxElement, Vec<SyntheticToken>>,
         mut append: FxHashMap<SyntaxElement, Vec<SyntheticToken>>,
-    ) -> Converter {
+    ) -> Convertor {
         let range = node.text_range();
         let mut preorder = node.preorder_with_tokens();
         let (first, synthetic) = Self::next_token(&mut preorder, &mut replace, &mut append);
-        Converter {
+        Convertor {
             id_alloc: { TokenIdAlloc { map: existing_token_map, global_offset, next_id } },
             current: first,
             current_synthetic: synthetic,
@@ -622,15 +590,15 @@ impl SynToken {
     }
 }
 
-impl SrcToken<Converter> for SynToken {
-    fn kind(&self, ctx: &Converter) -> SyntaxKind {
+impl SrcToken<Convertor> for SynToken {
+    fn kind(&self, _ctx: &Convertor) -> SyntaxKind {
         match self {
             SynToken::Ordinary(token) => token.kind(),
-            SynToken::Punch(..) => SyntaxKind::from_char(self.to_char(ctx).unwrap()).unwrap(),
+            SynToken::Punch(token, _) => token.kind(),
             SynToken::Synthetic(token) => token.kind,
         }
     }
-    fn to_char(&self, _ctx: &Converter) -> Option<char> {
+    fn to_char(&self, _ctx: &Convertor) -> Option<char> {
         match self {
             SynToken::Ordinary(_) => None,
             SynToken::Punch(it, i) => it.text().chars().nth((*i).into()),
@@ -638,7 +606,7 @@ impl SrcToken<Converter> for SynToken {
             SynToken::Synthetic(_) => None,
         }
     }
-    fn to_text(&self, _ctx: &Converter) -> SmolStr {
+    fn to_text(&self, _ctx: &Convertor) -> SmolStr {
         match self {
             SynToken::Ordinary(token) => token.text().into(),
             SynToken::Punch(token, _) => token.text().into(),
@@ -646,7 +614,7 @@ impl SrcToken<Converter> for SynToken {
         }
     }
 
-    fn synthetic_id(&self, _ctx: &Converter) -> Option<SyntheticTokenId> {
+    fn synthetic_id(&self, _ctx: &Convertor) -> Option<SyntheticTokenId> {
         match self {
             SynToken::Synthetic(token) => Some(token.id),
             _ => None,
@@ -654,7 +622,7 @@ impl SrcToken<Converter> for SynToken {
     }
 }
 
-impl TokenConverter for Converter {
+impl TokenConvertor for Convertor {
     type Token = SynToken;
     fn convert_doc_comment(&self, token: &Self::Token) -> Option<Vec<tt::TokenTree>> {
         convert_doc_comment(token.token()?)
@@ -683,7 +651,7 @@ impl TokenConverter for Converter {
         }
 
         let curr = self.current.clone()?;
-        if !self.range.contains_range(curr.text_range()) {
+        if !&self.range.contains_range(curr.text_range()) {
             return None;
         }
         let (new_current, new_synth) =
@@ -841,15 +809,12 @@ impl<'a> TtTreeSink<'a> {
         let next = last.bump();
         if let (
             Some(tt::buffer::TokenTreeRef::Leaf(tt::Leaf::Punct(curr), _)),
-            Some(tt::buffer::TokenTreeRef::Leaf(tt::Leaf::Punct(next), _)),
+            Some(tt::buffer::TokenTreeRef::Leaf(tt::Leaf::Punct(_), _)),
         ) = (last.token_tree(), next.token_tree())
         {
             // Note: We always assume the semi-colon would be the last token in
             // other parts of RA such that we don't add whitespace here.
-            //
-            // When `next` is a `Punct` of `'`, that's a part of a lifetime identifier so we don't
-            // need to add whitespace either.
-            if curr.spacing == tt::Spacing::Alone && curr.char != ';' && next.char != '\'' {
+            if curr.spacing == tt::Spacing::Alone && curr.char != ';' {
                 self.inner.token(WHITESPACE, " ");
                 self.text_pos += TextSize::of(' ');
             }
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index 407416d9f4..e1f651786d 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -106,14 +106,6 @@ impl GlobalState {
             status.health = lsp_ext::Health::Error;
             status.message = Some(error)
         }
-
-        if self.config.linked_projects().is_empty()
-            && self.config.detached_files().is_empty()
-            && self.config.notifications().cargo_toml_not_found
-        {
-            status.health = lsp_ext::Health::Warning;
-            status.message = Some("Workspace reload required".to_string())
-        }
         status
     }
 
@@ -435,14 +427,9 @@ impl GlobalState {
     fn fetch_workspace_error(&self) -> Result<(), String> {
         let mut buf = String::new();
 
-        let last_op_result = self.fetch_workspaces_queue.last_op_result();
-        if last_op_result.is_empty() {
-            stdx::format_to!(buf, "rust-analyzer failed to discover workspace");
-        } else {
-            for ws in last_op_result {
-                if let Err(err) = ws {
-                    stdx::format_to!(buf, "rust-analyzer failed to load workspace: {:#}\n", err);
-                }
+        for ws in self.fetch_workspaces_queue.last_op_result() {
+            if let Err(err) = ws {
+                stdx::format_to!(buf, "rust-analyzer failed to load workspace: {:#}\n", err);
             }
         }
 


Test patch:
diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs b/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs
index fc90c6e9f3..bc162d0fa2 100644
--- a/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs
@@ -94,11 +94,11 @@ macro_rules! m {
     ($($s:stmt)*) => (stringify!($($s |)*);)
 }
 stringify!(;
-| ;
-|92| ;
-|let x = 92| ;
+|;
+|92|;
+|let x = 92|;
 |loop {}
-| ;
+|;
 |);
 "#]],
     );
@@ -118,7 +118,7 @@ m!(.. .. ..);
 macro_rules! m {
     ($($p:pat)*) => (stringify!($($p |)*);)
 }
-stringify!(.. .. .. |);
+stringify!(.. .. ..|);
 "#]],
     );
 }
diff --git a/crates/hir-def/src/macro_expansion_tests/proc_macros.rs b/crates/hir-def/src/macro_expansion_tests/proc_macros.rs
index 118c14ed84..029821e5e8 100644
--- a/crates/hir-def/src/macro_expansion_tests/proc_macros.rs
+++ b/crates/hir-def/src/macro_expansion_tests/proc_macros.rs
@@ -82,14 +82,14 @@ fn attribute_macro_syntax_completion_2() {
 #[proc_macros::identity_when_valid]
 fn foo() { bar.; blub }
 "#,
-        expect![[r#"
+        expect![[r##"
 #[proc_macros::identity_when_valid]
 fn foo() { bar.; blub }
 
 fn foo() {
-    bar. ;
+    bar.;
     blub
-}"#]],
+}"##]],
     );
 }
 
diff --git a/crates/mbe/src/syntax_bridge/tests.rs b/crates/mbe/src/syntax_bridge/tests.rs
deleted file mode 100644
index 4e04d2bc1c..0000000000
--- a/crates/mbe/src/syntax_bridge/tests.rs
+++ /dev/null
@@ -1,93 +0,0 @@
-use std::collections::HashMap;
-
-use syntax::{ast, AstNode};
-use test_utils::extract_annotations;
-use tt::{
-    buffer::{TokenBuffer, TokenTreeRef},
-    Leaf, Punct, Spacing,
-};
-
-use super::syntax_node_to_token_tree;
-
-fn check_punct_spacing(fixture: &str) {
-    let source_file = ast::SourceFile::parse(fixture).ok().unwrap();
-    let (subtree, token_map) = syntax_node_to_token_tree(source_file.syntax());
-    let mut annotations: HashMap<_, _> = extract_annotations(fixture)
-        .into_iter()
-        .map(|(range, annotation)| {
-            let token = token_map.token_by_range(range).expect("no token found");
-            let spacing = match annotation.as_str() {
-                "Alone" => Spacing::Alone,
-                "Joint" => Spacing::Joint,
-                a => panic!("unknown annotation: {}", a),
-            };
-            (token, spacing)
-        })
-        .collect();
-
-    let buf = TokenBuffer::from_subtree(&subtree);
-    let mut cursor = buf.begin();
-    while !cursor.eof() {
-        while let Some(token_tree) = cursor.token_tree() {
-            if let TokenTreeRef::Leaf(Leaf::Punct(Punct { spacing, id, .. }), _) = token_tree {
-                if let Some(expected) = annotations.remove(&id) {
-                    assert_eq!(expected, *spacing);
-                }
-            }
-            cursor = cursor.bump_subtree();
-        }
-        cursor = cursor.bump();
-    }
-
-    assert!(annotations.is_empty(), "unchecked annotations: {:?}", annotations);
-}
-
-#[test]
-fn punct_spacing() {
-    check_punct_spacing(
-        r#"
-fn main() {
-    0+0;
-   //^ Alone
-    0+(0);
-   //^ Alone
-    0<=0;
-   //^ Joint
-   // ^ Alone
-    0<=(0);
-   // ^ Alone
-    a=0;
-   //^ Alone
-    a=(0);
-   //^ Alone
-    a+=0;
-   //^ Joint
-   // ^ Alone
-    a+=(0);
-   // ^ Alone
-    a&&b;
-   //^ Joint
-   // ^ Alone
-    a&&(b);
-   // ^ Alone
-    foo::bar;
-   //  ^ Joint
-   //   ^ Alone
-    use foo::{bar,baz,};
-   //       ^ Alone
-   //            ^ Alone
-   //                ^ Alone
-    struct Struct<'a> {};
-   //            ^ Joint
-   //             ^ Joint
-    Struct::<0>;
-   //       ^ Alone
-    Struct::<{0}>;
-   //       ^ Alone
-    ;;
-  //^ Joint
-  // ^ Alone
-}
-        "#,
-    );
-}