From 21d2cebcf1a417bce72da98aa638a20235c050db Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Sat, 1 Aug 2020 12:43:10 +1000 Subject: [PATCH] SSR: Matching trait associated constants, types and functions This fixes matching of things like `HashMap::default()` by resolving `HashMap` instead of `default` (which resolves to `Default::default`). Same for associated constants and types that are part of a trait implementation. However, we still don't support matching calls to trait methods. --- crates/ra_ide/src/ssr.rs | 4 +-- crates/ra_ssr/src/resolving.rs | 31 +++++++++++++--- crates/ra_ssr/src/tests.rs | 64 ++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/crates/ra_ide/src/ssr.rs b/crates/ra_ide/src/ssr.rs index 4348b43beb5..8be862fd6e1 100644 --- a/crates/ra_ide/src/ssr.rs +++ b/crates/ra_ide/src/ssr.rs @@ -21,8 +21,8 @@ // replacement occurs. For example if our replacement template is `foo::Bar` and we match some // code in the `foo` module, we'll insert just `Bar`. // -// Method calls should generally be written in UFCS form. e.g. `foo::Bar::baz($s, $a)` will match -// `$s.baz($a)`, provided the method call `baz` resolves to the method `foo::Bar::baz`. +// Inherent method calls should generally be written in UFCS form. e.g. `foo::Bar::baz($s, $a)` will +// match `$s.baz($a)`, provided the method call `baz` resolves to the method `foo::Bar::baz`. // // The scope of the search / replace will be restricted to the current selection if any, otherwise // it will apply to the whole workspace. diff --git a/crates/ra_ssr/src/resolving.rs b/crates/ra_ssr/src/resolving.rs index 6f62000f4a7..d5b65eaacf2 100644 --- a/crates/ra_ssr/src/resolving.rs +++ b/crates/ra_ssr/src/resolving.rs @@ -5,7 +5,7 @@ use parsing::Placeholder; use ra_db::FilePosition; use ra_syntax::{ast, SmolStr, SyntaxKind, SyntaxNode, SyntaxToken}; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use test_utils::mark; pub(crate) struct ResolutionScope<'db> { @@ -111,8 +111,10 @@ fn resolve( .resolution_scope .resolve_path(&path) .ok_or_else(|| error!("Failed to resolve path `{}`", node.text()))?; - resolved_paths.insert(node, ResolvedPath { resolution, depth }); - return Ok(()); + if self.ok_to_use_path_resolution(&resolution) { + resolved_paths.insert(node, ResolvedPath { resolution, depth }); + return Ok(()); + } } } for node in node.children() { @@ -136,6 +138,27 @@ fn path_contains_placeholder(&self, path: &ast::Path) -> bool { } false } + + fn ok_to_use_path_resolution(&self, resolution: &hir::PathResolution) -> bool { + match resolution { + hir::PathResolution::AssocItem(hir::AssocItem::Function(function)) => { + if function.has_self_param(self.resolution_scope.scope.db) { + // If we don't use this path resolution, then we won't be able to match method + // calls. e.g. `Foo::bar($s)` should match `x.bar()`. + true + } else { + mark::hit!(replace_associated_trait_default_function_call); + false + } + } + hir::PathResolution::AssocItem(_) => { + // Not a function. Could be a constant or an associated type. + mark::hit!(replace_associated_trait_constant); + false + } + _ => true, + } + } } impl<'db> ResolutionScope<'db> { @@ -176,7 +199,7 @@ fn resolve_path(&self, path: &ast::Path) -> Option { adt.ty(self.scope.db).iterate_path_candidates( self.scope.db, self.scope.module()?.krate(), - &FxHashSet::default(), + &self.scope.traits_in_scope(), Some(hir_path.segments().last()?.name), |_ty, assoc_item| Some(hir::PathResolution::AssocItem(assoc_item)), ) diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 2ae03c64c42..0a49a46e382 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs @@ -549,6 +549,70 @@ impl Bar { fn new() {} } ); } +#[test] +fn replace_associated_trait_default_function_call() { + mark::check!(replace_associated_trait_default_function_call); + assert_ssr_transform( + "Bar2::foo() ==>> Bar2::foo2()", + r#" + trait Foo { fn foo() {} } + pub struct Bar {} + impl Foo for Bar {} + pub struct Bar2 {} + impl Foo for Bar2 {} + impl Bar2 { fn foo2() {} } + fn main() { + Bar::foo(); + Bar2::foo(); + } + "#, + expect![[r#" + trait Foo { fn foo() {} } + pub struct Bar {} + impl Foo for Bar {} + pub struct Bar2 {} + impl Foo for Bar2 {} + impl Bar2 { fn foo2() {} } + fn main() { + Bar::foo(); + Bar2::foo2(); + } + "#]], + ); +} + +#[test] +fn replace_associated_trait_constant() { + mark::check!(replace_associated_trait_constant); + assert_ssr_transform( + "Bar2::VALUE ==>> Bar2::VALUE_2222", + r#" + trait Foo { const VALUE: i32; const VALUE_2222: i32; } + pub struct Bar {} + impl Foo for Bar { const VALUE: i32 = 1; const VALUE_2222: i32 = 2; } + pub struct Bar2 {} + impl Foo for Bar2 { const VALUE: i32 = 1; const VALUE_2222: i32 = 2; } + impl Bar2 { fn foo2() {} } + fn main() { + Bar::VALUE; + Bar2::VALUE; + } + "#, + expect![[r#" + trait Foo { const VALUE: i32; const VALUE_2222: i32; } + pub struct Bar {} + impl Foo for Bar { const VALUE: i32 = 1; const VALUE_2222: i32 = 2; } + pub struct Bar2 {} + impl Foo for Bar2 { const VALUE: i32 = 1; const VALUE_2222: i32 = 2; } + impl Bar2 { fn foo2() {} } + fn main() { + Bar::VALUE; + Bar2::VALUE_2222; + } + "#]], + ); +} + #[test] fn replace_path_in_different_contexts() { // Note the <|> inside module a::b which marks the point where the rule is interpreted. We