language-servers/rust-analyzer
1
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer.git synced 2025-10-02 22:54:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

SSR: Don't mix non-path-based rules with path-based

Browse source 
If any rules contain paths, then we reject any rules that don't contain paths. Allowing a mix leads to strange semantics, since the path-based rules only match things where the path refers to semantically the same thing, whereas the non-path-based rules could match anything. Specifically, if we have a rule like `foo ==>> bar` we only want to match the `foo` that is in the current scope, not any `foo`. However "foo" can be parsed as a pattern (BIND_PAT -> NAME -> IDENT). Allowing such a rule through would result in renaming everything called `foo` to `bar`. It'd also be slow, since without a path, we'd have to use the slow-scan search mechanism.
This commit is contained in:
David Lattimore 2020-07-29 16:01:00 +10:00
parent 5a8124273d
commit 3600c43f49
2 changed files with 62 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

24
crates/ra_ssr/src/parsing.rs
View file

@ -10,6 +10,7 @@ use crate::{SsrError, SsrPattern, SsrRule};
use ra_syntax::{ast, AstNode, SmolStr, SyntaxKind, SyntaxNode, T};
use rustc_hash::{FxHashMap, FxHashSet};
use std::str::FromStr;
use test_utils::mark;
#[derive(Debug)]
pub(crate) struct ParsedRule {
@ -102,14 +103,35 @@ impl RuleBuilder {
}
}
fn build(self) -> Result<Vec<ParsedRule>, SsrError> {
fn build(mut self) -> Result<Vec<ParsedRule>, SsrError> {
if self.rules.is_empty() {
bail!("Not a valid Rust expression, type, item, path or pattern");
}
// If any rules contain paths, then we reject any rules that don't contain paths. Allowing a
// mix leads to strange semantics, since the path-based rules only match things where the
// path refers to semantically the same thing, whereas the non-path-based rules could match
// anything. Specifically, if we have a rule like `foo ==>> bar` we only want to match the
// `foo` that is in the current scope, not any `foo`. However "foo" can be parsed as a
// pattern (BIND_PAT -> NAME -> IDENT). Allowing such a rule through would result in
// renaming everything called `foo` to `bar`. It'd also be slow, since without a path, we'd
// have to use the slow-scan search mechanism.
if self.rules.iter().any(|rule| contains_path(&rule.pattern)) {
let old_len = self.rules.len();
self.rules.retain(|rule| contains_path(&rule.pattern));
if self.rules.len() < old_len {
mark::hit!(pattern_is_a_single_segment_path);
}
}
Ok(self.rules)
}
}
/// Returns whether there are any paths in `node`.
fn contains_path(node: &SyntaxNode) -> bool {
node.kind() == SyntaxKind::PATH
|| node.descendants().any(|node| node.kind() == SyntaxKind::PATH)
}
impl FromStr for SsrRule {
type Err = SsrError;