Merge #991

991: Use Marker argument for item parsers r=matklad a=pcpthm

Before doing this for expressions, I found that the pattern (Marker argument) should be applied to the item parsers because visiblity and modifiers are parsed in a separate function.

Fixed some parser bugs:
- Fix pub_expr: `pub 42;` was allowed.
- Fix incorrect parsing of crate::path: incorrectly parsed as `crate` as a visibility.

Co-authored-by: pcpthm <[email protected]>

6 files changed, 61 insertions, 71 deletions

diff --git a/crates/ra_parser/src/grammar.rs b/crates/ra_parser/src/grammar.rs
index c9941fe93..e428faffb 100644
--- a/crates/ra_parser/src/grammar.rs
+++ b/crates/ra_parser/src/grammar.rs
@@ -86,7 +86,7 @@ impl BlockLike {
     }
 }
 
-fn opt_visibility(p: &mut Parser) {
+fn opt_visibility(p: &mut Parser) -> bool {
     match p.current() {
         PUB_KW => {
             let m = p.start();
@@ -116,13 +116,19 @@ fn opt_visibility(p: &mut Parser) {
         }
         // test crate_keyword_vis
         // crate fn main() { }
-        CRATE_KW => {
+        // struct S { crate field: u32 }
+        // struct T(crate u32);
+        //
+        // test crate_keyword_path
+        // fn foo() { crate::foo(); }
+        CRATE_KW if p.nth(1) != COLONCOLON => {
             let m = p.start();
             p.bump();
             m.complete(p, VISIBILITY);
         }
-        _ => (),
+        _ => return false,
     }
+    true
 }
 
 fn opt_alias(p: &mut Parser) {
diff --git a/crates/ra_parser/src/grammar/expressions.rs b/crates/ra_parser/src/grammar/expressions.rs
index ccbc905ab..83812e938 100644
--- a/crates/ra_parser/src/grammar/expressions.rs
+++ b/crates/ra_parser/src/grammar/expressions.rs
@@ -67,8 +67,6 @@ pub(crate) fn expr_block_contents(p: &mut Parser) {
             Err(m) => m,
         };
 
-        // test pub_expr
-        // fn foo() { pub 92; } //FIXME
         if has_attrs {
             m.abandon(p);
             p.error("expected a let statement or an item after attributes in block");
diff --git a/crates/ra_parser/src/grammar/items.rs b/crates/ra_parser/src/grammar/items.rs
index 94b93a02b..c24e6d1e0 100644
--- a/crates/ra_parser/src/grammar/items.rs
+++ b/crates/ra_parser/src/grammar/items.rs
@@ -67,11 +67,14 @@ pub(super) fn item_or_macro(p: &mut Parser, stop_on_r_curly: bool, flavor: ItemF
 }
 
 pub(super) fn maybe_item(p: &mut Parser, m: Marker, flavor: ItemFlavor) -> Result<(), Marker> {
-    opt_visibility(p);
-    if let Some(kind) = items_without_modifiers(p) {
-        m.complete(p, kind);
-        return Ok(());
-    }
+    // test_err pub_expr
+    // fn foo() { pub 92; }
+    let has_visibility = opt_visibility(p);
+
+    let m = match items_without_modifiers(p, m) {
+        Ok(()) => return Ok(()),
+        Err(m) => m,
+    };
 
     let mut has_mods = false;
 
@@ -152,10 +155,14 @@ pub(super) fn maybe_item(p: &mut Parser, m: Marker, flavor: ItemFlavor) -> Resul
             m.complete(p, IMPL_BLOCK);
         }
         _ => {
-            if !has_mods {
+            if !has_visibility && !has_mods {
                 return Err(m);
             } else {
-                p.error("expected fn, trait or impl");
+                if has_mods {
+                    p.error("expected fn, trait or impl");
+                } else {
+                    p.error("expected an item");
+                }
                 m.complete(p, ERROR);
             }
         }
@@ -163,23 +170,14 @@ pub(super) fn maybe_item(p: &mut Parser, m: Marker, flavor: ItemFlavor) -> Resul
     Ok(())
 }
 
-fn items_without_modifiers(p: &mut Parser) -> Option<SyntaxKind> {
+fn items_without_modifiers(p: &mut Parser, m: Marker) -> Result<(), Marker> {
     let la = p.nth(1);
-    let kind = match p.current() {
+    match p.current() {
         // test extern_crate
         // extern crate foo;
-        EXTERN_KW if la == CRATE_KW => {
-            extern_crate_item(p);
-            EXTERN_CRATE_ITEM
-        }
-        TYPE_KW => {
-            type_def(p);
-            TYPE_ALIAS_DEF
-        }
-        MOD_KW => {
-            mod_item(p);
-            MODULE
-        }
+        EXTERN_KW if la == CRATE_KW => extern_crate_item(p, m),
+        TYPE_KW => type_def(p, m),
+        MOD_KW => mod_item(p, m),
         STRUCT_KW => {
             // test struct_items
             // struct Foo;
@@ -190,14 +188,7 @@ fn items_without_modifiers(p: &mut Parser) -> Option<SyntaxKind> {
             //     a: i32,
             //     b: f32,
             // }
-            nominal::struct_def(p, STRUCT_KW);
-            if p.at(SEMI) {
-                p.err_and_bump(
-                    "expected item, found `;`\n\
-                     consider removing this semicolon",
-                );
-            }
-            STRUCT_DEF
+            nominal::struct_def(p, m, STRUCT_KW);
         }
         IDENT if p.at_contextual_kw("union") && p.nth(1) == IDENT => {
             // test union_items
@@ -206,25 +197,12 @@ fn items_without_modifiers(p: &mut Parser) -> Option<SyntaxKind> {
             //     a: i32,
             //     b: f32,
             // }
-            nominal::struct_def(p, UNION_KW);
-            STRUCT_DEF
-        }
-        ENUM_KW => {
-            nominal::enum_def(p);
-            ENUM_DEF
-        }
-        USE_KW => {
-            use_item::use_item(p);
-            USE_ITEM
-        }
-        CONST_KW if (la == IDENT || la == MUT_KW) => {
-            consts::const_def(p);
-            CONST_DEF
-        }
-        STATIC_KW => {
-            consts::static_def(p);
-            STATIC_DEF
+            nominal::struct_def(p, m, UNION_KW);
         }
+        ENUM_KW => nominal::enum_def(p, m),
+        USE_KW => use_item::use_item(p, m),
+        CONST_KW if (la == IDENT || la == MUT_KW) => consts::const_def(p, m),
+        STATIC_KW => consts::static_def(p, m),
         // test extern_block
         // extern {}
         EXTERN_KW
@@ -232,14 +210,20 @@ fn items_without_modifiers(p: &mut Parser) -> Option<SyntaxKind> {
         {
             abi(p);
             extern_item_list(p);
-            EXTERN_BLOCK
+            m.complete(p, EXTERN_BLOCK);
         }
-        _ => return None,
+        _ => return Err(m),
     };
-    Some(kind)
+    if p.at(SEMI) {
+        p.err_and_bump(
+            "expected item, found `;`\n\
+             consider removing this semicolon",
+        );
+    }
+    Ok(())
 }
 
-fn extern_crate_item(p: &mut Parser) {
+fn extern_crate_item(p: &mut Parser, m: Marker) {
     assert!(p.at(EXTERN_KW));
     p.bump();
     assert!(p.at(CRATE_KW));
@@ -247,6 +231,7 @@ fn extern_crate_item(p: &mut Parser) {
     name_ref(p);
     opt_alias(p);
     p.expect(SEMI);
+    m.complete(p, EXTERN_CRATE_ITEM);
 }
 
 pub(crate) fn extern_item_list(p: &mut Parser) {
@@ -295,7 +280,7 @@ fn fn_def(p: &mut Parser, flavor: ItemFlavor) {
 
 // test type_item
 // type Foo = Bar;
-fn type_def(p: &mut Parser) {
+fn type_def(p: &mut Parser, m: Marker) {
     assert!(p.at(TYPE_KW));
     p.bump();
 
@@ -317,9 +302,10 @@ fn type_def(p: &mut Parser) {
         types::type_(p);
     }
     p.expect(SEMI);
+    m.complete(p, TYPE_ALIAS_DEF);
 }
 
-pub(crate) fn mod_item(p: &mut Parser) {
+pub(crate) fn mod_item(p: &mut Parser, m: Marker) {
     assert!(p.at(MOD_KW));
     p.bump();
 
@@ -329,6 +315,7 @@ pub(crate) fn mod_item(p: &mut Parser) {
     } else if !p.eat(SEMI) {
         p.error("expected `;` or `{`");
     }
+    m.complete(p, MODULE);
 }
 
 pub(crate) fn mod_item_list(p: &mut Parser) {
diff --git a/crates/ra_parser/src/grammar/items/consts.rs b/crates/ra_parser/src/grammar/items/consts.rs
index 5a5852f83..e6e6011c6 100644
--- a/crates/ra_parser/src/grammar/items/consts.rs
+++ b/crates/ra_parser/src/grammar/items/consts.rs
@@ -1,14 +1,14 @@
 use super::*;
 
-pub(super) fn static_def(p: &mut Parser) {
-    const_or_static(p, STATIC_KW)
+pub(super) fn static_def(p: &mut Parser, m: Marker) {
+    const_or_static(p, m, STATIC_KW, STATIC_DEF)
 }
 
-pub(super) fn const_def(p: &mut Parser) {
-    const_or_static(p, CONST_KW)
+pub(super) fn const_def(p: &mut Parser, m: Marker) {
+    const_or_static(p, m, CONST_KW, CONST_DEF)
 }
 
-fn const_or_static(p: &mut Parser, kw: SyntaxKind) {
+fn const_or_static(p: &mut Parser, m: Marker, kw: SyntaxKind, def: SyntaxKind) {
     assert!(p.at(kw));
     p.bump();
     p.eat(MUT_KW); // TODO: validator to forbid const mut
@@ -18,4 +18,5 @@ fn const_or_static(p: &mut Parser, kw: SyntaxKind) {
         expressions::expr(p);
     }
     p.expect(SEMI);
+    m.complete(p, def);
 }
diff --git a/crates/ra_parser/src/grammar/items/nominal.rs b/crates/ra_parser/src/grammar/items/nominal.rs
index ff9b38f9c..a3579eebd 100644
--- a/crates/ra_parser/src/grammar/items/nominal.rs
+++ b/crates/ra_parser/src/grammar/items/nominal.rs
@@ -1,6 +1,6 @@
 use super::*;
 
-pub(super) fn struct_def(p: &mut Parser, kind: SyntaxKind) {
+pub(super) fn struct_def(p: &mut Parser, m: Marker, kind: SyntaxKind) {
     assert!(p.at(STRUCT_KW) || p.at_contextual_kw("union"));
     p.bump_remap(kind);
 
@@ -12,19 +12,16 @@ pub(super) fn struct_def(p: &mut Parser, kind: SyntaxKind) {
             match p.current() {
                 SEMI => {
                     p.bump();
-                    return;
                 }
                 L_CURLY => named_field_def_list(p),
                 _ => {
                     //TODO: special case `(` error message
                     p.error("expected `;` or `{`");
-                    return;
                 }
             }
         }
         SEMI if kind == STRUCT_KW => {
             p.bump();
-            return;
         }
         L_CURLY => named_field_def_list(p),
         L_PAREN if kind == STRUCT_KW => {
@@ -37,16 +34,15 @@ pub(super) fn struct_def(p: &mut Parser, kind: SyntaxKind) {
         }
         _ if kind == STRUCT_KW => {
             p.error("expected `;`, `{`, or `(`");
-            return;
         }
         _ => {
             p.error("expected `{`");
-            return;
         }
     }
+    m.complete(p, STRUCT_DEF);
 }
 
-pub(super) fn enum_def(p: &mut Parser) {
+pub(super) fn enum_def(p: &mut Parser, m: Marker) {
     assert!(p.at(ENUM_KW));
     p.bump();
     name_r(p, ITEM_RECOVERY_SET);
@@ -57,6 +53,7 @@ pub(super) fn enum_def(p: &mut Parser) {
     } else {
         p.error("expected `{`")
     }
+    m.complete(p, ENUM_DEF);
 }
 
 pub(crate) fn enum_variant_list(p: &mut Parser) {
diff --git a/crates/ra_parser/src/grammar/items/use_item.rs b/crates/ra_parser/src/grammar/items/use_item.rs
index b36612726..ea2f94604 100644
--- a/crates/ra_parser/src/grammar/items/use_item.rs
+++ b/crates/ra_parser/src/grammar/items/use_item.rs
@@ -1,10 +1,11 @@
 use super::*;
 
-pub(super) fn use_item(p: &mut Parser) {
+pub(super) fn use_item(p: &mut Parser, m: Marker) {
     assert!(p.at(USE_KW));
     p.bump();
     use_tree(p);
     p.expect(SEMI);
+    m.complete(p, USE_ITEM);
 }
 
 /// Parse a use 'tree', such as `some::path` in `use some::path;`