Merge #996

996: Allow attributes on top level expressions r=matklad a=pcpthm

This PR modifies parser to allow outer attributes on top level expression. Here, top level expression means either
- Expression statement e.g. `foo();`
- Last expression in a block without semicolon `bar()` in `{ foo(); bar() }`.

Except for binary operation expressions and `if` expressions, which are errors (feature gated) in rustc.
Attributes on inner expressions like `foo(#[a] 1)` are not implemented.

I first tried to implement this by passing `Maker` to expression parsers. However, this implementation couldn't parse `#[attr] foo()` correctly as `CallExpr(Attr(..), PathExpr(..), ArgList(..))` and instead parsed incorrectly as `CallExpr(PathExpr(Attr(..), ..), ArgList(..))` due to the way left recursion is handled.
In the end, I introduce `undo_completion` method. Which is not the suggested approach, but it seems not very bad.

Fix #759.


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

4 files changed, 94 insertions, 42 deletions

diff --git a/crates/ra_parser/src/event.rs b/crates/ra_parser/src/event.rs
index 6361d5d86..c1773e8e0 100644
--- a/crates/ra_parser/src/event.rs
+++ b/crates/ra_parser/src/event.rs
@@ -105,7 +105,9 @@ pub(super) fn process(sink: &mut dyn TreeSink, mut events: Vec<Event>) {
                     // append `A`'s forward_parent `B`
                     fp = match mem::replace(&mut events[idx], Event::tombstone()) {
                         Event::Start { kind, forward_parent } => {
-                            forward_parents.push(kind);
+                            if kind != TOMBSTONE {
+                                forward_parents.push(kind);
+                            }
                             forward_parent
                         }
                         _ => unreachable!(),
diff --git a/crates/ra_parser/src/grammar/expressions.rs b/crates/ra_parser/src/grammar/expressions.rs
index 83812e938..73e1acd5a 100644
--- a/crates/ra_parser/src/grammar/expressions.rs
+++ b/crates/ra_parser/src/grammar/expressions.rs
@@ -8,10 +8,10 @@ const EXPR_FIRST: TokenSet = LHS_FIRST;
 
 pub(super) fn expr(p: &mut Parser) -> BlockLike {
     let r = Restrictions { forbid_structs: false, prefer_stmt: false };
-    expr_bp(p, r, 1)
+    expr_bp(p, r, 1).1
 }
 
-pub(super) fn expr_stmt(p: &mut Parser) -> BlockLike {
+pub(super) fn expr_stmt(p: &mut Parser) -> (Option<CompletedMarker>, BlockLike) {
     let r = Restrictions { forbid_structs: false, prefer_stmt: true };
     expr_bp(p, r, 1)
 }
@@ -38,6 +38,13 @@ pub(crate) fn block(p: &mut Parser) {
     m.complete(p, BLOCK);
 }
 
+fn is_expr_stmt_attr_allowed(kind: SyntaxKind) -> bool {
+    match kind {
+        BIN_EXPR | RANGE_EXPR | IF_EXPR => false,
+        _ => true,
+    }
+}
+
 pub(crate) fn expr_block_contents(p: &mut Parser) {
     // This is checked by a validator
     attributes::inner_attributes(p);
@@ -55,6 +62,13 @@ pub(crate) fn expr_block_contents(p: &mut Parser) {
         // test block_items
         // fn a() { fn b() {} }
         let m = p.start();
+        // test attr_on_expr_stmt
+        // fn foo() {
+        //     #[A] foo();
+        //     #[B] bar!{}
+        //     #[C] #[D] {}
+        //     #[D] return ();
+        // }
         let has_attrs = p.at(POUND);
         attributes::outer_attributes(p);
         if p.at(LET_KW) {
@@ -67,35 +81,51 @@ pub(crate) fn expr_block_contents(p: &mut Parser) {
             Err(m) => m,
         };
 
-        if has_attrs {
-            m.abandon(p);
-            p.error("expected a let statement or an item after attributes in block");
-        } else {
-            let is_blocklike = expressions::expr_stmt(p) == BlockLike::Block;
-            if p.at(R_CURLY) {
+        let (cm, blocklike) = expr_stmt(p);
+        let kind = cm.as_ref().map(|cm| cm.kind()).unwrap_or(ERROR);
+
+        if has_attrs && !is_expr_stmt_attr_allowed(kind) {
+            // test_err attr_on_expr_not_allowed
+            // fn foo() {
+            //    #[A] 1 + 2;
+            //    #[B] if true {};
+            // }
+            p.error(format!("attributes are not allowed on {:?}", kind));
+        }
+
+        if p.at(R_CURLY) {
+            // test attr_on_last_expr_in_block
+            // fn foo() {
+            //     { #[A] bar!()? }
+            //     #[B] &()
+            // }
+            if let Some(cm) = cm {
+                cm.undo_completion(p).abandon(p);
+                m.complete(p, kind);
+            } else {
                 m.abandon(p);
+            }
+        } else {
+            // test no_semi_after_block
+            // fn foo() {
+            //     if true {}
+            //     loop {}
+            //     match () {}
+            //     while true {}
+            //     for _ in () {}
+            //     {}
+            //     {}
+            //     macro_rules! test {
+            //          () => {}
+            //     }
+            //     test!{}
+            // }
+            if blocklike.is_block() {
+                p.eat(SEMI);
             } else {
-                // test no_semi_after_block
-                // fn foo() {
-                //     if true {}
-                //     loop {}
-                //     match () {}
-                //     while true {}
-                //     for _ in () {}
-                //     {}
-                //     {}
-                //     macro_rules! test {
-                //          () => {}
-                //     }
-                //     test!{}
-                // }
-                if is_blocklike {
-                    p.eat(SEMI);
-                } else {
-                    p.expect(SEMI);
-                }
-                m.complete(p, EXPR_STMT);
+                p.expect(SEMI);
             }
+            m.complete(p, EXPR_STMT);
         }
     }
 
@@ -176,7 +206,7 @@ fn current_op(p: &Parser) -> (u8, Op) {
 }
 
 // Parses expression with binding power of at least bp.
-fn expr_bp(p: &mut Parser, r: Restrictions, bp: u8) -> BlockLike {
+fn expr_bp(p: &mut Parser, r: Restrictions, bp: u8) -> (Option<CompletedMarker>, BlockLike) {
     let mut lhs = match lhs(p, r) {
         Some((lhs, blocklike)) => {
             // test stmt_bin_expr_ambiguity
@@ -185,11 +215,11 @@ fn expr_bp(p: &mut Parser, r: Restrictions, bp: u8) -> BlockLike {
             //     {1} &2;
             // }
             if r.prefer_stmt && blocklike.is_block() {
-                return BlockLike::Block;
+                return (Some(lhs), BlockLike::Block);
             }
             lhs
         }
-        None => return BlockLike::NotBlock,
+        None => return (None, BlockLike::NotBlock),
     };
 
     loop {
@@ -208,7 +238,7 @@ fn expr_bp(p: &mut Parser, r: Restrictions, bp: u8) -> BlockLike {
         expr_bp(p, r, op_bp + 1);
         lhs = m.complete(p, if is_range { RANGE_EXPR } else { BIN_EXPR });
     }
-    BlockLike::NotBlock
+    (Some(lhs), BlockLike::NotBlock)
 }
 
 const LHS_FIRST: TokenSet =
diff --git a/crates/ra_parser/src/grammar/expressions/atom.rs b/crates/ra_parser/src/grammar/expressions/atom.rs
index d933288cd..a23977bfb 100644
--- a/crates/ra_parser/src/grammar/expressions/atom.rs
+++ b/crates/ra_parser/src/grammar/expressions/atom.rs
@@ -392,9 +392,9 @@ fn match_arm(p: &mut Parser) -> BlockLike {
         match_guard(p);
     }
     p.expect(FAT_ARROW);
-    let ret = expr_stmt(p);
+    let blocklike = expr_stmt(p).1;
     m.complete(p, MATCH_ARM);
-    ret
+    blocklike
 }
 
 // test match_guard
diff --git a/crates/ra_parser/src/parser.rs b/crates/ra_parser/src/parser.rs
index a18458148..3c326452b 100644
--- a/crates/ra_parser/src/parser.rs
+++ b/crates/ra_parser/src/parser.rs
@@ -212,8 +212,9 @@ impl Marker {
             }
             _ => unreachable!(),
         }
+        let finish_pos = p.events.len() as u32;
         p.push_event(Event::Finish);
-        CompletedMarker::new(self.pos, kind)
+        CompletedMarker::new(self.pos, finish_pos, kind)
     }
 
     /// Abandons the syntax tree node. All its children
@@ -230,11 +231,15 @@ impl Marker {
     }
 }
 
-pub(crate) struct CompletedMarker(u32, SyntaxKind);
+pub(crate) struct CompletedMarker {
+    start_pos: u32,
+    finish_pos: u32,
+    kind: SyntaxKind,
+}
 
 impl CompletedMarker {
-    fn new(pos: u32, kind: SyntaxKind) -> Self {
-        CompletedMarker(pos, kind)
+    fn new(start_pos: u32, finish_pos: u32, kind: SyntaxKind) -> Self {
+        CompletedMarker { start_pos, finish_pos, kind }
     }
 
     /// This method allows to create a new node which starts
@@ -251,17 +256,32 @@ impl CompletedMarker {
     /// distance to `NEWSTART` into forward_parent(=2 in this case);
     pub(crate) fn precede(self, p: &mut Parser) -> Marker {
         let new_pos = p.start();
-        let idx = self.0 as usize;
+        let idx = self.start_pos as usize;
         match p.events[idx] {
             Event::Start { ref mut forward_parent, .. } => {
-                *forward_parent = Some(new_pos.pos - self.0);
+                *forward_parent = Some(new_pos.pos - self.start_pos);
             }
             _ => unreachable!(),
         }
         new_pos
     }
 
+    /// Undo this completion and turns into a `Marker`
+    pub(crate) fn undo_completion(self, p: &mut Parser) -> Marker {
+        let start_idx = self.start_pos as usize;
+        let finish_idx = self.finish_pos as usize;
+        match p.events[start_idx] {
+            Event::Start { ref mut kind, forward_parent: None } => *kind = TOMBSTONE,
+            _ => unreachable!(),
+        }
+        match p.events[finish_idx] {
+            ref mut slot @ Event::Finish => *slot = Event::tombstone(),
+            _ => unreachable!(),
+        }
+        Marker::new(self.start_pos)
+    }
+
     pub(crate) fn kind(&self) -> SyntaxKind {
-        self.1
+        self.kind
     }
 }