fix: Cleanup param name inlay hint filtering

This commit is contained in:
Lukas Wirth 2025-03-31 07:43:08 +02:00
parent fb133c8c80
commit 2b382eb772
7 changed files with 188 additions and 119 deletions

View File

@ -260,7 +260,7 @@ fn resolve_impl_trait_item(
// attributes here. Use path resolution directly instead. // attributes here. Use path resolution directly instead.
// //
// FIXME: resolve type aliases (which are not yielded by iterate_path_candidates) // FIXME: resolve type aliases (which are not yielded by iterate_path_candidates)
method_resolution::iterate_path_candidates( _ = method_resolution::iterate_path_candidates(
&canonical, &canonical,
db, db,
environment, environment,

View File

@ -2922,10 +2922,14 @@ impl Trait {
db: &dyn HirDatabase, db: &dyn HirDatabase,
) -> Option<Vec<DynCompatibilityViolation>> { ) -> Option<Vec<DynCompatibilityViolation>> {
let mut violations = vec![]; let mut violations = vec![];
hir_ty::dyn_compatibility::dyn_compatibility_with_callback(db, self.id, &mut |violation| { _ = hir_ty::dyn_compatibility::dyn_compatibility_with_callback(
violations.push(violation); db,
ControlFlow::Continue(()) self.id,
}); &mut |violation| {
violations.push(violation);
ControlFlow::Continue(())
},
);
violations.is_empty().not().then_some(violations) violations.is_empty().not().then_some(violations)
} }
@ -5514,7 +5518,7 @@ impl Type {
.generic_def() .generic_def()
.map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d)); .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, &canonical,
db, db,
environment, environment,
@ -5601,7 +5605,7 @@ impl Type {
.generic_def() .generic_def()
.map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d)); .map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d));
method_resolution::iterate_path_candidates( _ = method_resolution::iterate_path_candidates(
&canonical, &canonical,
db, db,
environment, environment,

View File

@ -751,7 +751,7 @@ impl FunctionBody {
ast::Stmt::Item(_) => (), ast::Stmt::Item(_) => (),
ast::Stmt::LetStmt(stmt) => { ast::Stmt::LetStmt(stmt) => {
if let Some(pat) = stmt.pat() { if let Some(pat) = stmt.pat() {
walk_pat(&pat, &mut |pat| { _ = walk_pat(&pat, &mut |pat| {
cb(pat); cb(pat);
std::ops::ControlFlow::<(), ()>::Continue(()) std::ops::ControlFlow::<(), ()>::Continue(())
}); });

View File

@ -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()) { match ast::Stmt::cast(node.clone()) {
Some(ast::Stmt::LetStmt(l)) => { Some(ast::Stmt::LetStmt(l)) => {
if let Some(pat) = l.pat() { if let Some(pat) = l.pat() {
walk_pat(&pat, &mut |pat| { _ = walk_pat(&pat, &mut |pat| {
cb(pat); cb(pat);
ControlFlow::<(), ()>::Continue(()) 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) { } else if let Some(pat) = ast::Pat::cast(node) {
preorder.skip_subtree(); preorder.skip_subtree();
walk_pat(&pat, &mut |pat| { _ = walk_pat(&pat, &mut |pat| {
cb(pat); cb(pat);
ControlFlow::<(), ()>::Continue(()) ControlFlow::<(), ()>::Continue(())
}); });

View File

@ -1,4 +1,5 @@
//! Implementation of inlay hints for generic parameters. //! Implementation of inlay hints for generic parameters.
use either::Either;
use ide_db::{active_parameter::generic_def_for_node, famous_defs::FamousDefs}; use ide_db::{active_parameter::generic_def_for_node, famous_defs::FamousDefs};
use syntax::{ use syntax::{
AstNode, AstNode,
@ -6,7 +7,8 @@ use syntax::{
}; };
use crate::{ 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; 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 param_name = param.name(sema.db);
let should_hide = { let should_hide = {
let argument = get_string_representation(&arg)?; let param_name = param_name.as_str();
is_argument_similar_to_param_name(&argument, 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 { if should_hide {
@ -111,32 +122,34 @@ pub(crate) fn hints(
Some(()) 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 { return match arg {
ast::GenericArg::AssocTypeArg(_) => None, 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) => { ast::GenericArg::LifetimeArg(lifetime_arg) => {
let lifetime = lifetime_arg.lifetime()?; let lifetime = lifetime_arg.lifetime()?;
Some(lifetime.to_string()) Some(Either::Right(lifetime))
} }
ast::GenericArg::TypeArg(type_arg) => { ast::GenericArg::TypeArg(type_arg) => {
let ty = type_arg.ty()?; let ty = type_arg.ty()?;
Some( type_path(&ty).map(Either::Right).map(Either::Left)
type_path_segment(&ty)
.map_or_else(|| type_arg.to_string(), |segment| segment.to_string()),
)
} }
}; };
fn type_path_segment(ty: &ast::Type) -> Option<ast::PathSegment> { fn type_path(ty: &ast::Type) -> Option<ast::Path> {
match ty { match ty {
ast::Type::ArrayType(it) => type_path_segment(&it.ty()?), ast::Type::ArrayType(it) => type_path(&it.ty()?),
ast::Type::ForType(it) => type_path_segment(&it.ty()?), ast::Type::ForType(it) => type_path(&it.ty()?),
ast::Type::ParenType(it) => type_path_segment(&it.ty()?), ast::Type::ParenType(it) => type_path(&it.ty()?),
ast::Type::PathType(path_type) => path_type.path()?.segment(), ast::Type::PathType(path_type) => path_type.path(),
ast::Type::PtrType(it) => type_path_segment(&it.ty()?), ast::Type::PtrType(it) => type_path(&it.ty()?),
ast::Type::RefType(it) => type_path_segment(&it.ty()?), ast::Type::RefType(it) => type_path(&it.ty()?),
ast::Type::SliceType(it) => type_path_segment(&it.ty()?), ast::Type::SliceType(it) => type_path(&it.ty()?),
ast::Type::MacroType(macro_type) => macro_type.macro_call()?.path(),
_ => None, _ => None,
} }
} }

View File

@ -4,16 +4,15 @@
//! _ = max(/*x*/4, /*y*/4); //! _ = max(/*x*/4, /*y*/4);
//! ``` //! ```
use std::iter::zip;
use either::Either; use either::Either;
use hir::{Callable, Semantics}; use hir::Semantics;
use ide_db::{RootDatabase, famous_defs::FamousDefs}; use ide_db::{RootDatabase, famous_defs::FamousDefs};
use span::EditionedFileId; use span::EditionedFileId;
use stdx::to_lower_snake_case; use stdx::to_lower_snake_case;
use syntax::{ use syntax::ast::{self, AstNode, HasArgList, HasName, UnaryOp};
ToSmolStr,
ast::{self, AstNode, HasArgList, HasName, UnaryOp},
};
use crate::{InlayHint, InlayHintLabel, InlayHintPosition, InlayHintsConfig, InlayKind}; use crate::{InlayHint, InlayHintLabel, InlayHintPosition, InlayHintsConfig, InlayKind};
@ -29,6 +28,12 @@ pub(super) fn hints(
} }
let (callable, arg_list) = get_callable(sema, &expr)?; 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 let hints = callable
.params() .params()
.into_iter() .into_iter()
@ -40,7 +45,13 @@ pub(super) fn hints(
Some((p, param_name, arg, range)) Some((p, param_name, arg, range))
}) })
.filter(|(_, param_name, arg, _)| { .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, .. })| { .map(|(param, param_name, _, hir::FileRange { range, .. })| {
let colon = if config.render_colons { ":" } else { "" }; 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( fn should_hide_param_name_hint(
sema: &Semantics<'_, RootDatabase>, sema: &Semantics<'_, RootDatabase>,
callable: &hir::Callable, unary_function: bool,
function_name: Option<&str>,
param_name: &str, param_name: &str,
argument: &ast::Expr, argument: &ast::Expr,
) -> bool { ) -> bool {
@ -114,95 +129,128 @@ fn should_hide_param_name_hint(
return true; return true;
} }
if matches!(argument, ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(UnaryOp::Not)) { if param_name.starts_with("ra_fixture") {
return false; return true;
} }
let fn_name = match callable.kind() { if unary_function {
hir::CallableKind::Function(it) => Some(it.name(sema.db).as_str().to_smolstr()), if let Some(function_name) = function_name {
_ => None, if is_param_name_suffix_of_fn_name(param_name, function_name) {
}; return true;
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) if is_obvious_param(param_name) {
|| param_name.starts_with("ra_fixture") return true;
|| (callable.n_params() == 1 && is_obvious_param(param_name)) }
|| is_adt_constructor_similar_to_param_name(sema, argument, param_name) }
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. /// 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 strip_suffix(suffix)` will be hidden.
/// `fn stripsuffix(suffix)` will not 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 {
param_name: &str, fn_name == param_name
callable: &Callable, || fn_name
fn_name: Option<&str>, .len()
) -> bool { .checked_sub(param_name.len())
match (callable.n_params(), fn_name) { .and_then(|at| fn_name.is_char_boundary(at).then(|| fn_name.split_at(at)))
(1, Some(function)) => { .is_some_and(|(prefix, suffix)| {
function == param_name suffix.eq_ignore_ascii_case(param_name) && prefix.ends_with('_')
|| 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('_')
})
}
_ => false,
}
} }
fn is_argument_expr_similar_to_param_name(argument: &ast::Expr, param_name: &str) -> bool { fn is_argument_expr_similar_to_param_name(
let argument = match get_string_representation(argument) { sema: &Semantics<'_, RootDatabase>,
Some(argument) => argument, argument: &ast::Expr,
None => return false, param_name: &str,
}; ) -> bool {
is_argument_similar_to_param_name(&argument, param_name) 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)
}
None => false,
}
} }
/// Check whether param_name and argument are the same or /// Check whether param_name and argument are the same or
/// whether param_name is a prefix/suffix of argument(split at `_`). /// 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 { pub(super) fn is_argument_similar_to_param_name(
// std is honestly too panic happy... argument: &[ast::NameRef],
let str_split_at = |str: &str, at| str.is_char_boundary(at).then(|| argument.split_at(at)); 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 param_name = param_name.trim_start_matches('_'); let prefix_match = zip(argument.clone(), param_name.clone())
let argument = argument.trim_start_matches('_'); .all(|(arg, param)| arg.eq_ignore_ascii_case(param));
let postfix_match = || {
match str_split_at(argument, param_name.len()) { zip(argument.rev(), param_name.rev()).all(|(arg, param)| arg.eq_ignore_ascii_case(param))
Some((prefix, rest)) if prefix.eq_ignore_ascii_case(param_name) => { };
return rest.is_empty() || rest.starts_with('_'); prefix_match || postfix_match()
}
_ => (),
}
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
} }
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 { match expr {
ast::Expr::MethodCallExpr(method_call_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()?; let name_ref = method_call_expr.name_ref()?;
match name_ref.text().as_str() { if INSIGNIFICANT_METHOD_NAMES.contains(&name_ref.text().as_str()) {
"clone" | "as_ref" => method_call_expr.receiver().map(|rec| rec.to_string()), return receiver;
name_ref => Some(name_ref.to_owned()),
} }
Some(Either::Left(match receiver {
Some(Either::Left(mut left)) => {
left.push(name_ref);
left
}
Some(Either::Right(_)) | None => vec![name_ref],
}))
} }
ast::Expr::MacroExpr(macro_expr) => { ast::Expr::FieldExpr(field_expr) => {
Some(macro_expr.macro_call()?.path()?.segment()?.to_string()) 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::FieldExpr(field_expr) => Some(field_expr.name_ref()?.to_string()), // paths
ast::Expr::PathExpr(path_expr) => Some(path_expr.path()?.segment()?.to_string()), ast::Expr::MacroExpr(macro_expr) => macro_expr.macro_call()?.path().map(Either::Right),
ast::Expr::PrefixExpr(prefix_expr) => get_string_representation(&prefix_expr.expr()?), ast::Expr::RecordExpr(record_expr) => record_expr.path().map(Either::Right),
ast::Expr::RefExpr(ref_expr) => get_string_representation(&ref_expr.expr()?), ast::Expr::PathExpr(path_expr) => {
ast::Expr::CastExpr(cast_expr) => get_string_representation(&cast_expr.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::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, _ => None,
} }
} }
@ -210,30 +258,15 @@ fn get_string_representation(expr: &ast::Expr) -> Option<String> {
fn is_obvious_param(param_name: &str) -> bool { fn is_obvious_param(param_name: &str) -> bool {
// avoid displaying hints for common functions like map, filter, etc. // avoid displaying hints for common functions like map, filter, etc.
// or other obvious words used in std // or other obvious words used in std
let is_obvious_param_name = param_name.len() == 1 || INSIGNIFICANT_PARAMETER_NAMES.contains(&param_name)
matches!(param_name, "predicate" | "value" | "pat" | "rhs" | "other");
param_name.len() == 1 || is_obvious_param_name
} }
fn is_adt_constructor_similar_to_param_name( fn is_adt_constructor_similar_to_param_name(
sema: &Semantics<'_, RootDatabase>, sema: &Semantics<'_, RootDatabase>,
argument: &ast::Expr, path: &ast::Path,
param_name: &str, param_name: &str,
) -> bool { ) -> bool {
let path = match argument { (|| match sema.resolve_path(path)? {
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)? {
hir::PathResolution::Def(hir::ModuleDef::Adt(_)) => { hir::PathResolution::Def(hir::ModuleDef::Adt(_)) => {
Some(to_lower_snake_case(&path.segment()?.name_ref()?.text()) == param_name) 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 foo(param: u32) {}
fn bar(param_eter: u32) {} fn bar(param_eter: u32) {}
fn baz(a_d_e: u32) {}
enum CompletionKind { enum CompletionKind {
Keyword, Keyword,
@ -553,6 +587,14 @@ fn main() {
//^^^^^^^^^^^ param_eter //^^^^^^^^^^^ param_eter
non_ident_pat((0, 0)); 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
}"#, }"#,
); );
} }

View File

@ -36,6 +36,16 @@ impl ast::NameRef {
pub fn text(&self) -> TokenText<'_> { pub fn text(&self) -> TokenText<'_> {
text_of_first_token(self.syntax()) 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> { pub fn as_tuple_field(&self) -> Option<usize> {
self.text().parse().ok() self.text().parse().ok()