From 513a3615f6d462852c0135dc4ac30a2086e25c5a Mon Sep 17 00:00:00 2001 From: John Renner Date: Thu, 30 Apr 2020 10:41:24 -0700 Subject: [PATCH] Report invalid, nested, multi-segment crate-paths Specifically, things like: use foo::{crate::bar}; Are now being caught, when before we only caught: use foo::{crate}; --- .../ra_parser/src/grammar/items/use_item.rs | 2 +- crates/ra_syntax/src/validation.rs | 29 ++++- .../err/0040_illegal_crate_kw_location.rast | 113 ++++++++++-------- .../err/0040_illegal_crate_kw_location.rs | 2 +- .../parser/inline/ok/0002_use_tree_list.rast | 49 ++++---- .../parser/inline/ok/0002_use_tree_list.rs | 2 +- 6 files changed, 116 insertions(+), 81 deletions(-) diff --git a/crates/ra_parser/src/grammar/items/use_item.rs b/crates/ra_parser/src/grammar/items/use_item.rs index e3b991c8c6c..3a0c7a31a85 100644 --- a/crates/ra_parser/src/grammar/items/use_item.rs +++ b/crates/ra_parser/src/grammar/items/use_item.rs @@ -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 {path::from::root}; // 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!['{'] => { use_tree_list(p); } diff --git a/crates/ra_syntax/src/validation.rs b/crates/ra_syntax/src/validation.rs index a30bc97bb0e..f0b3dec631f 100644 --- a/crates/ra_syntax/src/validation.rs +++ b/crates/ra_syntax/src/validation.rs @@ -236,21 +236,40 @@ fn validate_crate_keyword_in_path_segment( }; // 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() { 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 + // 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; + } + + // 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())); } diff --git a/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rast b/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rast index 8306f736102..d2a5492733b 100644 --- a/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rast +++ b/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rast @@ -1,4 +1,4 @@ -SOURCE_FILE@0..83 +SOURCE_FILE@0..98 USE_ITEM@0..12 USE_KW@0..3 "use" WHITESPACE@3..4 " " @@ -9,11 +9,11 @@ SOURCE_FILE@0..83 CRATE_KW@6..11 "crate" SEMICOLON@11..12 ";" WHITESPACE@12..13 "\n" - USE_ITEM@13..39 + USE_ITEM@13..54 USE_KW@13..16 "use" WHITESPACE@16..17 " " - USE_TREE@17..38 - USE_TREE_LIST@17..38 + USE_TREE@17..53 + USE_TREE_LIST@17..53 L_CURLY@17..18 "{" USE_TREE@18..23 PATH@18..23 @@ -21,56 +21,71 @@ SOURCE_FILE@0..83 CRATE_KW@18..23 "crate" COMMA@23..24 "," WHITESPACE@24..25 " " - USE_TREE@25..37 + USE_TREE@25..52 PATH@25..28 PATH_SEGMENT@25..28 NAME_REF@25..28 IDENT@25..28 "foo" COLON2@28..30 "::" - USE_TREE_LIST@30..37 + USE_TREE_LIST@30..52 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" + USE_TREE@31..51 + PATH@31..51 + PATH@31..46 + PATH@31..41 + PATH@31..36 + PATH_SEGMENT@31..36 + CRATE_KW@31..36 "crate" + COLON2@36..38 "::" + PATH_SEGMENT@38..41 + NAME_REF@38..41 + IDENT@38..41 "foo" + COLON2@41..43 "::" + PATH_SEGMENT@43..46 + NAME_REF@43..46 + IDENT@43..46 "bar" + COLON2@46..48 "::" + PATH_SEGMENT@48..51 + NAME_REF@48..51 + IDENT@48..51 "baz" + R_CURLY@51..52 "}" + R_CURLY@52..53 "}" + SEMICOLON@53..54 ";" + WHITESPACE@54..55 "\n" + USE_ITEM@55..72 + USE_KW@55..58 "use" + WHITESPACE@58..59 " " + USE_TREE@59..71 + PATH@59..71 + PATH@59..64 + PATH_SEGMENT@59..64 + NAME_REF@59..64 + IDENT@59..64 "hello" + COLON2@64..66 "::" + PATH_SEGMENT@66..71 + CRATE_KW@66..71 "crate" + SEMICOLON@71..72 ";" + WHITESPACE@72..73 "\n" + USE_ITEM@73..97 + USE_KW@73..76 "use" + WHITESPACE@76..77 " " + 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 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 +error 66..71: 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 diff --git a/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rs b/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rs index bead4c0b6ea..508def2c7ef 100644 --- a/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rs +++ b/crates/ra_syntax/test_data/parser/err/0040_illegal_crate_kw_location.rs @@ -1,4 +1,4 @@ use ::crate; -use {crate, foo::{crate}}; +use {crate, foo::{crate::foo::bar::baz}}; use hello::crate; use hello::crate::there; diff --git a/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rast b/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rast index bd74b44a68b..cf3a90400a8 100644 --- a/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rast +++ b/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rast @@ -1,4 +1,4 @@ -SOURCE_FILE@0..250 +SOURCE_FILE@0..249 USE_ITEM@0..58 USE_KW@0..3 "use" WHITESPACE@3..4 " " @@ -104,32 +104,33 @@ SOURCE_FILE@0..250 WHITESPACE@166..167 " " COMMENT@167..179 "// Rust 2015" WHITESPACE@179..180 "\n" - USE_ITEM@180..206 + USE_ITEM@180..205 USE_KW@180..183 "use" WHITESPACE@183..184 " " - USE_TREE@184..205 + USE_TREE@184..204 COLON2@184..186 "::" - USE_TREE_LIST@186..205 + USE_TREE_LIST@186..204 L_CURLY@186..187 "{" - USE_TREE@187..204 - USE_TREE_LIST@187..204 + USE_TREE@187..203 + USE_TREE_LIST@187..203 L_CURLY@187..188 "{" - USE_TREE@188..203 - USE_TREE_LIST@188..203 + USE_TREE@188..202 + USE_TREE_LIST@188..202 L_CURLY@188..189 "{" - USE_TREE@189..202 - PATH@189..202 - PATH@189..194 - PATH_SEGMENT@189..194 - CRATE_KW@189..194 "crate" - COLON2@194..196 "::" - PATH_SEGMENT@196..202 - NAME_REF@196..202 - IDENT@196..202 "export" - R_CURLY@202..203 "}" - R_CURLY@203..204 "}" - R_CURLY@204..205 "}" - SEMICOLON@205..206 ";" - WHITESPACE@206..207 " " - COMMENT@207..249 "// Nonsensical but pe ..." - WHITESPACE@249..250 "\n" + USE_TREE@189..201 + PATH@189..201 + PATH@189..193 + PATH_SEGMENT@189..193 + NAME_REF@189..193 + IDENT@189..193 "root" + COLON2@193..195 "::" + PATH_SEGMENT@195..201 + NAME_REF@195..201 + IDENT@195..201 "export" + R_CURLY@201..202 "}" + R_CURLY@202..203 "}" + R_CURLY@203..204 "}" + SEMICOLON@204..205 ";" + WHITESPACE@205..206 " " + COMMENT@206..248 "// Nonsensical but pe ..." + WHITESPACE@248..249 "\n" diff --git a/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rs b/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rs index 06c387cee39..381cba1e29e 100644 --- a/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rs +++ b/crates/ra_syntax/test_data/parser/inline/ok/0002_use_tree_list.rs @@ -1,4 +1,4 @@ use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`) use {path::from::root}; // Rust 2015 use ::{some::arbritrary::path}; // Rust 2015 -use ::{{{crate::export}}}; // Nonsensical but perfectly legal nestnig +use ::{{{root::export}}}; // Nonsensical but perfectly legal nesting