From 05723cb50ddf91a57109c4dae6c99ec15cdc3d30 Mon Sep 17 00:00:00 2001 From: Matthew Sanetra Date: Thu, 1 Oct 2020 19:05:39 +0100 Subject: [PATCH 1/2] Add check if param name is similar to fn name --- crates/ide/src/inlay_hints.rs | 108 +++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 49d8e4ae14..b6113bda21 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -1,6 +1,6 @@ use assists::utils::FamousDefs; use either::Either; -use hir::{known, HirDisplay, Semantics}; +use hir::{known, Callable, HirDisplay, Semantics}; use ide_db::RootDatabase; use stdx::to_lower_snake_case; use syntax::{ @@ -170,7 +170,7 @@ fn get_param_name_hints( }; Some((param_name, arg)) }) - .filter(|(param_name, arg)| should_show_param_name_hint(sema, &callable, ¶m_name, &arg)) + .filter(|(param_name, arg)| should_show_param_name_hint(sema, &callable, param_name, &arg)) .map(|(param_name, arg)| InlayHint { range: arg.syntax().text_range(), kind: InlayKind::ParameterHint, @@ -334,9 +334,11 @@ fn should_show_param_name_hint( | hir::CallableKind::TupleEnumVariant(_) | hir::CallableKind::Closure => None, }; + if param_name.is_empty() || Some(param_name) == fn_name.as_ref().map(|s| s.trim_start_matches('_')) || is_argument_similar_to_param_name(sema, argument, param_name) + || is_param_name_similar_to_fn_name(param_name, callable, fn_name.as_ref()) || param_name.starts_with("ra_fixture") { return false; @@ -364,6 +366,26 @@ fn is_argument_similar_to_param_name( } } +fn is_param_name_similar_to_fn_name( + param_name: &str, + callable: &Callable, + fn_name: Option<&String>, +) -> bool { + // if it's the only parameter, don't show it if: + // - is the same as the function name, or + // - the function ends with '_' + param_name + + match (callable.n_params(), fn_name) { + (1, Some(function)) => { + function == param_name + || (function.len() > param_name.len() + && function.ends_with(param_name) + && function[..function.len() - param_name.len()].ends_with('_')) + } + _ => false, + } +} + fn is_enum_name_similar_to_param_name( sema: &Semantics, argument: &ast::Expr, @@ -456,6 +478,88 @@ fn main() { ); } + #[test] + fn param_name_similar_to_fn_name_still_hints() { + check_with_config( + InlayHintsConfig { + parameter_hints: true, + type_hints: false, + chaining_hints: false, + max_length: None, + }, + r#" +fn max(x: i32, y: i32) -> i32 { x + y } +fn main() { + let _x = max( + 4, + //^ x + 4, + //^ y + ); +}"#, + ); + } + + #[test] + fn param_name_similar_to_fn_name() { + check_with_config( + InlayHintsConfig { + parameter_hints: true, + type_hints: false, + chaining_hints: false, + max_length: None, + }, + r#" +fn param_with_underscore(with_underscore: i32) -> i32 { with_underscore } +fn main() { + let _x = param_with_underscore( + 4, + ); +}"#, + ); + } + + #[test] + fn param_name_same_as_fn_name() { + check_with_config( + InlayHintsConfig { + parameter_hints: true, + type_hints: false, + chaining_hints: false, + max_length: None, + }, + r#" +fn foo(foo: i32) -> i32 { foo } +fn main() { + let _x = foo( + 4, + ); +}"#, + ); + } + + #[test] + fn never_hide_param_when_multiple_params() { + check_with_config( + InlayHintsConfig { + parameter_hints: true, + type_hints: false, + chaining_hints: false, + max_length: None, + }, + r#" +fn foo(bar: i32, baz: i32) -> i32 { bar + baz } +fn main() { + let _x = foo( + 4, + //^ bar + 8, + //^ baz + ); +}"#, + ); + } + #[test] fn hints_disabled() { check_with_config( From 042413c35f52286ae57e9add0a8e3fd53a981462 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 29 Oct 2020 10:25:15 +0100 Subject: [PATCH 2/2] Keep generic annotations when qualifying things --- crates/assists/src/handlers/qualify_path.rs | 182 ++++++++++++++++++-- 1 file changed, 169 insertions(+), 13 deletions(-) diff --git a/crates/assists/src/handlers/qualify_path.rs b/crates/assists/src/handlers/qualify_path.rs index f436bdbbfa..d5bc4e574f 100644 --- a/crates/assists/src/handlers/qualify_path.rs +++ b/crates/assists/src/handlers/qualify_path.rs @@ -56,12 +56,14 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> ImportCandidate::QualifierStart(_) => { mark::hit!(qualify_path_qualifier_start); let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; - let segment = path.segment()?; - QualifyCandidate::QualifierStart(segment) + let (prev_segment, segment) = (path.qualifier()?.segment()?, path.segment()?); + QualifyCandidate::QualifierStart(segment, prev_segment.generic_arg_list()) } ImportCandidate::UnqualifiedName(_) => { mark::hit!(qualify_path_unqualified_name); - QualifyCandidate::UnqualifiedName + let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; + let generics = path.segment()?.generic_arg_list(); + QualifyCandidate::UnqualifiedName(generics) } ImportCandidate::TraitAssocItem(_) => { mark::hit!(qualify_path_trait_assoc_item); @@ -96,22 +98,25 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> } enum QualifyCandidate<'db> { - QualifierStart(ast::PathSegment), - UnqualifiedName, + QualifierStart(ast::PathSegment, Option), + UnqualifiedName(Option), TraitAssocItem(ast::Path, ast::PathSegment), TraitMethod(&'db RootDatabase, ast::MethodCallExpr), } impl QualifyCandidate<'_> { fn qualify(&self, mut replacer: impl FnMut(String), import: hir::ModPath, item: hir::ItemInNs) { + let import = mod_path_to_ast(&import); match self { - QualifyCandidate::QualifierStart(segment) => { - let import = mod_path_to_ast(&import); - replacer(format!("{}::{}", import, segment)); + QualifyCandidate::QualifierStart(segment, generics) => { + let generics = generics.as_ref().map_or_else(String::new, ToString::to_string); + replacer(format!("{}{}::{}", import, generics, segment)); + } + QualifyCandidate::UnqualifiedName(generics) => { + let generics = generics.as_ref().map_or_else(String::new, ToString::to_string); + replacer(format!("{}{}", import.to_string(), generics)); } - QualifyCandidate::UnqualifiedName => replacer(mod_path_to_ast(&import).to_string()), QualifyCandidate::TraitAssocItem(qualifier, segment) => { - let import = mod_path_to_ast(&import); replacer(format!("<{} as {}>::{}", qualifier, import, segment)); } &QualifyCandidate::TraitMethod(db, ref mcall_expr) => { @@ -124,25 +129,27 @@ impl QualifyCandidate<'_> { db: &RootDatabase, mcall_expr: &ast::MethodCallExpr, mut replacer: impl FnMut(String), - import: hir::ModPath, + import: ast::Path, item: hir::ItemInNs, ) -> Option<()> { let receiver = mcall_expr.receiver()?; let trait_method_name = mcall_expr.name_ref()?; + let generics = + mcall_expr.generic_arg_list().as_ref().map_or_else(String::new, ToString::to_string); let arg_list = mcall_expr.arg_list().map(|arg_list| arg_list.args()); let trait_ = item_as_trait(item)?; let method = find_trait_method(db, trait_, &trait_method_name)?; if let Some(self_access) = method.self_param(db).map(|sp| sp.access(db)) { - let import = mod_path_to_ast(&import); let receiver = match self_access { hir::Access::Shared => make::expr_ref(receiver, false), hir::Access::Exclusive => make::expr_ref(receiver, true), hir::Access::Owned => receiver, }; replacer(format!( - "{}::{}{}", + "{}::{}{}{}", import, trait_method_name, + generics, match arg_list.clone() { Some(args) => make::arg_list(iter::once(receiver).chain(args)), None => make::arg_list(iter::once(receiver)), @@ -1045,4 +1052,153 @@ fn main() { ", ); } + + #[test] + fn keep_generic_annotations() { + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +pub mod generic { pub struct Thing<'a, T>(&'a T); } + +//- /main.rs crate:main deps:dep +fn foo() -> Thin<|>g<'static, ()> {} + +fn main() {} +", + r" +fn foo() -> dep::generic::Thing<'static, ()> {} + +fn main() {} +", + ); + } + + #[test] + fn keep_generic_annotations_leading_colon() { + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +pub mod generic { pub struct Thing<'a, T>(&'a T); } + +//- /main.rs crate:main deps:dep +fn foo() -> Thin<|>g::<'static, ()> {} + +fn main() {} +", + r" +fn foo() -> dep::generic::Thing::<'static, ()> {} + +fn main() {} +", + ); + } + + #[test] + fn associated_struct_const_generic() { + check_assist( + qualify_path, + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + TestStruct::<()>::TEST_CONST<|> + } + ", + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + test_mod::TestStruct::<()>::TEST_CONST + } + ", + ); + } + + #[test] + fn associated_trait_const_generic() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + const TEST_CONST: u8; + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + test_mod::TestStruct::<()>::TEST_CONST<|> + } + ", + r" + mod test_mod { + pub trait TestTrait { + const TEST_CONST: u8; + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + as test_mod::TestTrait>::TEST_CONST + } + ", + ); + } + + #[test] + fn trait_method_generic() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_struct.test_meth<|>od::<()>() + } + ", + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_mod::TestTrait::test_method::<()>(&test_struct) + } + ", + ); + } }