aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-05-01 20:24:25 +0100
committerGitHub <[email protected]>2020-05-01 20:24:25 +0100
commit21588e15dfa8b117b503769119320e718bade351 (patch)
treec10be6c66f2d6220d98c316c44bb3f9677ad1a1d
parent302777f0041540bf6790fde2acc3bb5b04e72427 (diff)
parent3bb46042fb5b8ee421e350c54079cb68b4edc996 (diff)
Merge #4246
4246: Validate uses of self and super r=matklad a=djrenren This change follows on the validation of the `crate` keyword in paths. It verifies the following things: `super`: - May only be preceded by other `super` segments - If in a `UseItem` then all semantically preceding paths also consist only of `super` `self` - May only be the start of a path Just a note, a couple times while working on this I found myself really wanting a Visitor of some sort so that I could traverse descendants while skipping sub-trees that are unimportant. Iterators don't really work for this, so as you can see I reached for recursion. Considering paths are generally small a fancy debounced visitor probably isn't important but figured I'd say something in case we had something like this lying around and I wasn't using it. Co-authored-by: John Renner <[email protected]>
-rw-r--r--crates/ra_syntax/src/ast/generated/nodes.rs2
-rw-r--r--crates/ra_syntax/src/validation.rs119
-rw-r--r--crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rast70
-rw-r--r--crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rs4
-rw-r--r--crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rast27
-rw-r--r--crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rs2
-rw-r--r--crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rast26
-rw-r--r--crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rs1
-rw-r--r--xtask/src/ast_src.rs2
9 files changed, 178 insertions, 75 deletions
diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs
index 81260680f..3f16592b6 100644
--- a/crates/ra_syntax/src/ast/generated/nodes.rs
+++ b/crates/ra_syntax/src/ast/generated/nodes.rs
@@ -1241,6 +1241,8 @@ pub struct PathSegment {
1241impl PathSegment { 1241impl PathSegment {
1242 pub fn coloncolon_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![::]) } 1242 pub fn coloncolon_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![::]) }
1243 pub fn crate_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![crate]) } 1243 pub fn crate_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![crate]) }
1244 pub fn self_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![self]) }
1245 pub fn super_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![super]) }
1244 pub fn l_angle_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![<]) } 1246 pub fn l_angle_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![<]) }
1245 pub fn name_ref(&self) -> Option<NameRef> { support::child(&self.syntax) } 1247 pub fn name_ref(&self) -> Option<NameRef> { support::child(&self.syntax) }
1246 pub fn type_arg_list(&self) -> Option<TypeArgList> { support::child(&self.syntax) } 1248 pub fn type_arg_list(&self) -> Option<TypeArgList> { support::child(&self.syntax) }
diff --git a/crates/ra_syntax/src/validation.rs b/crates/ra_syntax/src/validation.rs
index f0b3dec63..e075cd801 100644
--- a/crates/ra_syntax/src/validation.rs
+++ b/crates/ra_syntax/src/validation.rs
@@ -96,7 +96,7 @@ pub(crate) fn validate(root: &SyntaxNode) -> Vec<SyntaxError> {
96 ast::RecordField(it) => validate_numeric_name(it.name_ref(), &mut errors), 96 ast::RecordField(it) => validate_numeric_name(it.name_ref(), &mut errors),
97 ast::Visibility(it) => validate_visibility(it, &mut errors), 97 ast::Visibility(it) => validate_visibility(it, &mut errors),
98 ast::RangeExpr(it) => validate_range_expr(it, &mut errors), 98 ast::RangeExpr(it) => validate_range_expr(it, &mut errors),
99 ast::PathSegment(it) => validate_crate_keyword_in_path_segment(it, &mut errors), 99 ast::PathSegment(it) => validate_path_keywords(it, &mut errors),
100 _ => (), 100 _ => (),
101 } 101 }
102 } 102 }
@@ -224,59 +224,82 @@ fn validate_range_expr(expr: ast::RangeExpr, errors: &mut Vec<SyntaxError>) {
224 } 224 }
225} 225}
226 226
227fn validate_crate_keyword_in_path_segment( 227fn validate_path_keywords(segment: ast::PathSegment, errors: &mut Vec<SyntaxError>) {
228 segment: ast::PathSegment, 228 use ast::PathSegmentKind;
229 errors: &mut Vec<SyntaxError>,
230) {
231 const ERR_MSG: &str = "The `crate` keyword is only allowed as the first segment of a path";
232 229
233 let crate_token = match segment.crate_token() { 230 let path = segment.parent_path();
234 None => return, 231 let is_path_start = segment.coloncolon_token().is_none() && path.qualifier().is_none();
235 Some(it) => it, 232
236 }; 233 if let Some(token) = segment.self_token() {
234 if !is_path_start {
235 errors.push(SyntaxError::new(
236 "The `self` keyword is only allowed as the first segment of a path",
237 token.text_range(),
238 ));
239 }
240 } else if let Some(token) = segment.crate_token() {
241 if !is_path_start || use_prefix(path).is_some() {
242 errors.push(SyntaxError::new(
243 "The `crate` keyword is only allowed as the first segment of a path",
244 token.text_range(),
245 ));
246 }
247 } else if let Some(token) = segment.super_token() {
248 if !all_supers(&path) {
249 errors.push(SyntaxError::new(
250 "The `super` keyword may only be preceded by other `super`s",
251 token.text_range(),
252 ));
253 return;
254 }
237 255
238 // Disallow both ::crate and foo::crate 256 let mut curr_path = path;
239 let mut path = segment.parent_path(); 257 while let Some(prefix) = use_prefix(curr_path) {
240 if segment.coloncolon_token().is_some() || path.qualifier().is_some() { 258 if !all_supers(&prefix) {
241 errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range())); 259 errors.push(SyntaxError::new(
242 return; 260 "The `super` keyword may only be preceded by other `super`s",
261 token.text_range(),
262 ));
263 return;
264 }
265 curr_path = prefix;
266 }
243 } 267 }
244 268
245 // For expressions and types, validation is complete, but we still have 269 fn use_prefix(mut path: ast::Path) -> Option<ast::Path> {
246 // to handle invalid UseItems like this: 270 for node in path.syntax().ancestors().skip(1) {
247 // 271 match_ast! {
248 // use foo:{crate::bar::baz}; 272 match node {
249 // 273 ast::UseTree(it) => if let Some(tree_path) = it.path() {
250 // To handle this we must inspect the parent `UseItem`s and `UseTree`s 274 // Even a top-level path exists within a `UseTree` so we must explicitly
251 // but right now we're looking deep inside the nested `Path` nodes because 275 // allow our path but disallow anything else
252 // `Path`s are left-associative: 276 if tree_path != path {
253 // 277 return Some(tree_path);
254 // ((crate)::bar)::baz) 278 }
255 // ^ current value of path 279 },
256 // 280 ast::UseTreeList(_it) => continue,
257 // So we need to climb to the top 281 ast::Path(parent) => path = parent,
258 while let Some(parent) = path.parent_path() { 282 _ => return None,
259 path = parent; 283 }
284 };
285 }
286 return None;
260 } 287 }
261 288
262 // Now that we've found the whole path we need to see if there's a prefix 289 fn all_supers(path: &ast::Path) -> bool {
263 // somewhere in the UseTree hierarchy. This check is arbitrarily deep 290 let segment = match path.segment() {
264 // because rust allows arbitrary nesting like so: 291 Some(it) => it,
265 // 292 None => return false,
266 // use {foo::{{{{crate::bar::baz}}}}};
267 for node in path.syntax().ancestors().skip(1) {
268 match_ast! {
269 match node {
270 ast::UseTree(it) => if let Some(tree_path) = it.path() {
271 // Even a top-level path exists within a `UseTree` so we must explicitly
272 // allow our path but disallow anything else
273 if tree_path != path {
274 errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range()));
275 }
276 },
277 ast::UseTreeList(_it) => continue,
278 _ => return,
279 }
280 }; 293 };
294
295 if segment.kind() != Some(PathSegmentKind::SuperKw) {
296 return false;
297 }
298
299 if let Some(ref subpath) = path.qualifier() {
300 return all_supers(subpath);
301 }
302
303 return true;
281 } 304 }
282} 305}
diff --git a/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rast b/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rast
new file mode 100644
index 000000000..d0360c467
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rast
@@ -0,0 +1,70 @@
1[email protected]
2 [email protected]
3 [email protected] "use"
4 [email protected] " "
5 [email protected]
6 [email protected]
7 [email protected]
8 [email protected] "::"
9 [email protected] "super"
10 [email protected] ";"
11 [email protected] "\n"
12 [email protected]
13 [email protected] "use"
14 [email protected] " "
15 [email protected]
16 [email protected]
17 [email protected]
18 [email protected]
19 [email protected]
20 [email protected] "a"
21 [email protected] "::"
22 [email protected]
23 [email protected] "super"
24 [email protected] ";"
25 [email protected] "\n"
26 [email protected]
27 [email protected] "use"
28 [email protected] " "
29 [email protected]
30 [email protected]
31 [email protected]
32 [email protected]
33 [email protected]
34 [email protected] "super"
35 [email protected] "::"
36 [email protected]
37 [email protected]
38 [email protected] "a"
39 [email protected] "::"
40 [email protected]
41 [email protected] "super"
42 [email protected] ";"
43 [email protected] "\n"
44 [email protected]
45 [email protected] "use"
46 [email protected] " "
47 [email protected]
48 [email protected]
49 [email protected]
50 [email protected]
51 [email protected] "a"
52 [email protected] "::"
53 [email protected]
54 [email protected] "{"
55 [email protected]
56 [email protected]
57 [email protected]
58 [email protected]
59 [email protected] "super"
60 [email protected] "::"
61 [email protected]
62 [email protected]
63 [email protected] "b"
64 [email protected] "}"
65 [email protected] ";"
66 [email protected] "\n"
67error 6..11: The `super` keyword may only be preceded by other `super`s
68error 20..25: The `super` keyword may only be preceded by other `super`s
69error 41..46: The `super` keyword may only be preceded by other `super`s
70error 56..61: The `super` keyword may only be preceded by other `super`s
diff --git a/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rs b/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rs
new file mode 100644
index 000000000..bd4d58042
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rs
@@ -0,0 +1,4 @@
1use ::super;
2use a::super;
3use super::a::super;
4use a::{super::b};
diff --git a/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rast b/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rast
new file mode 100644
index 000000000..4f382b06c
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rast
@@ -0,0 +1,27 @@
1[email protected]
2 [email protected]
3 [email protected] "use"
4 [email protected] " "
5 [email protected]
6 [email protected]
7 [email protected]
8 [email protected] "::"
9 [email protected] "self"
10 [email protected] ";"
11 [email protected] "\n"
12 [email protected]
13 [email protected] "use"
14 [email protected] " "
15 [email protected]
16 [email protected]
17 [email protected]
18 [email protected]
19 [email protected]
20 [email protected] "a"
21 [email protected] "::"
22 [email protected]
23 [email protected] "self"
24 [email protected] ";"
25 [email protected] "\n"
26error 6..10: The `self` keyword is only allowed as the first segment of a path
27error 19..23: The `self` keyword is only allowed as the first segment of a path
diff --git a/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rs b/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rs
new file mode 100644
index 000000000..b9e1d7d8b
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rs
@@ -0,0 +1,2 @@
1use ::self;
2use a::self;
diff --git a/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rast b/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rast
index a5a90df7b..05d9c05ad 100644
--- a/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rast
+++ b/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rast
@@ -1,4 +1,4 @@
1SOURCE_FILE@0..65 1SOURCE_FILE@0..38
2 [email protected] 2 [email protected]
3 [email protected] "use" 3 [email protected] "use"
4 [email protected] " " 4 [email protected] " "
@@ -31,27 +31,3 @@ [email protected]
31 [email protected] "bar" 31 [email protected] "bar"
32 [email protected] ";" 32 [email protected] ";"
33 [email protected] "\n" 33 [email protected] "\n"
34 [email protected]
35 [email protected] "use"
36 [email protected] " "
37 [email protected]
38 [email protected]
39 [email protected]
40 [email protected]
41 [email protected]
42 [email protected]
43 [email protected] "::"
44 [email protected] "self"
45 [email protected] "::"
46 [email protected]
47 [email protected]
48 [email protected] "a"
49 [email protected] "::"
50 [email protected]
51 [email protected] "super"
52 [email protected] "::"
53 [email protected]
54 [email protected]
55 [email protected] "bar"
56 [email protected] ";"
57 [email protected] "\n"
diff --git a/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rs b/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rs
index faf6a42c7..9d9eb9917 100644
--- a/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rs
+++ b/crates/ra_syntax/test_data/parser/ok/0013_use_path_self_super.rs
@@ -1,3 +1,2 @@
1use self::foo; 1use self::foo;
2use super::super::bar; 2use super::super::bar;
3use ::self::a::super::bar;
diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs
index bdd42cb76..703fb9be9 100644
--- a/xtask/src/ast_src.rs
+++ b/xtask/src/ast_src.rs
@@ -593,7 +593,7 @@ pub(crate) const AST_SRC: AstSrc = AstSrc {
593 qualifier: Path, 593 qualifier: Path,
594 } 594 }
595 struct PathSegment { 595 struct PathSegment {
596 T![::], T![crate], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>] 596 T![::], T![crate], T![self], T![super], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>]
597 } 597 }
598 struct TypeArgList { 598 struct TypeArgList {
599 T![::], 599 T![::],