Auto merge of #13989 - Veykril:hover, r=Veykril

internal: Remove hover fallback in favor of ranged hover

The fallback is usually more annoying than useful at this point (it messes with the range of diagnostic popups a lot), we now have a ranged hover to check the type of something which works a lot better.

Closes https://github.com/rust-lang/rust-analyzer/issues/11602

crates/ide/src/hover.rs

@@ -15,7 +15,7 @@
     FxIndexSet, RootDatabase,
 };
 use itertools::Itertools;
-use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode, SyntaxToken, T};
+use syntax::{ast, AstNode, SyntaxKind::*, SyntaxNode, T};
 
 use crate::{
     doc_links::token_as_doc_comment,
@@ -86,30 +86,38 @@ pub struct HoverResult {
 // image::https://user-images.githubusercontent.com/48062697/113020658-b5f98b80-917a-11eb-9f88-3dbc27320c95.gif[]
 pub(crate) fn hover(
     db: &RootDatabase,
-    file_range: FileRange,
+    frange @ FileRange { file_id, range }: FileRange,
     config: &HoverConfig,
 ) -> Option<RangeInfo<HoverResult>> {
     let sema = &hir::Semantics::new(db);
-    let mut res = hover_impl(sema, file_range, config)?;
+    let file = sema.parse(file_id).syntax().clone();
+    let mut res = if range.is_empty() {
+        hover_simple(sema, FilePosition { file_id, offset: range.start() }, file, config)
+    } else {
+        hover_ranged(sema, frange, file, config)
+    }?;
+
     if let HoverDocFormat::PlainText = config.format {
         res.info.markup = remove_markdown(res.info.markup.as_str()).into();
     }
     Some(res)
 }
 
-fn hover_impl(
+fn hover_simple(
     sema: &Semantics<'_, RootDatabase>,
-    FileRange { file_id, range }: FileRange,
+    FilePosition { file_id, offset }: FilePosition,
+    file: SyntaxNode,
     config: &HoverConfig,
 ) -> Option<RangeInfo<HoverResult>> {
-    let file = sema.parse(file_id).syntax().clone();
-    if !range.is_empty() {
-        return hover_ranged(&file, range, sema, config);
-    }
-    let offset = range.start();
-
     let original_token = pick_best_token(file.token_at_offset(offset), |kind| match kind {
-        IDENT | INT_NUMBER | LIFETIME_IDENT | T![self] | T![super] | T![crate] | T![Self] => 4,
+        IDENT
+        | INT_NUMBER
+        | LIFETIME_IDENT
+        | T![self]
+        | T![super]
+        | T![crate]
+        | T![Self]
+        | T![_] => 4,
         // index and prefix ops
         T!['['] | T![']'] | T![?] | T![*] | T![-] | T![!] => 3,
         kind if kind.is_keyword() => 2,
@@ -142,19 +150,18 @@ fn hover_impl(
     } else {
         sema.descend_into_macros_with_same_text(original_token.clone())
     };
+    let descended = || descended.iter();
 
-    // try lint hover
+    let result = descended()
-    let result = descended
+        // try lint hover
-        .iter()
         .find_map(|token| {
             // FIXME: Definition should include known lints and the like instead of having this special case here
             let attr = token.parent_ancestors().find_map(ast::Attr::cast)?;
             render::try_for_lint(&attr, token)
         })
-        // try item definitions
+        // try definitions
         .or_else(|| {
-            descended
+            descended()
-                .iter()
                 .filter_map(|token| {
                     let node = token.parent()?;
                     let class = IdentClass::classify_token(sema, token)?;
@@ -175,10 +182,12 @@ fn hover_impl(
                 })
         })
         // try keywords
-        .or_else(|| descended.iter().find_map(|token| render::keyword(sema, config, token)))
+        .or_else(|| descended().find_map(|token| render::keyword(sema, config, token)))
-        // try rest item hover
+        // try _ hovers
+        .or_else(|| descended().find_map(|token| render::underscore(sema, config, token)))
+        // try rest pattern hover
         .or_else(|| {
-            descended.iter().find_map(|token| {
+            descended().find_map(|token| {
                 if token.kind() != DOT2 {
                     return None;
                 }
@@ -194,16 +203,41 @@ fn hover_impl(
             })
         });
 
-    result
+    result.map(|mut res: HoverResult| {
-        .map(|mut res: HoverResult| {
+        res.actions = dedupe_or_merge_hover_actions(res.actions);
-            res.actions = dedupe_or_merge_hover_actions(res.actions);
+        RangeInfo::new(original_token.text_range(), res)
-            RangeInfo::new(original_token.text_range(), res)
+    })
-        })
+}
-        // fallback to type hover if there aren't any other suggestions
+
-        // this finds its own range instead of using the closest token's range
+fn hover_ranged(
-        .or_else(|| {
+    sema: &Semantics<'_, RootDatabase>,
-            descended.iter().find_map(|token| hover_type_fallback(sema, config, token, token))
+    FileRange { range, .. }: FileRange,
-        })
+    file: SyntaxNode,
+    config: &HoverConfig,
+) -> Option<RangeInfo<HoverResult>> {
+    // FIXME: make this work in attributes
+    let expr_or_pat = file
+        .covering_element(range)
+        .ancestors()
+        .take_while(|it| ast::MacroCall::can_cast(it.kind()) || !ast::Item::can_cast(it.kind()))
+        .find_map(Either::<ast::Expr, ast::Pat>::cast)?;
+    let res = match &expr_or_pat {
+        Either::Left(ast::Expr::TryExpr(try_expr)) => render::try_expr(sema, config, try_expr),
+        Either::Left(ast::Expr::PrefixExpr(prefix_expr))
+            if prefix_expr.op_kind() == Some(ast::UnaryOp::Deref) =>
+        {
+            render::deref_expr(sema, config, prefix_expr)
+        }
+        _ => None,
+    };
+    let res = res.or_else(|| render::type_info_of(sema, config, &expr_or_pat));
+    res.map(|it| {
+        let range = match expr_or_pat {
+            Either::Left(it) => it.syntax().text_range(),
+            Either::Right(it) => it.syntax().text_range(),
+        };
+        RangeInfo::new(range, it)
+    })
 }
 
 pub(crate) fn hover_for_definition(
@@ -220,77 +254,19 @@ pub(crate) fn hover_for_definition(
     render::definition(sema.db, definition, famous_defs.as_ref(), config).map(|markup| {
         HoverResult {
             markup: render::process_markup(sema.db, definition, &markup, config),
-            actions: show_implementations_action(sema.db, definition)
+            actions: [
-                .into_iter()
+                show_implementations_action(sema.db, definition),
-                .chain(show_fn_references_action(sema.db, definition))
+                show_fn_references_action(sema.db, definition),
-                .chain(runnable_action(sema, definition, file_id))
+                runnable_action(sema, definition, file_id),
-                .chain(goto_type_action_for_def(sema.db, definition))
+                goto_type_action_for_def(sema.db, definition),
-                .collect(),
+            ]
+            .into_iter()
+            .flatten()
+            .collect(),
         }
     })
 }
 
-fn hover_ranged(
-    file: &SyntaxNode,
-    range: syntax::TextRange,
-    sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
-) -> Option<RangeInfo<HoverResult>> {
-    // FIXME: make this work in attributes
-    let expr_or_pat =
-        file.covering_element(range).ancestors().find_map(Either::<ast::Expr, ast::Pat>::cast)?;
-    let res = match &expr_or_pat {
-        Either::Left(ast::Expr::TryExpr(try_expr)) => render::try_expr(sema, config, try_expr),
-        Either::Left(ast::Expr::PrefixExpr(prefix_expr))
-            if prefix_expr.op_kind() == Some(ast::UnaryOp::Deref) =>
-        {
-            render::deref_expr(sema, config, prefix_expr)
-        }
-        _ => None,
-    };
-    let res = res.or_else(|| render::type_info(sema, config, &expr_or_pat));
-    res.map(|it| {
-        let range = match expr_or_pat {
-            Either::Left(it) => it.syntax().text_range(),
-            Either::Right(it) => it.syntax().text_range(),
-        };
-        RangeInfo::new(range, it)
-    })
-}
-
-fn hover_type_fallback(
-    sema: &Semantics<'_, RootDatabase>,
-    config: &HoverConfig,
-    token: &SyntaxToken,
-    original_token: &SyntaxToken,
-) -> Option<RangeInfo<HoverResult>> {
-    let node =
-        token.parent_ancestors().take_while(|it| !ast::Item::can_cast(it.kind())).find(|n| {
-            ast::Expr::can_cast(n.kind())
-                || ast::Pat::can_cast(n.kind())
-                || ast::Type::can_cast(n.kind())
-        })?;
-
-    let expr_or_pat = match_ast! {
-        match node {
-            ast::Expr(it) => Either::Left(it),
-            ast::Pat(it) => Either::Right(it),
-            // If this node is a MACRO_CALL, it means that `descend_into_macros_many` failed to resolve.
-            // (e.g expanding a builtin macro). So we give up here.
-            ast::MacroCall(_it) => return None,
-            _ => return None,
-        }
-    };
-
-    let res = render::type_info(sema, config, &expr_or_pat)?;
-
-    let range = sema
-        .original_range_opt(&node)
-        .map(|frange| frange.range)
-        .unwrap_or_else(|| original_token.text_range());
-    Some(RangeInfo::new(range, res))
-}
-
 fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option<HoverAction> {
     fn to_action(nav_target: NavigationTarget) -> HoverAction {
         HoverAction::Implementation(FilePosition {

crates/ide/src/hover/render.rs

@@ -29,7 +29,7 @@
     HoverAction, HoverConfig, HoverResult, Markup,
 };
 
-pub(super) fn type_info(
+pub(super) fn type_info_of(
     sema: &Semantics<'_, RootDatabase>,
     _config: &HoverConfig,
     expr_or_pat: &Either<ast::Expr, ast::Pat>,
@@ -38,34 +38,7 @@ pub(super) fn type_info(
         Either::Left(expr) => sema.type_of_expr(expr)?,
         Either::Right(pat) => sema.type_of_pat(pat)?,
     };
-
+    type_info(sema, _config, original, adjusted)
-    let mut res = HoverResult::default();
-    let mut targets: Vec<hir::ModuleDef> = Vec::new();
-    let mut push_new_def = |item: hir::ModuleDef| {
-        if !targets.contains(&item) {
-            targets.push(item);
-        }
-    };
-    walk_and_push_ty(sema.db, &original, &mut push_new_def);
-
-    res.markup = if let Some(adjusted_ty) = adjusted {
-        walk_and_push_ty(sema.db, &adjusted_ty, &mut push_new_def);
-        let original = original.display(sema.db).to_string();
-        let adjusted = adjusted_ty.display(sema.db).to_string();
-        let static_text_diff_len = "Coerced to: ".len() - "Type: ".len();
-        format!(
-            "```text\nType: {:>apad$}\nCoerced to: {:>opad$}\n```\n",
-            original,
-            adjusted,
-            apad = static_text_diff_len + adjusted.len().max(original.len()),
-            opad = original.len(),
-        )
-        .into()
-    } else {
-        Markup::fenced_block(&original.display(sema.db))
-    };
-    res.actions.push(HoverAction::goto_type_from_targets(sema.db, targets));
-    Some(res)
 }
 
 pub(super) fn try_expr(
@@ -217,6 +190,48 @@ pub(super) fn deref_expr(
     Some(res)
 }
 
+pub(super) fn underscore(
+    sema: &Semantics<'_, RootDatabase>,
+    config: &HoverConfig,
+    token: &SyntaxToken,
+) -> Option<HoverResult> {
+    if token.kind() != T![_] {
+        return None;
+    }
+    let parent = token.parent()?;
+    let _it = match_ast! {
+        match parent {
+            ast::InferType(it) => it,
+            ast::UnderscoreExpr(it) => return type_info_of(sema, config, &Either::Left(ast::Expr::UnderscoreExpr(it))),
+            ast::WildcardPat(it) => return type_info_of(sema, config, &Either::Right(ast::Pat::WildcardPat(it))),
+            _ => return None,
+        }
+    };
+    // let it = infer_type.syntax().parent()?;
+    // match_ast! {
+    //     match it {
+    //         ast::LetStmt(_it) => (),
+    //         ast::Param(_it) => (),
+    //         ast::RetType(_it) => (),
+    //         ast::TypeArg(_it) => (),
+
+    //         ast::CastExpr(_it) => (),
+    //         ast::ParenType(_it) => (),
+    //         ast::TupleType(_it) => (),
+    //         ast::PtrType(_it) => (),
+    //         ast::RefType(_it) => (),
+    //         ast::ArrayType(_it) => (),
+    //         ast::SliceType(_it) => (),
+    //         ast::ForType(_it) => (),
+    //         _ => return None,
+    //     }
+    // }
+
+    // FIXME: https://github.com/rust-lang/rust-analyzer/issues/11762, this currently always returns Unknown
+    // type_info(sema, config, sema.resolve_type(&ast::Type::InferType(it))?, None)
+    None
+}
+
 pub(super) fn keyword(
     sema: &Semantics<'_, RootDatabase>,
     config: &HoverConfig,
@@ -458,6 +473,41 @@ pub(super) fn definition(
     markup(docs, label, mod_path)
 }
 
+fn type_info(
+    sema: &Semantics<'_, RootDatabase>,
+    _config: &HoverConfig,
+    original: hir::Type,
+    adjusted: Option<hir::Type>,
+) -> Option<HoverResult> {
+    let mut res = HoverResult::default();
+    let mut targets: Vec<hir::ModuleDef> = Vec::new();
+    let mut push_new_def = |item: hir::ModuleDef| {
+        if !targets.contains(&item) {
+            targets.push(item);
+        }
+    };
+    walk_and_push_ty(sema.db, &original, &mut push_new_def);
+
+    res.markup = if let Some(adjusted_ty) = adjusted {
+        walk_and_push_ty(sema.db, &adjusted_ty, &mut push_new_def);
+        let original = original.display(sema.db).to_string();
+        let adjusted = adjusted_ty.display(sema.db).to_string();
+        let static_text_diff_len = "Coerced to: ".len() - "Type: ".len();
+        format!(
+            "```text\nType: {:>apad$}\nCoerced to: {:>opad$}\n```\n",
+            original,
+            adjusted,
+            apad = static_text_diff_len + adjusted.len().max(original.len()),
+            opad = original.len(),
+        )
+        .into()
+    } else {
+        Markup::fenced_block(&original.display(sema.db))
+    };
+    res.actions.push(HoverAction::goto_type_from_targets(sema.db, targets));
+    Some(res)
+}
+
 fn render_builtin_attr(db: &RootDatabase, attr: hir::BuiltinAttr) -> Option<Markup> {
     let name = attr.name(db);
     let desc = format!("#[{name}]");

crates/ide/src/hover/tests.rs

@@ -213,25 +213,6 @@ fn abc()
     );
 }
 
-#[test]
-fn hover_shows_type_of_an_expression() {
-    check(
-        r#"
-pub fn foo() -> u32 { 1 }
-
-fn main() {
-    let foo_test = foo()$0;
-}
-"#,
-        expect![[r#"
-            *foo()*
-            ```rust
-            u32
-            ```
-        "#]],
-    );
-}
-
 #[test]
 fn hover_remove_markdown_if_configured() {
     check_hover_no_markdown(
@@ -239,12 +220,14 @@ fn hover_remove_markdown_if_configured() {
 pub fn foo() -> u32 { 1 }
 
 fn main() {
-    let foo_test = foo()$0;
+    let foo_test = foo$0();
 }
 "#,
         expect![[r#"
-            *foo()*
+            *foo*
-            u32
+            test
+
+            pub fn foo() -> u32
         "#]],
     );
 }
@@ -304,33 +287,6 @@ pub fn foo() -> u32
             "#]],
     );
 
-    // Multiple candidates but results are ambiguous.
-    check(
-        r#"
-//- /a.rs
-pub fn foo() -> u32 { 1 }
-
-//- /b.rs
-pub fn foo() -> &str { "" }
-
-//- /c.rs
-pub fn foo(a: u32, b: u32) {}
-
-//- /main.rs
-mod a;
-mod b;
-mod c;
-
-fn main() { let foo_test = fo$0o(); }
-        "#,
-        expect![[r#"
-                *foo*
-                ```rust
-                {unknown}
-                ```
-            "#]],
-    );
-
     // Use literal `crate` in path
     check(
         r#"
@@ -1194,33 +1150,19 @@ fn test_hover_through_func_in_macro_recursive() {
 macro_rules! id_deep { ($($tt:tt)*) => { $($tt)* } }
 macro_rules! id { ($($tt:tt)*) => { id_deep!($($tt)*) } }
 fn bar() -> u32 { 0 }
-fn foo() { let a = id!([0u32, bar($0)] ); }
+fn foo() { let a = id!([0u32, bar$0()] ); }
 "#,
         expect![[r#"
-                *bar()*
+            *bar*
-                ```rust
-                u32
-                ```
-            "#]],
-    );
-}
 
-#[test]
+            ```rust
-fn test_hover_through_literal_string_in_macro() {
+            test
-    check(
+            ```
-        r#"
+
-macro_rules! arr { ($($tt:tt)*) => { [$($tt)*] } }
+            ```rust
-fn foo() {
+            fn bar() -> u32
-    let mastered_for_itunes = "";
+            ```
-    let _ = arr!("Tr$0acks", &mastered_for_itunes);
+        "#]],
-}
-"#,
-        expect![[r#"
-                *"Tracks"*
-                ```rust
-                &str
-                ```
-            "#]],
     );
 }
 
@@ -5592,3 +5534,81 @@ fn main() {
         "#]],
     );
 }
+
+#[test]
+fn hover_underscore_pat() {
+    check(
+        r#"
+fn main() {
+    let _$0 = 0;
+}
+"#,
+        expect![[r#"
+            *_*
+            ```rust
+            i32
+            ```
+        "#]],
+    );
+    check(
+        r#"
+fn main() {
+    let (_$0,) = (0,);
+}
+"#,
+        expect![[r#"
+            *_*
+            ```rust
+            i32
+            ```
+        "#]],
+    );
+}
+
+#[test]
+fn hover_underscore_expr() {
+    check(
+        r#"
+fn main() {
+    _$0 = 0;
+}
+"#,
+        expect![[r#"
+            *_*
+            ```rust
+            i32
+            ```
+        "#]],
+    );
+    check(
+        r#"
+fn main() {
+    (_$0,) = (0,);
+}
+"#,
+        expect![[r#"
+            *_*
+            ```rust
+            i32
+            ```
+        "#]],
+    );
+}
+
+#[test]
+fn hover_underscore_type() {
+    check_hover_no_result(
+        r#"
+fn main() {
+    let x: _$0 = 0;
+}
+"#,
+    );
+    check_hover_no_result(
+        r#"
+fn main() {
+    let x: (_$0,) = (0,);
+}
+"#,
+    );
+}