Merge pull request #19482 from Veykril/push-orrorupvlqpt

fix: Cleanup param name inlay hint filtering

7 files changed, 187 insertions, 118 deletions

diff --git a/src/tools/rust-analyzer/crates/hir/src/attrs.rs b/src/tools/rust-analyzer/crates/hir/src/attrs.rs
index 70637028eff..e71b51bfa43 100644
--- a/src/tools/rust-analyzer/crates/hir/src/attrs.rs
+++ b/src/tools/rust-analyzer/crates/hir/src/attrs.rs
@@ -260,7 +260,7 @@ fn resolve_impl_trait_item(
     // attributes here. Use path resolution directly instead.
     //
     // FIXME: resolve type aliases (which are not yielded by iterate_path_candidates)
-    method_resolution::iterate_path_candidates(
+    _ = method_resolution::iterate_path_candidates(
         &canonical,
         db,
         environment,
diff --git a/src/tools/rust-analyzer/crates/hir/src/lib.rs b/src/tools/rust-analyzer/crates/hir/src/lib.rs
index c5ed0446839..7f0382fa93f 100644
--- a/src/tools/rust-analyzer/crates/hir/src/lib.rs
+++ b/src/tools/rust-analyzer/crates/hir/src/lib.rs
@@ -2922,10 +2922,14 @@ impl Trait {
         db: &dyn HirDatabase,
     ) -> Option<Vec<DynCompatibilityViolation>> {
         let mut violations = vec![];
-        hir_ty::dyn_compatibility::dyn_compatibility_with_callback(db, self.id, &mut |violation| {
-            violations.push(violation);
-            ControlFlow::Continue(())
-        });
+        _ = hir_ty::dyn_compatibility::dyn_compatibility_with_callback(
+            db,
+            self.id,
+            &mut |violation| {
+                violations.push(violation);
+                ControlFlow::Continue(())
+            },
+        );
         violations.is_empty().not().then_some(violations)
     }
 
@@ -5514,7 +5518,7 @@ impl Type {
             .generic_def()
             .map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d));
 
-        method_resolution::iterate_method_candidates_dyn(
+        _ = method_resolution::iterate_method_candidates_dyn(
             &canonical,
             db,
             environment,
@@ -5601,7 +5605,7 @@ impl Type {
             .generic_def()
             .map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d));
 
-        method_resolution::iterate_path_candidates(
+        _ = method_resolution::iterate_path_candidates(
             &canonical,
             db,
             environment,
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_function.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_function.rs
index abc0698b014..5a6a7ede352 100644
--- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_function.rs
+++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_function.rs
@@ -751,7 +751,7 @@ impl FunctionBody {
                         ast::Stmt::Item(_) => (),
                         ast::Stmt::LetStmt(stmt) => {
                             if let Some(pat) = stmt.pat() {
-                                walk_pat(&pat, &mut |pat| {
+                                _ = walk_pat(&pat, &mut |pat| {
                                     cb(pat);
                                     std::ops::ControlFlow::<(), ()>::Continue(())
                                 });
diff --git a/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/node_ext.rs b/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/node_ext.rs
index cd47524caa3..bdff64dd081 100644
--- a/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/node_ext.rs
+++ b/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/node_ext.rs
@@ -121,7 +121,7 @@ pub fn walk_patterns_in_expr(start: &ast::Expr, cb: &mut dyn FnMut(ast::Pat)) {
         match ast::Stmt::cast(node.clone()) {
             Some(ast::Stmt::LetStmt(l)) => {
                 if let Some(pat) = l.pat() {
-                    walk_pat(&pat, &mut |pat| {
+                    _ = walk_pat(&pat, &mut |pat| {
                         cb(pat);
                         ControlFlow::<(), ()>::Continue(())
                     });
@@ -159,7 +159,7 @@ pub fn walk_patterns_in_expr(start: &ast::Expr, cb: &mut dyn FnMut(ast::Pat)) {
                     }
                 } else if let Some(pat) = ast::Pat::cast(node) {
                     preorder.skip_subtree();
-                    walk_pat(&pat, &mut |pat| {
+                    _ = walk_pat(&pat, &mut |pat| {
                         cb(pat);
                         ControlFlow::<(), ()>::Continue(())
                     });
diff --git a/src/tools/rust-analyzer/crates/ide/src/inlay_hints/generic_param.rs b/src/tools/rust-analyzer/crates/ide/src/inlay_hints/generic_param.rs
index fc1083fdca6..730732df860 100644
--- a/src/tools/rust-analyzer/crates/ide/src/inlay_hints/generic_param.rs
+++ b/src/tools/rust-analyzer/crates/ide/src/inlay_hints/generic_param.rs
@@ -1,4 +1,5 @@
 //! Implementation of inlay hints for generic parameters.
+use either::Either;
 use ide_db::{active_parameter::generic_def_for_node, famous_defs::FamousDefs};
 use syntax::{
     AstNode,
@@ -6,7 +7,8 @@ use syntax::{
 };
 
 use crate::{
-    InlayHint, InlayHintLabel, InlayHintsConfig, InlayKind, inlay_hints::GenericParameterHints,
+    InlayHint, InlayHintLabel, InlayHintsConfig, InlayKind,
+    inlay_hints::{GenericParameterHints, param_name},
 };
 
 use super::param_name::is_argument_similar_to_param_name;
@@ -62,8 +64,17 @@ pub(crate) fn hints(
         let param_name = param.name(sema.db);
 
         let should_hide = {
-            let argument = get_string_representation(&arg)?;
-            is_argument_similar_to_param_name(&argument, param_name.as_str())
+            let param_name = param_name.as_str();
+            get_segment_representation(&arg).map_or(false, |seg| match seg {
+                Either::Left(Either::Left(argument)) => {
+                    is_argument_similar_to_param_name(&argument, param_name)
+                }
+                Either::Left(Either::Right(argument)) => argument
+                    .segment()
+                    .and_then(|it| it.name_ref())
+                    .is_some_and(|it| it.text().eq_ignore_ascii_case(param_name)),
+                Either::Right(lifetime) => lifetime.text().eq_ignore_ascii_case(param_name),
+            })
         };
 
         if should_hide {
@@ -111,32 +122,34 @@ pub(crate) fn hints(
     Some(())
 }
 
-fn get_string_representation(arg: &ast::GenericArg) -> Option<String> {
+fn get_segment_representation(
+    arg: &ast::GenericArg,
+) -> Option<Either<Either<Vec<ast::NameRef>, ast::Path>, ast::Lifetime>> {
     return match arg {
         ast::GenericArg::AssocTypeArg(_) => None,
-        ast::GenericArg::ConstArg(const_arg) => Some(const_arg.to_string()),
+        ast::GenericArg::ConstArg(const_arg) => {
+            param_name::get_segment_representation(&const_arg.expr()?).map(Either::Left)
+        }
         ast::GenericArg::LifetimeArg(lifetime_arg) => {
             let lifetime = lifetime_arg.lifetime()?;
-            Some(lifetime.to_string())
+            Some(Either::Right(lifetime))
         }
         ast::GenericArg::TypeArg(type_arg) => {
             let ty = type_arg.ty()?;
-            Some(
-                type_path_segment(&ty)
-                    .map_or_else(|| type_arg.to_string(), |segment| segment.to_string()),
-            )
+            type_path(&ty).map(Either::Right).map(Either::Left)
         }
     };
 
-    fn type_path_segment(ty: &ast::Type) -> Option<ast::PathSegment> {
+    fn type_path(ty: &ast::Type) -> Option<ast::Path> {
         match ty {
-            ast::Type::ArrayType(it) => type_path_segment(&it.ty()?),
-            ast::Type::ForType(it) => type_path_segment(&it.ty()?),
-            ast::Type::ParenType(it) => type_path_segment(&it.ty()?),
-            ast::Type::PathType(path_type) => path_type.path()?.segment(),
-            ast::Type::PtrType(it) => type_path_segment(&it.ty()?),
-            ast::Type::RefType(it) => type_path_segment(&it.ty()?),
-            ast::Type::SliceType(it) => type_path_segment(&it.ty()?),
+            ast::Type::ArrayType(it) => type_path(&it.ty()?),
+            ast::Type::ForType(it) => type_path(&it.ty()?),
+            ast::Type::ParenType(it) => type_path(&it.ty()?),
+            ast::Type::PathType(path_type) => path_type.path(),
+            ast::Type::PtrType(it) => type_path(&it.ty()?),
+            ast::Type::RefType(it) => type_path(&it.ty()?),
+            ast::Type::SliceType(it) => type_path(&it.ty()?),
+            ast::Type::MacroType(macro_type) => macro_type.macro_call()?.path(),
             _ => None,
         }
     }
diff --git a/src/tools/rust-analyzer/crates/ide/src/inlay_hints/param_name.rs b/src/tools/rust-analyzer/crates/ide/src/inlay_hints/param_name.rs
index 44ea5351fbd..99c698ce029 100644
--- a/src/tools/rust-analyzer/crates/ide/src/inlay_hints/param_name.rs
+++ b/src/tools/rust-analyzer/crates/ide/src/inlay_hints/param_name.rs
@@ -4,16 +4,15 @@
 //! _ = max(/*x*/4, /*y*/4);
 //! ```
 
+use std::iter::zip;
+
 use either::Either;
-use hir::{Callable, Semantics};
+use hir::Semantics;
 use ide_db::{RootDatabase, famous_defs::FamousDefs};
 
 use span::EditionedFileId;
 use stdx::to_lower_snake_case;
-use syntax::{
-    ToSmolStr,
-    ast::{self, AstNode, HasArgList, HasName, UnaryOp},
-};
+use syntax::ast::{self, AstNode, HasArgList, HasName, UnaryOp};
 
 use crate::{InlayHint, InlayHintLabel, InlayHintPosition, InlayHintsConfig, InlayKind};
 
@@ -29,6 +28,12 @@ pub(super) fn hints(
     }
 
     let (callable, arg_list) = get_callable(sema, &expr)?;
+    let unary_function = callable.n_params() == 1;
+    let function_name = match callable.kind() {
+        hir::CallableKind::Function(function) => Some(function.name(sema.db)),
+        _ => None,
+    };
+    let function_name = function_name.as_ref().map(|it| it.as_str());
     let hints = callable
         .params()
         .into_iter()
@@ -40,7 +45,13 @@ pub(super) fn hints(
             Some((p, param_name, arg, range))
         })
         .filter(|(_, param_name, arg, _)| {
-            !should_hide_param_name_hint(sema, &callable, param_name.as_str(), arg)
+            !should_hide_param_name_hint(
+                sema,
+                unary_function,
+                function_name,
+                param_name.as_str(),
+                arg,
+            )
         })
         .map(|(param, param_name, _, hir::FileRange { range, .. })| {
             let colon = if config.render_colons { ":" } else { "" };
@@ -94,9 +105,13 @@ fn get_callable(
     }
 }
 
+const INSIGNIFICANT_METHOD_NAMES: &[&str] = &["clone", "as_ref", "into"];
+const INSIGNIFICANT_PARAMETER_NAMES: &[&str] = &["predicate", "value", "pat", "rhs", "other"];
+
 fn should_hide_param_name_hint(
     sema: &Semantics<'_, RootDatabase>,
-    callable: &hir::Callable,
+    unary_function: bool,
+    function_name: Option<&str>,
     param_name: &str,
     argument: &ast::Expr,
 ) -> bool {
@@ -114,95 +129,128 @@ fn should_hide_param_name_hint(
         return true;
     }
 
-    if matches!(argument, ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(UnaryOp::Not)) {
-        return false;
+    if param_name.starts_with("ra_fixture") {
+        return true;
     }
 
-    let fn_name = match callable.kind() {
-        hir::CallableKind::Function(it) => Some(it.name(sema.db).as_str().to_smolstr()),
-        _ => None,
-    };
-    let fn_name = fn_name.as_deref();
-    is_param_name_suffix_of_fn_name(param_name, callable, fn_name)
-        || is_argument_expr_similar_to_param_name(argument, param_name)
-        || param_name.starts_with("ra_fixture")
-        || (callable.n_params() == 1 && is_obvious_param(param_name))
-        || is_adt_constructor_similar_to_param_name(sema, argument, param_name)
+    if unary_function {
+        if let Some(function_name) = function_name {
+            if is_param_name_suffix_of_fn_name(param_name, function_name) {
+                return true;
+            }
+        }
+        if is_obvious_param(param_name) {
+            return true;
+        }
+    }
+
+    is_argument_expr_similar_to_param_name(sema, argument, param_name)
 }
 
 /// Hide the parameter name of a unary function if it is a `_` - prefixed suffix of the function's name, or equal.
 ///
 /// `fn strip_suffix(suffix)` will be hidden.
 /// `fn stripsuffix(suffix)` will not be hidden.
-fn is_param_name_suffix_of_fn_name(
+fn is_param_name_suffix_of_fn_name(param_name: &str, fn_name: &str) -> bool {
+    fn_name == param_name
+        || fn_name
+            .len()
+            .checked_sub(param_name.len())
+            .and_then(|at| fn_name.is_char_boundary(at).then(|| fn_name.split_at(at)))
+            .is_some_and(|(prefix, suffix)| {
+                suffix.eq_ignore_ascii_case(param_name) && prefix.ends_with('_')
+            })
+}
+
+fn is_argument_expr_similar_to_param_name(
+    sema: &Semantics<'_, RootDatabase>,
+    argument: &ast::Expr,
     param_name: &str,
-    callable: &Callable,
-    fn_name: Option<&str>,
 ) -> bool {
-    match (callable.n_params(), fn_name) {
-        (1, Some(function)) => {
-            function == param_name
-                || function
-                    .len()
-                    .checked_sub(param_name.len())
-                    .and_then(|at| function.is_char_boundary(at).then(|| function.split_at(at)))
-                    .is_some_and(|(prefix, suffix)| {
-                        suffix.eq_ignore_ascii_case(param_name) && prefix.ends_with('_')
-                    })
+    match get_segment_representation(argument) {
+        Some(Either::Left(argument)) => is_argument_similar_to_param_name(&argument, param_name),
+        Some(Either::Right(path)) => {
+            path.segment()
+                .and_then(|it| it.name_ref())
+                .is_some_and(|name_ref| name_ref.text().eq_ignore_ascii_case(param_name))
+                || is_adt_constructor_similar_to_param_name(sema, &path, param_name)
         }
-        _ => false,
+        None => false,
     }
 }
 
-fn is_argument_expr_similar_to_param_name(argument: &ast::Expr, param_name: &str) -> bool {
-    let argument = match get_string_representation(argument) {
-        Some(argument) => argument,
-        None => return false,
-    };
-    is_argument_similar_to_param_name(&argument, param_name)
-}
-
 /// Check whether param_name and argument are the same or
 /// whether param_name is a prefix/suffix of argument(split at `_`).
-pub(super) fn is_argument_similar_to_param_name(argument: &str, param_name: &str) -> bool {
-    // std is honestly too panic happy...
-    let str_split_at = |str: &str, at| str.is_char_boundary(at).then(|| argument.split_at(at));
-
-    let param_name = param_name.trim_start_matches('_');
-    let argument = argument.trim_start_matches('_');
-
-    match str_split_at(argument, param_name.len()) {
-        Some((prefix, rest)) if prefix.eq_ignore_ascii_case(param_name) => {
-            return rest.is_empty() || rest.starts_with('_');
-        }
-        _ => (),
-    }
-    match argument.len().checked_sub(param_name.len()).and_then(|at| str_split_at(argument, at)) {
-        Some((rest, suffix)) if param_name.eq_ignore_ascii_case(suffix) => {
-            return rest.is_empty() || rest.ends_with('_');
-        }
-        _ => (),
-    }
-    false
+pub(super) fn is_argument_similar_to_param_name(
+    argument: &[ast::NameRef],
+    param_name: &str,
+) -> bool {
+    debug_assert!(!argument.is_empty());
+    debug_assert!(!param_name.is_empty());
+    let param_name = param_name.split('_');
+    let argument = argument.iter().flat_map(|it| it.text_non_mutable().split('_'));
+
+    let prefix_match = zip(argument.clone(), param_name.clone())
+        .all(|(arg, param)| arg.eq_ignore_ascii_case(param));
+    let postfix_match = || {
+        zip(argument.rev(), param_name.rev()).all(|(arg, param)| arg.eq_ignore_ascii_case(param))
+    };
+    prefix_match || postfix_match()
 }
 
-fn get_string_representation(expr: &ast::Expr) -> Option<String> {
+pub(super) fn get_segment_representation(
+    expr: &ast::Expr,
+) -> Option<Either<Vec<ast::NameRef>, ast::Path>> {
     match expr {
         ast::Expr::MethodCallExpr(method_call_expr) => {
+            let receiver =
+                method_call_expr.receiver().and_then(|expr| get_segment_representation(&expr));
             let name_ref = method_call_expr.name_ref()?;
-            match name_ref.text().as_str() {
-                "clone" | "as_ref" => method_call_expr.receiver().map(|rec| rec.to_string()),
-                name_ref => Some(name_ref.to_owned()),
+            if INSIGNIFICANT_METHOD_NAMES.contains(&name_ref.text().as_str()) {
+                return receiver;
             }
+            Some(Either::Left(match receiver {
+                Some(Either::Left(mut left)) => {
+                    left.push(name_ref);
+                    left
+                }
+                Some(Either::Right(_)) | None => vec![name_ref],
+            }))
+        }
+        ast::Expr::FieldExpr(field_expr) => {
+            let expr = field_expr.expr().and_then(|expr| get_segment_representation(&expr));
+            let name_ref = field_expr.name_ref()?;
+            let res = match expr {
+                Some(Either::Left(mut left)) => {
+                    left.push(name_ref);
+                    left
+                }
+                Some(Either::Right(_)) | None => vec![name_ref],
+            };
+            Some(Either::Left(res))
         }
-        ast::Expr::MacroExpr(macro_expr) => {
-            Some(macro_expr.macro_call()?.path()?.segment()?.to_string())
+        // paths
+        ast::Expr::MacroExpr(macro_expr) => macro_expr.macro_call()?.path().map(Either::Right),
+        ast::Expr::RecordExpr(record_expr) => record_expr.path().map(Either::Right),
+        ast::Expr::PathExpr(path_expr) => {
+            let path = path_expr.path()?;
+            // single segment paths are likely locals
+            Some(match path.as_single_name_ref() {
+                None => Either::Right(path),
+                Some(name_ref) => Either::Left(vec![name_ref]),
+            })
         }
-        ast::Expr::FieldExpr(field_expr) => Some(field_expr.name_ref()?.to_string()),
-        ast::Expr::PathExpr(path_expr) => Some(path_expr.path()?.segment()?.to_string()),
-        ast::Expr::PrefixExpr(prefix_expr) => get_string_representation(&prefix_expr.expr()?),
-        ast::Expr::RefExpr(ref_expr) => get_string_representation(&ref_expr.expr()?),
-        ast::Expr::CastExpr(cast_expr) => get_string_representation(&cast_expr.expr()?),
+        ast::Expr::PrefixExpr(prefix_expr) if prefix_expr.op_kind() == Some(UnaryOp::Not) => None,
+        // recurse
+        ast::Expr::PrefixExpr(prefix_expr) => get_segment_representation(&prefix_expr.expr()?),
+        ast::Expr::RefExpr(ref_expr) => get_segment_representation(&ref_expr.expr()?),
+        ast::Expr::CastExpr(cast_expr) => get_segment_representation(&cast_expr.expr()?),
+        ast::Expr::CallExpr(call_expr) => get_segment_representation(&call_expr.expr()?),
+        ast::Expr::AwaitExpr(await_expr) => get_segment_representation(&await_expr.expr()?),
+        ast::Expr::IndexExpr(index_expr) => get_segment_representation(&index_expr.base()?),
+        ast::Expr::ParenExpr(paren_expr) => get_segment_representation(&paren_expr.expr()?),
+        ast::Expr::TryExpr(try_expr) => get_segment_representation(&try_expr.expr()?),
+        // ast::Expr::ClosureExpr(closure_expr) => todo!(),
         _ => None,
     }
 }
@@ -210,30 +258,15 @@ fn get_string_representation(expr: &ast::Expr) -> Option<String> {
 fn is_obvious_param(param_name: &str) -> bool {
     // avoid displaying hints for common functions like map, filter, etc.
     // or other obvious words used in std
-    let is_obvious_param_name =
-        matches!(param_name, "predicate" | "value" | "pat" | "rhs" | "other");
-    param_name.len() == 1 || is_obvious_param_name
+    param_name.len() == 1 || INSIGNIFICANT_PARAMETER_NAMES.contains(&param_name)
 }
 
 fn is_adt_constructor_similar_to_param_name(
     sema: &Semantics<'_, RootDatabase>,
-    argument: &ast::Expr,
+    path: &ast::Path,
     param_name: &str,
 ) -> bool {
-    let path = match argument {
-        ast::Expr::CallExpr(c) => c.expr().and_then(|e| match e {
-            ast::Expr::PathExpr(p) => p.path(),
-            _ => None,
-        }),
-        ast::Expr::PathExpr(p) => p.path(),
-        ast::Expr::RecordExpr(r) => r.path(),
-        _ => return false,
-    };
-    let path = match path {
-        Some(it) => it,
-        None => return false,
-    };
-    (|| match sema.resolve_path(&path)? {
+    (|| match sema.resolve_path(path)? {
         hir::PathResolution::Def(hir::ModuleDef::Adt(_)) => {
             Some(to_lower_snake_case(&path.segment()?.name_ref()?.text()) == param_name)
         }
@@ -501,6 +534,7 @@ fn enum_matches_param_name(completion_kind: CompletionKind) {}
 
 fn foo(param: u32) {}
 fn bar(param_eter: u32) {}
+fn baz(a_d_e: u32) {}
 
 enum CompletionKind {
     Keyword,
@@ -553,6 +587,14 @@ fn main() {
       //^^^^^^^^^^^ param_eter
 
     non_ident_pat((0, 0));
+
+    baz(a.d.e);
+    baz(a.dc.e);
+     // ^^^^^^ a_d_e
+    baz(ac.d.e);
+     // ^^^^^^ a_d_e
+    baz(a.d.ec);
+     // ^^^^^^ a_d_e
 }"#,
         );
     }
diff --git a/src/tools/rust-analyzer/crates/syntax/src/ast/node_ext.rs b/src/tools/rust-analyzer/crates/syntax/src/ast/node_ext.rs
index a5ac59dd7d7..b9ccd34cff0 100644
--- a/src/tools/rust-analyzer/crates/syntax/src/ast/node_ext.rs
+++ b/src/tools/rust-analyzer/crates/syntax/src/ast/node_ext.rs
@@ -36,6 +36,16 @@ impl ast::NameRef {
     pub fn text(&self) -> TokenText<'_> {
         text_of_first_token(self.syntax())
     }
+    pub fn text_non_mutable(&self) -> &str {
+        fn first_token(green_ref: &GreenNodeData) -> &GreenTokenData {
+            green_ref.children().next().and_then(NodeOrToken::into_token).unwrap()
+        }
+
+        match self.syntax().green() {
+            Cow::Borrowed(green_ref) => first_token(green_ref).text(),
+            Cow::Owned(_) => unreachable!(),
+        }
+    }
 
     pub fn as_tuple_field(&self) -> Option<usize> {
         self.text().parse().ok()