From 8d09ab86edfc01405fd0045bef82e0642efd5f01 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Thu, 23 Jul 2020 21:28:31 +1000 Subject: SSR: Disable matching within use declarations It currently does the wrong thing when the use declaration contains braces. --- crates/ra_ssr/src/search.rs | 29 ++++++++++++++++++++++++++++- crates/ra_ssr/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index 9a4e35e96..141e7d026 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs @@ -10,7 +10,8 @@ use ra_ide_db::{ defs::Definition, search::{Reference, SearchScope}, }; -use ra_syntax::{ast, AstNode, SyntaxNode}; +use ra_syntax::{ast, AstNode, SyntaxKind, SyntaxNode}; +use test_utils::mark; /// A cache for the results of find_usages. This is for when we have multiple patterns that have the /// same path. e.g. if the pattern was `foo::Bar` that can parse as a path, an expression, a type @@ -59,6 +60,10 @@ impl<'db> MatchFinder<'db> { .skip(first_path.depth as usize) .next() { + if !is_search_permitted_ancestors(&node_to_match) { + mark::hit!(use_declaration_with_braces); + continue; + } if let Ok(m) = matching::get_match(false, rule, &node_to_match, &None, &self.sema) { @@ -123,6 +128,9 @@ impl<'db> MatchFinder<'db> { restrict_range: &Option, matches_out: &mut Vec, ) { + if !is_search_permitted(code) { + return; + } if let Ok(m) = matching::get_match(false, rule, &code, restrict_range, &self.sema) { matches_out.push(m); } @@ -149,6 +157,25 @@ impl<'db> MatchFinder<'db> { } } +/// Returns whether we support matching within `node` and all of its ancestors. +fn is_search_permitted_ancestors(node: &SyntaxNode) -> bool { + if let Some(parent) = node.parent() { + if !is_search_permitted_ancestors(&parent) { + return false; + } + } + is_search_permitted(node) +} + +/// Returns whether we support matching within this kind of node. +fn is_search_permitted(node: &SyntaxNode) -> bool { + // FIXME: Properly handle use declarations. At the moment, if our search pattern is `foo::bar` + // and the code is `use foo::{baz, bar}`, we'll match `bar`, since it resolves to `foo::bar`. + // However we'll then replace just the part we matched `bar`. We probably need to instead remove + // `bar` and insert a new use declaration. + node.kind() != SyntaxKind::USE_ITEM +} + impl UsageCache { fn find(&mut self, definition: &Definition) -> Option<&[Reference]> { // We expect a very small number of cache entries (generally 1), so a linear scan should be diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 33742dc8e..f564c6129 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs @@ -804,3 +804,26 @@ fn overlapping_possible_matches() { &["foo(foo(42))", "foo(foo(foo(foo(42))))"], ); } + +#[test] +fn use_declaration_with_braces() { + // It would be OK for a path rule to match and alter a use declaration. We shouldn't mess it up + // though. In particular, we must not change `use foo::{baz, bar}` to `use foo::{baz, + // foo2::bar2}`. + mark::check!(use_declaration_with_braces); + assert_ssr_transform( + "foo::bar ==>> foo2::bar2", + r#" + mod foo { pub fn bar() {} pub fn baz() {} } + mod foo2 { pub fn bar2() {} } + use foo::{baz, bar}; + fn main() { bar() } + "#, + expect![[" + mod foo { pub fn bar() {} pub fn baz() {} } + mod foo2 { pub fn bar2() {} } + use foo::{baz, bar}; + fn main() { foo2::bar2() } + "]], + ) +} -- cgit v1.2.3