Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate uses of self and super #4246

Merged
merged 1 commit into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/ra_syntax/src/ast/generated/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,8 @@ pub struct PathSegment {
impl PathSegment {
pub fn coloncolon_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![::]) }
pub fn crate_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![crate]) }
pub fn self_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![self]) }
pub fn super_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![super]) }
pub fn l_angle_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![<]) }
pub fn name_ref(&self) -> Option<NameRef> { support::child(&self.syntax) }
pub fn type_arg_list(&self) -> Option<TypeArgList> { support::child(&self.syntax) }
Expand Down
119 changes: 71 additions & 48 deletions crates/ra_syntax/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub(crate) fn validate(root: &SyntaxNode) -> Vec<SyntaxError> {
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),
_ => (),
}
}
Expand Down Expand Up @@ -224,59 +224,82 @@ fn validate_range_expr(expr: ast::RangeExpr, errors: &mut Vec<SyntaxError>) {
}
}

fn validate_crate_keyword_in_path_segment(
segment: ast::PathSegment,
errors: &mut Vec<SyntaxError>,
) {
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<SyntaxError>) {
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<ast::Path> {
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;
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
use ::super;
use a::super;
use super::a::super;
use a::{super::b};
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
use ::self;
use a::self;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
SOURCE_FILE@0..65
SOURCE_FILE@0..38
USE_ITEM@0..14
USE_KW@0..3 "use"
WHITESPACE@3..4 " "
Expand Down Expand Up @@ -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"
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
use self::foo;
use super::super::bar;
use ::self::a::super::bar;
2 changes: 1 addition & 1 deletion xtask/src/ast_src.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![::],
Expand Down