Skip to content

Commit

Permalink
SSR: Allow self in patterns.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidlattimore committed Aug 1, 2020
1 parent 5af32ae commit 6bbeffc
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 6 deletions.
7 changes: 1 addition & 6 deletions crates/ra_ssr/src/lib.rs
Expand Up @@ -66,12 +66,7 @@ impl<'db> MatchFinder<'db> {
restrict_ranges.retain(|range| !range.range.is_empty());
let sema = Semantics::new(db);
let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context);
MatchFinder {
sema: Semantics::new(db),
rules: Vec::new(),
resolution_scope,
restrict_ranges,
}
MatchFinder { sema, rules: Vec::new(), resolution_scope, restrict_ranges }
}

/// Constructs an instance using the start of the first file in `db` as the lookup context.
Expand Down
29 changes: 29 additions & 0 deletions crates/ra_ssr/src/resolving.rs
Expand Up @@ -11,6 +11,7 @@ use test_utils::mark;
pub(crate) struct ResolutionScope<'db> {
scope: hir::SemanticsScope<'db>,
hygiene: hir::Hygiene,
node: SyntaxNode,
}

pub(crate) struct ResolvedRule {
Expand All @@ -25,6 +26,7 @@ pub(crate) struct ResolvedPattern {
// Paths in `node` that we've resolved.
pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>,
pub(crate) ufcs_function_calls: FxHashMap<SyntaxNode, hir::Function>,
pub(crate) contains_self: bool,
}

pub(crate) struct ResolvedPath {
Expand Down Expand Up @@ -68,6 +70,7 @@ struct Resolver<'a, 'db> {

impl Resolver<'_, '_> {
fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> {
use ra_syntax::{SyntaxElement, T};
let mut resolved_paths = FxHashMap::default();
self.resolve(pattern.clone(), 0, &mut resolved_paths)?;
let ufcs_function_calls = resolved_paths
Expand All @@ -85,11 +88,17 @@ impl Resolver<'_, '_> {
None
})
.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 {
node: pattern,
resolved_paths,
placeholders_by_stand_in: self.placeholders_by_stand_in.clone(),
ufcs_function_calls,
contains_self,
})
}

Expand All @@ -101,6 +110,10 @@ impl Resolver<'_, '_> {
) -> Result<(), SsrError> {
use ra_syntax::ast::AstNode;
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
// 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`.
Expand Down Expand Up @@ -157,6 +170,18 @@ impl<'db> ResolutionScope<'db> {
ResolutionScope {
scope,
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> {
let mut node = self.node.clone();
loop {
if node.kind() == SyntaxKind::FN {
return Some(node);
}
node = node.parent()?;
}
}

Expand Down Expand Up @@ -186,6 +211,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
/// 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
Expand Down
9 changes: 9 additions & 0 deletions crates/ra_ssr/src/search.rs
Expand Up @@ -33,6 +33,15 @@ impl<'db> MatchFinder<'db> {
usage_cache: &mut UsageCache,
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() {
self.slow_scan(rule, matches_out);
return;
Expand Down
35 changes: 35 additions & 0 deletions crates/ra_ssr/src/tests.rs
Expand Up @@ -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)
}
}
"#]],
);
}

0 comments on commit 6bbeffc

Please sign in to comment.