Report invalid, nested, multi-segment crate-paths

djrenren · djrenren · commit 513a3615f6d4 · 2020-04-30T11:16:09.000-07:00
Specifically, things like:

use foo::{crate::bar};

Are now being caught, when before we only caught:

use foo::{crate};
diff --git 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
@@ -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
@@ -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,68 +9,83 @@ 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
             PATH_SEGMENT@18..23
               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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ fn use_tree(p: &mut Parser, top_level: bool) {`
`47`	`47`	// use {crate::path::from::root, or::path::from::crate_name}; // Rust 2018 (with a crate named `or`)
`48`	`48`	`// use {path::from::root}; // Rust 2015`
`49`	`49`	`// use ::{some::arbritrary::path}; // Rust 2015`
`50`		`- // use ::{{{crate::export}}}; // Nonsensical but perfectly legal nestnig`
	`50`	`+ // use ::{{{root::export}}}; // Nonsensical but perfectly legal nesting`
`51`	`51`	`T!['{'] => {`
`52`	`52`	`use_tree_list(p);`
`53`	`53`	`}`
-Original file line number
+Diff line change
@@ @@ -1,4 +1,4 @@ @@
 -SOURCE_FILE@0..83
 +SOURCE_FILE@0..98
   [email protected]
     [email protected] "use"
     [email protected] " "
           [email protected] "crate"
     [email protected] ";"
   [email protected] "\n"
 -  USE_ITEM@13..39
 +  USE_ITEM@13..54
     [email protected] "use"
     [email protected] " "
 -    USE_TREE@17..38
 -      USE_TREE_LIST@17..38
 +    USE_TREE@17..53
 +      USE_TREE_LIST@17..53
         [email protected] "{"
         [email protected]
           [email protected]
             [email protected]
               [email protected] "crate"
         [email protected] ","
         [email protected] " "
 -        USE_TREE@25..37
 +        USE_TREE@25..52
           [email protected]
             [email protected]
               [email protected]
                 [email protected] "foo"
           [email protected] "::"
 -          USE_TREE_LIST@30..37
 +          USE_TREE_LIST@30..52
             [email protected] "{"
 -            [email protected]
 -              [email protected]
 -                [email protected]
 -                  [email protected] "crate"
 -            [email protected] "}"
 -        [email protected] "}"
 -    [email protected] ";"
 -  [email protected] "\n"
 -  [email protected]
 -    [email protected] "use"
 -    [email protected] " "
 -    [email protected]
 -      [email protected]
 -        [email protected]
 -          [email protected]
 -            [email protected]
 -              [email protected] "hello"
 -        [email protected] "::"
 -        [email protected]
 -          [email protected] "crate"
 -    [email protected] ";"
 -  [email protected] "\n"
 -  [email protected]
 -    [email protected] "use"
 -    [email protected] " "
 -    [email protected]
 -      [email protected]
 -        [email protected]
 -          [email protected]
 -            [email protected]
 -              [email protected]
 -                [email protected] "hello"
 -          [email protected] "::"
 -          [email protected]
 -            [email protected] "crate"
 -        [email protected] "::"
 -        [email protected]
 -          [email protected]
 -            [email protected] "there"
 -    [email protected] ";"
 -  [email protected] "\n"
 +            [email protected]
 +              [email protected]
 +                [email protected]
 +                  [email protected]
 +                    [email protected]
 +                      [email protected]
 +                        [email protected] "crate"
 +                    [email protected] "::"
 +                    [email protected]
 +                      [email protected]
 +                        [email protected] "foo"
 +                  [email protected] "::"
 +                  [email protected]
 +                    [email protected]
 +                      [email protected] "bar"
 +                [email protected] "::"
 +                [email protected]
 +                  [email protected]
 +                    [email protected] "baz"
 +            [email protected] "}"
 +        [email protected] "}"
 +    [email protected] ";"
 +  [email protected] "\n"
 +  [email protected]
 +    [email protected] "use"
 +    [email protected] " "
 +    [email protected]
 +      [email protected]
 +        [email protected]
 +          [email protected]
 +            [email protected]
 +              [email protected] "hello"
 +        [email protected] "::"
 +        [email protected]
 +          [email protected] "crate"
 +    [email protected] ";"
 +  [email protected] "\n"
 +  [email protected]
 +    [email protected] "use"
 +    [email protected] " "
 +    [email protected]
 +      [email protected]
 +        [email protected]
 +          [email protected]
 +            [email protected]
 +              [email protected]
 +                [email protected] "hello"
 +          [email protected] "::"
 +          [email protected]
 +            [email protected] "crate"
 +        [email protected] "::"
 +        [email protected]
 +          [email protected]
 +            [email protected] "there"
 +    [email protected] ";"
 +  [email protected] "\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