From cf55806257776baf7db6b02d260bdaa9e851c7d4 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 29 Jul 2020 11:44:01 +1000 Subject: SSR: Restrict to current selection if any The selection is also used to avoid unnecessary work, but only to the file level. Further restricting unnecessary work is left for later. --- crates/ra_ssr/src/search.rs | 87 +++++++++++++++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 23 deletions(-) (limited to 'crates/ra_ssr/src/search.rs') diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index bcf0f0468..0f512cb62 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs @@ -5,12 +5,13 @@ use crate::{ resolving::{ResolvedPath, ResolvedPattern, ResolvedRule}, Match, MatchFinder, }; -use ra_db::FileRange; +use ra_db::{FileId, FileRange}; use ra_ide_db::{ defs::Definition, search::{Reference, SearchScope}, }; use ra_syntax::{ast, AstNode, SyntaxKind, SyntaxNode}; +use rustc_hash::FxHashSet; use test_utils::mark; /// A cache for the results of find_usages. This is for when we have multiple patterns that have the @@ -54,11 +55,7 @@ impl<'db> MatchFinder<'db> { mark::hit!(use_declaration_with_braces); continue; } - if let Ok(m) = - matching::get_match(false, rule, &node_to_match, &None, &self.sema) - { - matches_out.push(m); - } + self.try_add_match(rule, &node_to_match, &None, matches_out); } } } @@ -121,25 +118,39 @@ impl<'db> MatchFinder<'db> { // FIXME: We should ideally have a test that checks that we edit local roots and not library // roots. This probably would require some changes to fixtures, since currently everything // seems to get put into a single source root. - use ra_db::SourceDatabaseExt; - use ra_ide_db::symbol_index::SymbolsDatabase; let mut files = Vec::new(); - for &root in self.sema.db.local_roots().iter() { - let sr = self.sema.db.source_root(root); - files.extend(sr.iter()); - } + self.search_files_do(|file_id| { + files.push(file_id); + }); SearchScope::files(&files) } fn slow_scan(&self, rule: &ResolvedRule, matches_out: &mut Vec) { - use ra_db::SourceDatabaseExt; - use ra_ide_db::symbol_index::SymbolsDatabase; - for &root in self.sema.db.local_roots().iter() { - let sr = self.sema.db.source_root(root); - for file_id in sr.iter() { - let file = self.sema.parse(file_id); - let code = file.syntax(); - self.slow_scan_node(code, rule, &None, matches_out); + self.search_files_do(|file_id| { + let file = self.sema.parse(file_id); + let code = file.syntax(); + self.slow_scan_node(code, rule, &None, matches_out); + }) + } + + fn search_files_do(&self, mut callback: impl FnMut(FileId)) { + if self.restrict_ranges.is_empty() { + // Unrestricted search. + use ra_db::SourceDatabaseExt; + use ra_ide_db::symbol_index::SymbolsDatabase; + for &root in self.sema.db.local_roots().iter() { + let sr = self.sema.db.source_root(root); + for file_id in sr.iter() { + callback(file_id); + } + } + } else { + // Search is restricted, deduplicate file IDs (generally only one). + let mut files = FxHashSet::default(); + for range in &self.restrict_ranges { + if files.insert(range.file_id) { + callback(range.file_id); + } } } } @@ -154,9 +165,7 @@ impl<'db> MatchFinder<'db> { if !is_search_permitted(code) { return; } - if let Ok(m) = matching::get_match(false, rule, &code, restrict_range, &self.sema) { - matches_out.push(m); - } + self.try_add_match(rule, &code, restrict_range, matches_out); // If we've got a macro call, we already tried matching it pre-expansion, which is the only // way to match the whole macro, now try expanding it and matching the expansion. if let Some(macro_call) = ast::MacroCall::cast(code.clone()) { @@ -178,6 +187,38 @@ impl<'db> MatchFinder<'db> { self.slow_scan_node(&child, rule, restrict_range, matches_out); } } + + fn try_add_match( + &self, + rule: &ResolvedRule, + code: &SyntaxNode, + restrict_range: &Option, + matches_out: &mut Vec, + ) { + if !self.within_range_restrictions(code) { + mark::hit!(replace_nonpath_within_selection); + return; + } + if let Ok(m) = matching::get_match(false, rule, code, restrict_range, &self.sema) { + matches_out.push(m); + } + } + + /// Returns whether `code` is within one of our range restrictions if we have any. No range + /// restrictions is considered unrestricted and always returns true. + fn within_range_restrictions(&self, code: &SyntaxNode) -> bool { + if self.restrict_ranges.is_empty() { + // There is no range restriction. + return true; + } + let node_range = self.sema.original_range(code); + for range in &self.restrict_ranges { + if range.file_id == node_range.file_id && range.range.contains_range(node_range.range) { + return true; + } + } + false + } } /// Returns whether we support matching within `node` and all of its ancestors. -- cgit v1.2.3 From 9697d23cbe7b0ad897139b15f1b1ffe1cab6ad89 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 30 Jul 2020 14:12:04 +0200 Subject: Rename UseItem -> Use --- crates/ra_ssr/src/search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates/ra_ssr/src/search.rs') diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index 0f512cb62..213dc494f 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs @@ -237,7 +237,7 @@ fn is_search_permitted(node: &SyntaxNode) -> bool { // 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 + node.kind() != SyntaxKind::USE } impl UsageCache { -- cgit v1.2.3 From 6bbeffc8c56893548f5667844f59ce5a76f9fd98 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Sat, 1 Aug 2020 17:41:42 +1000 Subject: SSR: Allow `self` in patterns. It's now consistent with other variables in that if the pattern references self, only the `self` in scope where the rule is invoked will be accepted. Since `self` doesn't work the same as other paths, this is implemented by restricting the search to just the current function. Prior to this change (since path resolution was implemented), having self in a pattern would just result in no matches. --- crates/ra_ssr/src/search.rs | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'crates/ra_ssr/src/search.rs') diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index 213dc494f..85ffa2ac2 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs @@ -33,6 +33,15 @@ impl<'db> MatchFinder<'db> { usage_cache: &mut UsageCache, matches_out: &mut Vec, ) { + if rule.pattern.contains_self { + // If the pattern contains `self` we restrict the scope of the search to just the + // current method. No other method can reference the same `self`. This makes the + // behavior of `self` consistent with other variables. + if let Some(current_function) = self.resolution_scope.current_function() { + self.slow_scan_node(¤t_function, rule, &None, matches_out); + } + return; + } if pick_path_for_usages(&rule.pattern).is_none() { self.slow_scan(rule, matches_out); return; -- cgit v1.2.3