5639: SSR: Allow `self` in patterns. r=jonas-schievink a=davidlattimore

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.

Co-authored-by: David Lattimore <dml@google.com>
This commit is contained in:
bors[bot] 2020-08-05 22:07:35 +00:00 committed by GitHub
commit ed4687f698
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 68 additions and 6 deletions

View file

@ -66,12 +66,7 @@ impl<'db> MatchFinder<'db> {
restrict_ranges.retain(|range| !range.range.is_empty()); restrict_ranges.retain(|range| !range.range.is_empty());
let sema = Semantics::new(db); let sema = Semantics::new(db);
let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context); let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context);
MatchFinder { MatchFinder { sema, rules: Vec::new(), resolution_scope, restrict_ranges }
sema: Semantics::new(db),
rules: Vec::new(),
resolution_scope,
restrict_ranges,
}
} }
/// Constructs an instance using the start of the first file in `db` as the lookup context. /// Constructs an instance using the start of the first file in `db` as the lookup context.

View file

@ -11,6 +11,7 @@ use test_utils::mark;
pub(crate) struct ResolutionScope<'db> { pub(crate) struct ResolutionScope<'db> {
scope: hir::SemanticsScope<'db>, scope: hir::SemanticsScope<'db>,
hygiene: hir::Hygiene, hygiene: hir::Hygiene,
node: SyntaxNode,
} }
pub(crate) struct ResolvedRule { pub(crate) struct ResolvedRule {
@ -25,6 +26,7 @@ pub(crate) struct ResolvedPattern {
// Paths in `node` that we've resolved. // Paths in `node` that we've resolved.
pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>, pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>,
pub(crate) ufcs_function_calls: FxHashMap<SyntaxNode, hir::Function>, pub(crate) ufcs_function_calls: FxHashMap<SyntaxNode, hir::Function>,
pub(crate) contains_self: bool,
} }
pub(crate) struct ResolvedPath { pub(crate) struct ResolvedPath {
@ -68,6 +70,7 @@ struct Resolver<'a, 'db> {
impl Resolver<'_, '_> { impl Resolver<'_, '_> {
fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> { fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> {
use ra_syntax::{SyntaxElement, T};
let mut resolved_paths = FxHashMap::default(); let mut resolved_paths = FxHashMap::default();
self.resolve(pattern.clone(), 0, &mut resolved_paths)?; self.resolve(pattern.clone(), 0, &mut resolved_paths)?;
let ufcs_function_calls = resolved_paths let ufcs_function_calls = resolved_paths
@ -85,11 +88,17 @@ impl Resolver<'_, '_> {
None None
}) })
.collect(); .collect();
let contains_self =
pattern.descendants_with_tokens().any(|node_or_token| match node_or_token {
SyntaxElement::Token(t) => t.kind() == T![self],
_ => false,
});
Ok(ResolvedPattern { Ok(ResolvedPattern {
node: pattern, node: pattern,
resolved_paths, resolved_paths,
placeholders_by_stand_in: self.placeholders_by_stand_in.clone(), placeholders_by_stand_in: self.placeholders_by_stand_in.clone(),
ufcs_function_calls, ufcs_function_calls,
contains_self,
}) })
} }
@ -101,6 +110,10 @@ impl Resolver<'_, '_> {
) -> Result<(), SsrError> { ) -> Result<(), SsrError> {
use ra_syntax::ast::AstNode; use ra_syntax::ast::AstNode;
if let Some(path) = ast::Path::cast(node.clone()) { if let Some(path) = ast::Path::cast(node.clone()) {
if is_self(&path) {
// Self cannot be resolved like other paths.
return Ok(());
}
// Check if this is an appropriate place in the path to resolve. If the path is // Check if this is an appropriate place in the path to resolve. If the path is
// something like `a::B::<i32>::c` then we want to resolve `a::B`. If the path contains // something like `a::B::<i32>::c` then we want to resolve `a::B`. If the path contains
// a placeholder. e.g. `a::$b::c` then we want to resolve `a`. // a placeholder. e.g. `a::$b::c` then we want to resolve `a`.
@ -157,9 +170,15 @@ impl<'db> ResolutionScope<'db> {
ResolutionScope { ResolutionScope {
scope, scope,
hygiene: hir::Hygiene::new(sema.db, resolve_context.file_id.into()), hygiene: hir::Hygiene::new(sema.db, resolve_context.file_id.into()),
node,
} }
} }
/// Returns the function in which SSR was invoked, if any.
pub(crate) fn current_function(&self) -> Option<SyntaxNode> {
self.node.ancestors().find(|node| node.kind() == SyntaxKind::FN).map(|node| node.clone())
}
fn resolve_path(&self, path: &ast::Path) -> Option<hir::PathResolution> { fn resolve_path(&self, path: &ast::Path) -> Option<hir::PathResolution> {
let hir_path = hir::Path::from_src(path.clone(), &self.hygiene)?; let hir_path = hir::Path::from_src(path.clone(), &self.hygiene)?;
// First try resolving the whole path. This will work for things like // First try resolving the whole path. This will work for things like
@ -186,6 +205,10 @@ impl<'db> ResolutionScope<'db> {
} }
} }
fn is_self(path: &ast::Path) -> bool {
path.segment().map(|segment| segment.self_token().is_some()).unwrap_or(false)
}
/// Returns a suitable node for resolving paths in the current scope. If we create a scope based on /// Returns a suitable node for resolving paths in the current scope. If we create a scope based on
/// a statement node, then we can't resolve local variables that were defined in the current scope /// a statement node, then we can't resolve local variables that were defined in the current scope
/// (only in parent scopes). So we find another node, ideally a child of the statement where local /// (only in parent scopes). So we find another node, ideally a child of the statement where local

View file

@ -33,6 +33,15 @@ impl<'db> MatchFinder<'db> {
usage_cache: &mut UsageCache, usage_cache: &mut UsageCache,
matches_out: &mut Vec<Match>, matches_out: &mut Vec<Match>,
) { ) {
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(&current_function, rule, &None, matches_out);
}
return;
}
if pick_path_for_usages(&rule.pattern).is_none() { if pick_path_for_usages(&rule.pattern).is_none() {
self.slow_scan(rule, matches_out); self.slow_scan(rule, matches_out);
return; return;

View file

@ -1044,3 +1044,38 @@ fn replace_nonpath_within_selection() {
}"#]], }"#]],
); );
} }
#[test]
fn replace_self() {
// `foo(self)` occurs twice in the code, however only the first occurrence is the `self` that's
// in scope where the rule is invoked.
assert_ssr_transform(
"foo(self) ==>> bar(self)",
r#"
struct S1 {}
fn foo(_: &S1) {}
fn bar(_: &S1) {}
impl S1 {
fn f1(&self) {
foo(self)<|>
}
fn f2(&self) {
foo(self)
}
}
"#,
expect![[r#"
struct S1 {}
fn foo(_: &S1) {}
fn bar(_: &S1) {}
impl S1 {
fn f1(&self) {
bar(self)
}
fn f2(&self) {
foo(self)
}
}
"#]],
);
}