Merge #4227

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 <[email protected]>

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 e3b991c8c..3a0c7a31a 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 a30bc97bb..f0b3dec63 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 8306f7361..d2a549273 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 @@
-[email protected]3
+SOURCE_FILE@0..98
   [email protected]
     [email protected] "use"
     [email protected] " "
@@ -9,11 +9,11 @@ [email protected]
           [email protected] "crate"
     [email protected] ";"
   [email protected] "\n"
-  USE_ITEM@13..39
+  USE_ITEM@13..54
     [email protected] "use"
     [email protected] " "
-    [email protected]8
-      [email protected]8
+    USE_TREE@17..53
+      USE_TREE_LIST@17..53
         [email protected] "{"
         [email protected]
           [email protected]
@@ -21,56 +21,71 @@ [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
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 bead4c0b6..508def2c7 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 bd74b44a6..cf3a90400 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 @@
-[email protected]50
+[email protected]49
   [email protected]
     [email protected] "use"
     [email protected] " "
@@ -104,32 +104,33 @@ [email protected]
   [email protected] " "
   [email protected] "// Rust 2015"
   [email protected] "\n"
-  [email protected]6
+  [email protected]5
     [email protected] "use"
     [email protected] " "
-    [email protected]5
+    [email protected]4
       [email protected] "::"
-      [email protected]5
+      [email protected]4
         [email protected] "{"
-        [email protected]4
-          [email protected]4
+        [email protected]3
+          [email protected]3
             [email protected] "{"
-            [email protected]3
-              [email protected]3
+            [email protected]2
+              [email protected]2
                 [email protected] "{"
-                [email protected]
-                  [email protected]
-                    [email protected]
-                      [email protected]
-                        [email protected] "crate"
-                    [email protected] "::"
-                    [email protected]
-                      [email protected]
-                        [email protected] "export"
-                [email protected] "}"
-            [email protected] "}"
-        [email protected] "}"
-    [email protected] ";"
-  [email protected] " "
-  [email protected] "// Nonsensical but pe ..."
-  [email protected] "\n"
+                [email protected]
+                  [email protected]
+                    [email protected]
+                      [email protected]
+                        [email protected]
+                          [email protected] "root"
+                    [email protected] "::"
+                    [email protected]
+                      [email protected]
+                        [email protected] "export"
+                [email protected] "}"
+            [email protected] "}"
+        [email protected] "}"
+    [email protected] ";"
+  [email protected] " "
+  [email protected] "// Nonsensical but pe ..."
+  [email protected] "\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 06c387cee..381cba1e2 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