4178: Validate the location of `crate` in paths r=matklad a=djrenren

**This solution does not fully handle `use` statements. See below**

This pull requests implements simple validation of usages of the `crate` keyword in `Path`s. Specifically it validates that:

- If a `PathSegment` is starts with the `crate` keyword, it is also the first segment of the `Path`
- All other usages of `crate` in `Path`s are considered errors.

This aligns with `rustc`'s rules. Unlike rustc this implementation does not issue a special error message in the case of `::crate` but it does catch the error.

Furthermore, this change does not cover all error cases. Specifically the following is not caught:

```rust
use foo::{crate}
```

This is because this check is context sensitive. From an AST perspective, `crate` is the root of the `Path`. Only by inspecting the full `UseItem` do we see that it is not in fact the root. This problem becomes worse because `UseTree`s are allowed to be arbitrarily nested:

```rust
use {crate, {{crate, foo::{crate}}}
```

So this is a hard problem to solve without essentially a breadth-first search. In a traditional compiler, I'd say this error is most easily found during the AST -> HIR conversion pass but within rust-analyzer I'm not sure where it belongs.  

Under the implementation in this PR, such errors are ignored so we're *more correct* just not *entirely correct*. 

Co-authored-by: John Renner <john@jrenner.net>
This commit is contained in:
bors[bot] 2020-04-30 10:17:40 +00:00 committed by GitHub
commit 95e8766db6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 121 additions and 1 deletions

View file

@ -1249,6 +1249,7 @@ 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 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) }

View file

@ -96,6 +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),
_ => (),
}
}
@ -222,3 +223,41 @@ 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";
let crate_token = match segment.crate_token() {
None => return,
Some(it) => it,
};
// Disallow both ::crate and foo::crate
let 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;
}
// We now know that the path variable describes a complete path.
// For expressions and types, validation is complete, but we still have
// to handle UseItems like this:
// use foo:{crate};
// so we crawl upwards looking for any preceding paths on `UseTree`s
for node in path.syntax().ancestors().skip(1) {
match_ast! {
match node {
ast::UseTree(it) => if let Some(tree_path) = it.path() {
if tree_path != path {
errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range()));
}
},
ast::UseTreeList(_it) => continue,
_ => return,
}
};
}
}

View file

@ -0,0 +1,76 @@
SOURCE_FILE@0..83
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 "::"
CRATE_KW@6..11 "crate"
SEMICOLON@11..12 ";"
WHITESPACE@12..13 "\n"
USE_ITEM@13..39
USE_KW@13..16 "use"
WHITESPACE@16..17 " "
USE_TREE@17..38
USE_TREE_LIST@17..38
L_CURLY@17..18 "{"
USE_TREE@18..23
PATH@18..23
PATH_SEGMENT@18..23
CRATE_KW@18..23 "crate"
COMMA@23..24 ","
WHITESPACE@24..25 " "
USE_TREE@25..37
PATH@25..28
PATH_SEGMENT@25..28
NAME_REF@25..28
IDENT@25..28 "foo"
COLON2@28..30 "::"
USE_TREE_LIST@30..37
L_CURLY@30..31 "{"
USE_TREE@31..36
PATH@31..36
PATH_SEGMENT@31..36
CRATE_KW@31..36 "crate"
R_CURLY@36..37 "}"
R_CURLY@37..38 "}"
SEMICOLON@38..39 ";"
WHITESPACE@39..40 "\n"
USE_ITEM@40..57
USE_KW@40..43 "use"
WHITESPACE@43..44 " "
USE_TREE@44..56
PATH@44..56
PATH@44..49
PATH_SEGMENT@44..49
NAME_REF@44..49
IDENT@44..49 "hello"
COLON2@49..51 "::"
PATH_SEGMENT@51..56
CRATE_KW@51..56 "crate"
SEMICOLON@56..57 ";"
WHITESPACE@57..58 "\n"
USE_ITEM@58..82
USE_KW@58..61 "use"
WHITESPACE@61..62 " "
USE_TREE@62..81
PATH@62..81
PATH@62..74
PATH@62..67
PATH_SEGMENT@62..67
NAME_REF@62..67
IDENT@62..67 "hello"
COLON2@67..69 "::"
PATH_SEGMENT@69..74
CRATE_KW@69..74 "crate"
COLON2@74..76 "::"
PATH_SEGMENT@76..81
NAME_REF@76..81
IDENT@76..81 "there"
SEMICOLON@81..82 ";"
WHITESPACE@82..83 "\n"
error 6..11: The `crate` keyword is only allowed as the first segment of a path
error 31..36: The `crate` keyword is only allowed as the first segment of a path
error 51..56: The `crate` keyword is only allowed as the first segment of a path
error 69..74: The `crate` keyword is only allowed as the first segment of a path

View file

@ -0,0 +1,4 @@
use ::crate;
use {crate, foo::{crate}};
use hello::crate;
use hello::crate::there;

View file

@ -595,7 +595,7 @@ pub(crate) const AST_SRC: AstSrc = AstSrc {
qualifier: Path,
}
struct PathSegment {
T![::], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>]
T![::], T![crate], T![<], NameRef, TypeArgList, ParamList, RetType, PathType, T![>]
}
struct TypeArgList {
T![::],