mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #4227

Browse Source 
4227: Report invalid, nested, multi-segment crate-paths r=matklad a=djrenren

There was a bug in the previous path-validating code that didn't detect multi-segment paths that started with `crate`.

```rust
// Successfully reported
use foo::{crate};

// BUG: was not being reported
use foo::{crate::bar};
```

This was due to my confusion about path-associativity. That is, the path with no qualifier is the innermost path, not the outermost. I've updated the code with a lot of comments to explain what's going on. 

This bug was discovered when I found an erroneous `ok` test which I reported here: 
https://github.com/rust-analyzer/rust-analyzer/issues/4226

This test now fails and has been modified, hopefully in the spirit of the original test, to be correct.  Sorry about submitting the bug in the first place!

Co-authored-by: John Renner <john@jrenner.net>
This commit is contained in:
bors[bot] 2020-04-30 18:37:35 +00:00 committed by GitHub
commit 745bd45ddb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 116 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/ra_parser/src/grammar/items/use_item.rs
View File

@ -47,7 +47,7 @@ fn use_tree(p: &mut Parser, top_level: bool) {
// use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`) // use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`)
// use {path::from::root}; // Rust 2015 // use {path::from::root}; // Rust 2015
// use ::{some::arbritrary::path}; // Rust 2015 // use ::{some::arbritrary::path}; // Rust 2015
// use ::{{{crate::export}}}; // Nonsensical but perfectly legal nestnig // use ::{{{root::export}}}; // Nonsensical but perfectly legal nesting
T!['{'] => { T!['{'] => {
use_tree_list(p); use_tree_list(p);
} }

29
crates/ra_syntax/src/validation.rs
View File

@ -236,21 +236,40 @@ fn validate_crate_keyword_in_path_segment(
}; };
// Disallow both ::crate and foo::crate // Disallow both ::crate and foo::crate
let path = segment.parent_path(); let mut path = segment.parent_path();
if segment.coloncolon_token().is_some() || path.qualifier().is_some() { if segment.coloncolon_token().is_some() || path.qualifier().is_some() {
errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range())); errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range()));
return; return;
} }
// We now know that the path variable describes a complete path.
// For expressions and types, validation is complete, but we still have // For expressions and types, validation is complete, but we still have
// to handle UseItems like this: // to handle invalid UseItems like this:
// use foo:{crate}; //
// so we crawl upwards looking for any preceding paths on `UseTree`s // 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;
}
// 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) { for node in path.syntax().ancestors().skip(1) {
match_ast! { match_ast! {
match node { match node {
ast::UseTree(it) => if let Some(tree_path) = it.path() { 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 { if tree_path != path {
errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range())); errors.push(SyntaxError::new(ERR_MSG, crate_token.text_range()));
} }

113
crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rast
View File

@ -1,4 +1,4 @@
SOURCE_FILE@0..83 SOURCE_FILE@0..98
USE_ITEM@0..12 USE_ITEM@0..12
USE_KW@0..3 "use" USE_KW@0..3 "use"
WHITESPACE@3..4 " " WHITESPACE@3..4 " "
@ -9,11 +9,11 @@ SOURCE_FILE@0..83
CRATE_KW@6..11 "crate" CRATE_KW@6..11 "crate"
SEMICOLON@11..12 ";" SEMICOLON@11..12 ";"
WHITESPACE@12..13 "\n" WHITESPACE@12..13 "\n"
USE_ITEM@13..39 USE_ITEM@13..54
USE_KW@13..16 "use" USE_KW@13..16 "use"
WHITESPACE@16..17 " " WHITESPACE@16..17 " "
USE_TREE@17..38 USE_TREE@17..53
USE_TREE_LIST@17..38 USE_TREE_LIST@17..53
L_CURLY@17..18 "{" L_CURLY@17..18 "{"
USE_TREE@18..23 USE_TREE@18..23
PATH@18..23 PATH@18..23
@ -21,56 +21,71 @@ SOURCE_FILE@0..83
CRATE_KW@18..23 "crate" CRATE_KW@18..23 "crate"
COMMA@23..24 "," COMMA@23..24 ","
WHITESPACE@24..25 " " WHITESPACE@24..25 " "
USE_TREE@25..37 USE_TREE@25..52
PATH@25..28 PATH@25..28
PATH_SEGMENT@25..28 PATH_SEGMENT@25..28
NAME_REF@25..28 NAME_REF@25..28
IDENT@25..28 "foo" IDENT@25..28 "foo"
COLON2@28..30 "::" COLON2@28..30 "::"
USE_TREE_LIST@30..37 USE_TREE_LIST@30..52
L_CURLY@30..31 "{" L_CURLY@30..31 "{"
USE_TREE@31..36 USE_TREE@31..51
PATH@31..36 PATH@31..51
PATH_SEGMENT@31..36 PATH@31..46
CRATE_KW@31..36 "crate" PATH@31..41
R_CURLY@36..37 "}" PATH@31..36
R_CURLY@37..38 "}" PATH_SEGMENT@31..36
SEMICOLON@38..39 ";" CRATE_KW@31..36 "crate"
WHITESPACE@39..40 "\n" COLON2@36..38 "::"
USE_ITEM@40..57 PATH_SEGMENT@38..41
USE_KW@40..43 "use" NAME_REF@38..41
WHITESPACE@43..44 " " IDENT@38..41 "foo"
USE_TREE@44..56 COLON2@41..43 "::"
PATH@44..56 PATH_SEGMENT@43..46
PATH@44..49 NAME_REF@43..46
PATH_SEGMENT@44..49 IDENT@43..46 "bar"
NAME_REF@44..49 COLON2@46..48 "::"
IDENT@44..49 "hello" PATH_SEGMENT@48..51
COLON2@49..51 "::" NAME_REF@48..51
PATH_SEGMENT@51..56 IDENT@48..51 "baz"
CRATE_KW@51..56 "crate" R_CURLY@51..52 "}"
SEMICOLON@56..57 ";" R_CURLY@52..53 "}"
WHITESPACE@57..58 "\n" SEMICOLON@53..54 ";"
USE_ITEM@58..82 WHITESPACE@54..55 "\n"
USE_KW@58..61 "use" USE_ITEM@55..72
WHITESPACE@61..62 " " USE_KW@55..58 "use"
USE_TREE@62..81 WHITESPACE@58..59 " "
PATH@62..81 USE_TREE@59..71
PATH@62..74 PATH@59..71
PATH@62..67 PATH@59..64
PATH_SEGMENT@62..67 PATH_SEGMENT@59..64
NAME_REF@62..67 NAME_REF@59..64
IDENT@62..67 "hello" IDENT@59..64 "hello"
COLON2@67..69 "::" COLON2@64..66 "::"
PATH_SEGMENT@69..74 PATH_SEGMENT@66..71
CRATE_KW@69..74 "crate" CRATE_KW@66..71 "crate"
COLON2@74..76 "::" SEMICOLON@71..72 ";"
PATH_SEGMENT@76..81 WHITESPACE@72..73 "\n"
NAME_REF@76..81 USE_ITEM@73..97
IDENT@76..81 "there" USE_KW@73..76 "use"
SEMICOLON@81..82 ";" WHITESPACE@76..77 " "
WHITESPACE@82..83 "\n" USE_TREE@77..96
PATH@77..96
PATH@77..89
PATH@77..82
PATH_SEGMENT@77..82
NAME_REF@77..82
IDENT@77..82 "hello"
COLON2@82..84 "::"
PATH_SEGMENT@84..89
CRATE_KW@84..89 "crate"
COLON2@89..91 "::"
PATH_SEGMENT@91..96
NAME_REF@91..96
IDENT@91..96 "there"
SEMICOLON@96..97 ";"
WHITESPACE@97..98 "\n"
error 6..11: The `crate` keyword is only allowed as the first segment of a path 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 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 66..71: 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 error 84..89: The `crate` keyword is only allowed as the first segment of a path

2
crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rs
View File

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

49
crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rast
View File

@ -1,4 +1,4 @@
SOURCE_FILE@0..250 SOURCE_FILE@0..249
USE_ITEM@0..58 USE_ITEM@0..58
USE_KW@0..3 "use" USE_KW@0..3 "use"
WHITESPACE@3..4 " " WHITESPACE@3..4 " "
@ -104,32 +104,33 @@ SOURCE_FILE@0..250
WHITESPACE@166..167 " " WHITESPACE@166..167 " "
COMMENT@167..179 "// Rust 2015" COMMENT@167..179 "// Rust 2015"
WHITESPACE@179..180 "\n" WHITESPACE@179..180 "\n"
USE_ITEM@180..206 USE_ITEM@180..205
USE_KW@180..183 "use" USE_KW@180..183 "use"
WHITESPACE@183..184 " " WHITESPACE@183..184 " "
USE_TREE@184..205 USE_TREE@184..204
COLON2@184..186 "::" COLON2@184..186 "::"
USE_TREE_LIST@186..205 USE_TREE_LIST@186..204
L_CURLY@186..187 "{" L_CURLY@186..187 "{"
USE_TREE@187..204 USE_TREE@187..203
USE_TREE_LIST@187..204 USE_TREE_LIST@187..203
L_CURLY@187..188 "{" L_CURLY@187..188 "{"
USE_TREE@188..203 USE_TREE@188..202
USE_TREE_LIST@188..203 USE_TREE_LIST@188..202
L_CURLY@188..189 "{" L_CURLY@188..189 "{"
USE_TREE@189..202 USE_TREE@189..201
PATH@189..202 PATH@189..201
PATH@189..194 PATH@189..193
PATH_SEGMENT@189..194 PATH_SEGMENT@189..193
CRATE_KW@189..194 "crate" NAME_REF@189..193
COLON2@194..196 "::" IDENT@189..193 "root"
PATH_SEGMENT@196..202 COLON2@193..195 "::"
NAME_REF@196..202 PATH_SEGMENT@195..201
IDENT@196..202 "export" NAME_REF@195..201
R_CURLY@202..203 "}" IDENT@195..201 "export"
R_CURLY@203..204 "}" R_CURLY@201..202 "}"
R_CURLY@204..205 "}" R_CURLY@202..203 "}"
SEMICOLON@205..206 ";" R_CURLY@203..204 "}"
WHITESPACE@206..207 " " SEMICOLON@204..205 ";"
COMMENT@207..249 "// Nonsensical but pe ..." WHITESPACE@205..206 " "
WHITESPACE@249..250 "\n" COMMENT@206..248 "// Nonsensical but pe ..."
WHITESPACE@248..249 "\n"

2
crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rs
View File

@ -1,4 +1,4 @@
use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`) use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`)
use {path::from::root}; // Rust 2015 use {path::from::root}; // Rust 2015
use ::{some::arbritrary::path}; // Rust 2015 use ::{some::arbritrary::path}; // Rust 2015
use ::{{{crate::export}}}; // Nonsensical but perfectly legal nestnig use ::{{{root::export}}}; // Nonsensical but perfectly legal nesting