aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-05-01 20:24:25 +0100
committerGitHub <noreply@github.com>2020-05-01 20:24:25 +0100
commit21588e15dfa8b117b503769119320e718bade351 (patch)
treec10be6c66f2d6220d98c316c44bb3f9677ad1a1d /crates
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 <john@jrenner.net>
Diffstat (limited to 'crates')
-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
8 files changed, 177 insertions, 74 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 @@
1SOURCE_FILE@0..67
2 USE_ITEM@0..12
3 USE_KW@0..3 "use"
4 WHITESPACE@3..4 " "
5 USE_TREE@4..11
6 PATH@4..11
7 PATH_SEGMENT@4..11
8 COLON2@4..6 "::"
9 SUPER_KW@6..11 "super"
10 SEMICOLON@11..12 ";"
11 WHITESPACE@12..13 "\n"
12 USE_ITEM@13..26
13 USE_KW@13..16 "use"
14 WHITESPACE@16..17 " "
15 USE_TREE@17..25
16 PATH@17..25
17 PATH@17..18
18 PATH_SEGMENT@17..18
19 NAME_REF@17..18
20 IDENT@17..18 "a"
21 COLON2@18..20 "::"
22 PATH_SEGMENT@20..25
23 SUPER_KW@20..25 "super"
24 SEMICOLON@25..26 ";"
25 WHITESPACE@26..27 "\n"
26 USE_ITEM@27..47
27 USE_KW@27..30 "use"
28 WHITESPACE@30..31 " "
29 USE_TREE@31..46
30 PATH@31..46
31 PATH@31..39
32 PATH@31..36
33 PATH_SEGMENT@31..36
34 SUPER_KW@31..36 "super"
35 COLON2@36..38 "::"
36 PATH_SEGMENT@38..39
37 NAME_REF@38..39
38 IDENT@38..39 "a"
39 COLON2@39..41 "::"
40 PATH_SEGMENT@41..46
41 SUPER_KW@41..46 "super"
42 SEMICOLON@46..47 ";"
43 WHITESPACE@47..48 "\n"
44 USE_ITEM@48..66
45 USE_KW@48..51 "use"
46 WHITESPACE@51..52 " "
47 USE_TREE@52..65
48 PATH@52..53
49 PATH_SEGMENT@52..53
50 NAME_REF@52..53
51 IDENT@52..53 "a"
52 COLON2@53..55 "::"
53 USE_TREE_LIST@55..65
54 L_CURLY@55..56 "{"
55 USE_TREE@56..64
56 PATH@56..64
57 PATH@56..61
58 PATH_SEGMENT@56..61
59 SUPER_KW@56..61 "super"
60 COLON2@61..63 "::"
61 PATH_SEGMENT@63..64
62 NAME_REF@63..64
63 IDENT@63..64 "b"
64 R_CURLY@64..65 "}"
65 SEMICOLON@65..66 ";"
66 WHITESPACE@66..67 "\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 @@
1SOURCE_FILE@0..25
2 USE_ITEM@0..11
3 USE_KW@0..3 "use"
4 WHITESPACE@3..4 " "
5 USE_TREE@4..10
6 PATH@4..10
7 PATH_SEGMENT@4..10
8 COLON2@4..6 "::"
9 SELF_KW@6..10 "self"
10 SEMICOLON@10..11 ";"
11 WHITESPACE@11..12 "\n"
12 USE_ITEM@12..24
13 USE_KW@12..15 "use"
14 WHITESPACE@15..16 " "
15 USE_TREE@16..23
16 PATH@16..23
17 PATH@16..17
18 PATH_SEGMENT@16..17
19 NAME_REF@16..17
20 IDENT@16..17 "a"
21 COLON2@17..19 "::"
22 PATH_SEGMENT@19..23
23 SELF_KW@19..23 "self"
24 SEMICOLON@23..24 ";"
25 WHITESPACE@24..25 "\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 USE_ITEM@0..14 2 USE_ITEM@0..14
3 USE_KW@0..3 "use" 3 USE_KW@0..3 "use"
4 WHITESPACE@3..4 " " 4 WHITESPACE@3..4 " "
@@ -31,27 +31,3 @@ SOURCE_FILE@0..65
31 IDENT@33..36 "bar" 31 IDENT@33..36 "bar"
32 SEMICOLON@36..37 ";" 32 SEMICOLON@36..37 ";"
33 WHITESPACE@37..38 "\n" 33 WHITESPACE@37..38 "\n"
34 USE_ITEM@38..64
35 USE_KW@38..41 "use"
36 WHITESPACE@41..42 " "
37 USE_TREE@42..63
38 PATH@42..63
39 PATH@42..58
40 PATH@42..51
41 PATH@42..48
42 PATH_SEGMENT@42..48
43 COLON2@42..44 "::"
44 SELF_KW@44..48 "self"
45 COLON2@48..50 "::"
46 PATH_SEGMENT@50..51
47 NAME_REF@50..51
48 IDENT@50..51 "a"
49 COLON2@51..53 "::"
50 PATH_SEGMENT@53..58
51 SUPER_KW@53..58 "super"
52 COLON2@58..60 "::"
53 PATH_SEGMENT@60..63
54 NAME_REF@60..63
55 IDENT@60..63 "bar"
56 SEMICOLON@63..64 ";"
57 WHITESPACE@64..65 "\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;
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-28 05:59:18 +0100