From 3bb46042fb5b8ee421e350c54079cb68b4edc996 Mon Sep 17 00:00:00 2001 From: John Renner Date: Fri, 1 May 2020 08:59:24 -0700 Subject: Validate uses of self and super --- crates/ra_syntax/src/ast/generated/nodes.rs | 2 + crates/ra_syntax/src/validation.rs | 119 ++++++++++++--------- .../err/0041_illegal_super_keyword_location.rast | 70 ++++++++++++ .../err/0041_illegal_super_keyword_location.rs | 4 + .../err/0042_illegal_self_keyword_location.rast | 27 +++++ .../err/0042_illegal_self_keyword_location.rs | 2 + .../parser/ok/0013_use_path_self_super.rast | 26 +---- .../parser/ok/0013_use_path_self_super.rs | 1 - xtask/src/ast_src.rs | 2 +- 9 files changed, 178 insertions(+), 75 deletions(-) create mode 100644 crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rast create mode 100644 crates/ra_syntax/test_data/parser/err/0041_illegal_super_keyword_location.rs create mode 100644 crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rast create mode 100644 crates/ra_syntax/test_data/parser/err/0042_illegal_self_keyword_location.rs diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs index d2253d4af..2096f12f1 100644 --- a/crates/ra_syntax/src/ast/generated/nodes.rs +++ b/crates/ra_syntax/src/ast/generated/nodes.rs @@ -1251,6 +1251,8 @@ pub struct PathSegment { impl PathSegment { pub fn coloncolon_token(&self) -> Option { support::token(&self.syntax, T![::]) } pub fn crate_token(&self) -> Option { support::token(&self.syntax, T![crate]) } + pub fn self_token(&self) -> Option { support::token(&self.syntax, T![self]) } + pub fn super_token(&self) -> Option { support::token(&self.syntax, T![super]) } pub fn l_angle_token(&self) -> Option { support::token(&self.syntax, T![<]) } pub fn name_ref(&self) -> Option { support::child(&self.syntax) } pub fn type_arg_list(&self) -> Option { 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 { ast::RecordField(it) => validate_numeric_name(it.name_ref(), &mut errors), ast::Visibility(it) => validate_visibility(it, &mut errors), ast::RangeExpr(it) => validate_range_expr(it, &mut errors), - ast::PathSegment(it) => validate_crate_keyword_in_path_segment(it, &mut errors), + ast::PathSegment(it) => validate_path_keywords(it, &mut errors), _ => (), } } @@ -224,59 +224,82 @@ fn validate_range_expr(expr: ast::RangeExpr, errors: &mut Vec) { } } -fn validate_crate_keyword_in_path_segment( - segment: ast::PathSegment, - errors: &mut Vec, -) { - const ERR_MSG: &str = "The `crate` keyword is only allowed as the first segment of a path"; +fn validate_path_keywords(segment: ast::PathSegment, errors: &mut Vec) { + use ast::PathSegmentKind; - let crate_token = match segment.crate_token() { - None => return, - Some(it) => it, - }; + let path = segment.parent_path(); + let is_path_start = segment.coloncolon_token().is_none() && path.qualifier().is_none(); + + if let Some(token) = segment.self_token() { + if !is_path_start { + errors.push(SyntaxError::new( + "The `self` keyword is only allowed as the first segment of a path", + token.text_range(), + )); + } + } else if let Some(token) = segment.crate_token() { + if !is_path_start || use_prefix(path).is_some() { + errors.push(SyntaxError::new( + "The `crate` keyword is only allowed as the first segment of a path", + token.text_range(), + )); + } + } else if let Some(token) = segment.super_token() { + if !all_supers(&path) { + errors.push(SyntaxError::new( + "The `super` keyword may only be preceded by other `super`s", + token.text_range(), + )); + return; + } - // Disallow both ::crate and foo::crate - let mut path = segment.parent_path(); - if segment.coloncolon_token().is_some() || path.qualifier().is_some() { - errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range())); - return; + let mut curr_path = path; + while let Some(prefix) = use_prefix(curr_path) { + if !all_supers(&prefix) { + errors.push(SyntaxError::new( + "The `super` keyword may only be preceded by other `super`s", + token.text_range(), + )); + return; + } + curr_path = prefix; + } } - // For expressions and types, validation is complete, but we still have - // to handle invalid UseItems like this: - // - // use foo:{crate::bar::baz}; - // - // To handle this we must inspect the parent `UseItem`s and `UseTree`s - // but right now we're looking deep inside the nested `Path` nodes because - // `Path`s are left-associative: - // - // ((crate)::bar)::baz) - // ^ current value of path - // - // So we need to climb to the top - while let Some(parent) = path.parent_path() { - path = parent; + fn use_prefix(mut path: ast::Path) -> Option { + for node in path.syntax().ancestors().skip(1) { + match_ast! { + match node { + ast::UseTree(it) => if let Some(tree_path) = it.path() { + // Even a top-level path exists within a `UseTree` so we must explicitly + // allow our path but disallow anything else + if tree_path != path { + return Some(tree_path); + } + }, + ast::UseTreeList(_it) => continue, + ast::Path(parent) => path = parent, + _ => return None, + } + }; + } + return None; } - // Now that we've found the whole path we need to see if there's a prefix - // somewhere in the UseTree hierarchy. This check is arbitrarily deep - // because rust allows arbitrary nesting like so: - // - // use {foo::{{{{crate::bar::baz}}}}}; - for node in path.syntax().ancestors().skip(1) { - match_ast! { - match node { - ast::UseTree(it) => if let Some(tree_path) = it.path() { - // Even a top-level path exists within a `UseTree` so we must explicitly - // allow our path but disallow anything else - if tree_path != path { - errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range())); - } - }, - ast::UseTreeList(_it) => continue, - _ => return, - } + fn all_supers(path: &ast::Path) -> bool { + let segment = match path.segment() { + Some(it) => it, + None => return false, }; + + if segment.kind() != Some(PathSegmentKind::SuperKw) { + return false; + } + + if let Some(ref subpath) = path.qualifier() { + return all_supers(subpath); + } + + return true; } } 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 @@ +SOURCE_FILE@0..67 + USE_ITEM@0..12 + USE_KW@0..3 "use" + WHITESPACE@3..4 " " + USE_TREE@4..11 + PATH@4..11 + PATH_SEGMENT@4..11 + COLON2@4..6 "::" + SUPER_KW@6..11 "super" + SEMICOLON@11..12 ";" + WHITESPACE@12..13 "\n" + USE_ITEM@13..26 + USE_KW@13..16 "use" + WHITESPACE@16..17 " " + USE_TREE@17..25 + PATH@17..25 + PATH@17..18 + PATH_SEGMENT@17..18 + NAME_REF@17..18 + IDENT@17..18 "a" + COLON2@18..20 "::" + PATH_SEGMENT@20..25 + SUPER_KW@20..25 "super" + SEMICOLON@25..26 ";" + WHITESPACE@26..27 "\n" + USE_ITEM@27..47 + USE_KW@27..30 "use" + WHITESPACE@30..31 " " + USE_TREE@31..46 + PATH@31..46 + PATH@31..39 + PATH@31..36 + PATH_SEGMENT@31..36 + SUPER_KW@31..36 "super" + COLON2@36..38 "::" + PATH_SEGMENT@38..39 + NAME_REF@38..39 + IDENT@38..39 "a" + COLON2@39..41 "::" + PATH_SEGMENT@41..46 + SUPER_KW@41..46 "super" + SEMICOLON@46..47 ";" + WHITESPACE@47..48 "\n" + USE_ITEM@48..66 + USE_KW@48..51 "use" + WHITESPACE@51..52 " " + USE_TREE@52..65 + PATH@52..53 + PATH_SEGMENT@52..53 + NAME_REF@52..53 + IDENT@52..53 "a" + COLON2@53..55 "::" + USE_TREE_LIST@55..65 + L_CURLY@55..56 "{" + USE_TREE@56..64 + PATH@56..64 + PATH@56..61 + PATH_SEGMENT@56..61 + SUPER_KW@56..61 "super" + COLON2@61..63 "::" + PATH_SEGMENT@63..64 + NAME_REF@63..64 + IDENT@63..64 "b" + R_CURLY@64..65 "}" + SEMICOLON@65..66 ";" + WHITESPACE@66..67 "\n" +error 6..11: The `super` keyword may only be preceded by other `super`s +error 20..25: The `super` keyword may only be preceded by other `super`s +error 41..46: The `super` keyword may only be preceded by other `super`s +error 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 @@ +use ::super; +use a::super; +use super::a::super; +use 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 @@ +SOURCE_FILE@0..25 + USE_ITEM@0..11 + USE_KW@0..3 "use" + WHITESPACE@3..4 " " + USE_TREE@4..10 + PATH@4..10 + PATH_SEGMENT@4..10 + COLON2@4..6 "::" + SELF_KW@6..10 "self" + SEMICOLON@10..11 ";" + WHITESPACE@11..12 "\n" + USE_ITEM@12..24 + USE_KW@12..15 "use" + WHITESPACE@15..16 " " + USE_TREE@16..23 + PATH@16..23 + PATH@16..17 + PATH_SEGMENT@16..17 + NAME_REF@16..17 + IDENT@16..17 "a" + COLON2@17..19 "::" + PATH_SEGMENT@19..23 + SELF_KW@19..23 "self" + SEMICOLON@23..24 ";" + WHITESPACE@24..25 "\n" +error 6..10: The `self` keyword is only allowed as the first segment of a path +error 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 @@ +use ::self; +use 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 @@ -SOURCE_FILE@0..65 +SOURCE_FILE@0..38 USE_ITEM@0..14 USE_KW@0..3 "use" WHITESPACE@3..4 " " @@ -31,27 +31,3 @@ SOURCE_FILE@0..65 IDENT@33..36 "bar" SEMICOLON@36..37 ";" WHITESPACE@37..38 "\n" - USE_ITEM@38..64 - USE_KW@38..41 "use" - WHITESPACE@41..42 " " - USE_TREE@42..63 - PATH@42..63 - PATH@42..58 - PATH@42..51 - PATH@42..48 - PATH_SEGMENT@42..48 - COLON2@42..44 "::" - SELF_KW@44..48 "self" - COLON2@48..50 "::" - PATH_SEGMENT@50..51 - NAME_REF@50..51 - IDENT@50..51 "a" - COLON2@51..53 "::" - PATH_SEGMENT@53..58 - SUPER_KW@53..58 "super" - COLON2@58..60 "::" - PATH_SEGMENT@60..63 - NAME_REF@60..63 - IDENT@60..63 "bar" - SEMICOLON@63..64 ";" - 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 @@ use self::foo; use super::super::bar; -use ::self::a::super::bar; diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs index c14804aad..1abb62f6f 100644 --- a/xtask/src/ast_src.rs +++ b/xtask/src/ast_src.rs @@ -595,7 +595,7 @@ pub(crate) const AST_SRC: AstSrc = AstSrc { qualifier: Path, } struct PathSegment { - T![::], T![crate], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>] + T![::], T![crate], T![self], T![super], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>] } struct TypeArgList { T![::], -- cgit v1.2.3