Repository: rust-analyzer/rust-analyzer
Pull request #11167 (2022-01-02 15:43:37)

internal: check top level entry point invariants
bors r+
🤖


Patch:
diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs
index 1141365e82..7d7807206f 100644
--- a/crates/mbe/src/syntax_bridge.rs
+++ b/crates/mbe/src/syntax_bridge.rs
@@ -1,7 +1,7 @@
 //! Conversions between [`SyntaxNode`] and [`tt::TokenTree`].
 
 use rustc_hash::{FxHashMap, FxHashSet};
-use stdx::non_empty_vec::NonEmptyVec;
+use stdx::{never, non_empty_vec::NonEmptyVec};
 use syntax::{
     ast::{self, make::tokens::doc_comment},
     AstToken, Parse, PreorderWithTokens, SmolStr, SyntaxElement, SyntaxKind,
@@ -66,8 +66,7 @@ pub fn token_tree_to_syntax_node(
             parser::Step::Error { msg } => tree_sink.error(msg.to_string()),
         }
     }
-    if tree_sink.roots.len() != 1 {
-        cov_mark::hit!(expansion_does_not_parse_as_expression);
+    if never!(tree_sink.roots.len() != 1) {
         return Err(ExpandError::ConversionError);
     }
     //FIXME: would be cool to report errors
diff --git a/crates/parser/src/grammar.rs b/crates/parser/src/grammar.rs
index 6ffb4c191b..0240a6f14f 100644
--- a/crates/parser/src/grammar.rs
+++ b/crates/parser/src/grammar.rs
@@ -135,6 +135,32 @@ pub(crate) mod entry {
             }
             m.complete(p, ERROR);
         }
+
+        pub(crate) fn expr(p: &mut Parser) {
+            let m = p.start();
+            expressions::expr(p);
+            if p.at(EOF) {
+                m.abandon(p);
+                return;
+            }
+            while !p.at(EOF) {
+                p.bump_any();
+            }
+            m.complete(p, ERROR);
+        }
+
+        pub(crate) fn meta_item(p: &mut Parser) {
+            let m = p.start();
+            attributes::meta(p);
+            if p.at(EOF) {
+                m.abandon(p);
+                return;
+            }
+            while !p.at(EOF) {
+                p.bump_any();
+            }
+            m.complete(p, ERROR);
+        }
     }
 }
 
diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs
index 72d529d6cf..cff4ca4ba2 100644
--- a/crates/parser/src/lib.rs
+++ b/crates/parser/src/lib.rs
@@ -41,48 +41,6 @@ pub use crate::{
     syntax_kind::SyntaxKind,
 };
 
-/// Parse a prefix of the input as a given syntactic construct.
-///
-/// This is used by macro-by-example parser to implement things like `$i:item`
-/// and the naming of variants follows the naming of macro fragments.
-///
-/// Note that this is generally non-optional -- the result is intentionally not
-/// `Option<Output>`. The way MBE work, by the time we *try* to parse `$e:expr`
-/// we already commit to expression. In other words, this API by design can't be
-/// used to implement "rollback and try another alternative" logic.
-#[derive(Debug)]
-pub enum PrefixEntryPoint {
-    Vis,
-    Block,
-    Stmt,
-    Pat,
-    Ty,
-    Expr,
-    Path,
-    Item,
-    MetaItem,
-}
-
-impl PrefixEntryPoint {
-    pub fn parse(&self, input: &Input) -> Output {
-        let entry_point: fn(&'_ mut parser::Parser) = match self {
-            PrefixEntryPoint::Vis => grammar::entry::prefix::vis,
-            PrefixEntryPoint::Block => grammar::entry::prefix::block,
-            PrefixEntryPoint::Stmt => grammar::entry::prefix::stmt,
-            PrefixEntryPoint::Pat => grammar::entry::prefix::pat,
-            PrefixEntryPoint::Ty => grammar::entry::prefix::ty,
-            PrefixEntryPoint::Expr => grammar::entry::prefix::expr,
-            PrefixEntryPoint::Path => grammar::entry::prefix::path,
-            PrefixEntryPoint::Item => grammar::entry::prefix::item,
-            PrefixEntryPoint::MetaItem => grammar::entry::prefix::meta_item,
-        };
-        let mut p = parser::Parser::new(input);
-        entry_point(&mut p);
-        let events = p.finish();
-        event::process(events)
-    }
-}
-
 /// Parse the whole of the input as a given syntactic construct.
 ///
 /// This covers two main use-cases:
@@ -99,9 +57,11 @@ impl PrefixEntryPoint {
 /// ```
 ///
 /// the input to the macro will be parsed with [`PrefixEntryPoint::Item`], and
-/// the result will be [`TopEntryPoint::Items`].
+/// the result will be [`TopEntryPoint::MacroItems`].
 ///
-/// This *should* (but currently doesn't) guarantee that all input is consumed.
+/// [`TopEntryPoint::parse`] makes a guarantee that
+///   * all input is consumed
+///   * the result is a valid tree (there's one root node)
 #[derive(Debug)]
 pub enum TopEntryPoint {
     SourceFile,
@@ -123,9 +83,67 @@ impl TopEntryPoint {
             TopEntryPoint::MacroItems => grammar::entry::top::macro_items,
             TopEntryPoint::Pattern => grammar::entry::top::pattern,
             TopEntryPoint::Type => grammar::entry::top::type_,
-            // FIXME
-            TopEntryPoint::Expr => grammar::entry::prefix::expr,
-            TopEntryPoint::MetaItem => grammar::entry::prefix::meta_item,
+            TopEntryPoint::Expr => grammar::entry::top::expr,
+            TopEntryPoint::MetaItem => grammar::entry::top::meta_item,
+        };
+        let mut p = parser::Parser::new(input);
+        entry_point(&mut p);
+        let events = p.finish();
+        let res = event::process(events);
+
+        if cfg!(debug_assertions) {
+            let mut depth = 0;
+            let mut first = true;
+            for step in res.iter() {
+                assert!(depth > 0 || first);
+                first = false;
+                match step {
+                    Step::Enter { .. } => depth += 1,
+                    Step::Exit => depth -= 1,
+                    Step::Token { .. } | Step::Error { .. } => (),
+                }
+            }
+            assert!(!first, "no tree at all");
+        }
+
+        res
+    }
+}
+
+/// Parse a prefix of the input as a given syntactic construct.
+///
+/// This is used by macro-by-example parser to implement things like `$i:item`
+/// and the naming of variants follows the naming of macro fragments.
+///
+/// Note that this is generally non-optional -- the result is intentionally not
+/// `Option<Output>`. The way MBE work, by the time we *try* to parse `$e:expr`
+/// we already commit to expression. In other words, this API by design can't be
+/// used to implement "rollback and try another alternative" logic.
+#[derive(Debug)]
+pub enum PrefixEntryPoint {
+    Vis,
+    Block,
+    Stmt,
+    Pat,
+    Ty,
+    Expr,
+    Path,
+    Item,
+    MetaItem,
+}
+
+impl PrefixEntryPoint {
+    pub fn parse(&self, input: &Input) -> Output {
+        let entry_point: fn(&'_ mut parser::Parser) = match self {
+            PrefixEntryPoint::Vis => grammar::entry::prefix::vis,
+            PrefixEntryPoint::Block => grammar::entry::prefix::block,
+            PrefixEntryPoint::Stmt => grammar::entry::prefix::stmt,
+            PrefixEntryPoint::Pat => grammar::entry::prefix::pat,
+            PrefixEntryPoint::Ty => grammar::entry::prefix::ty,
+            PrefixEntryPoint::Expr => grammar::entry::prefix::expr,
+            PrefixEntryPoint::Path => grammar::entry::prefix::path,
+            PrefixEntryPoint::Item => grammar::entry::prefix::item,
+            PrefixEntryPoint::MetaItem => grammar::entry::prefix::meta_item,
         };
         let mut p = parser::Parser::new(input);
         entry_point(&mut p);


Test patch:
diff --git a/crates/hir_def/src/macro_expansion_tests/mbe/tt_conversion.rs b/crates/hir_def/src/macro_expansion_tests/mbe/tt_conversion.rs
index 4c58ea9ba6..5f4b7d6d0b 100644
--- a/crates/hir_def/src/macro_expansion_tests/mbe/tt_conversion.rs
+++ b/crates/hir_def/src/macro_expansion_tests/mbe/tt_conversion.rs
@@ -105,21 +105,21 @@ macro_rules! m2 { ($x:ident) => {} }
 
 #[test]
 fn expansion_does_not_parse_as_expression() {
-    cov_mark::check!(expansion_does_not_parse_as_expression);
     check(
         r#"
 macro_rules! stmts {
     () => { let _ = 0; }
 }
 
-fn f() { let _ = stmts!(); }
+fn f() { let _ = stmts!/*+errors*/(); }
 "#,
         expect![[r#"
 macro_rules! stmts {
     () => { let _ = 0; }
 }
 
-fn f() { let _ = /* error: could not convert tokens */; }
+fn f() { let _ = /* parse error: expected expression */
+let _ = 0;; }
 "#]],
     )
 }
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index 82fc385040..2bfda1aff2 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -1148,7 +1148,7 @@ fn foo() { let a = id!([0u32, bar($0)] ); }
 fn test_hover_through_literal_string_in_macro() {
     check(
         r#"
-macro_rules! arr { ($($tt:tt)*) => { [$($tt)*)] } }
+macro_rules! arr { ($($tt:tt)*) => { [$($tt)*] } }
 fn foo() {
     let mastered_for_itunes = "";
     let _ = arr!("Tr$0acks", &mastered_for_itunes);
diff --git a/crates/parser/src/tests.rs b/crates/parser/src/tests.rs
index c0437bc3b8..cb25abdfea 100644
--- a/crates/parser/src/tests.rs
+++ b/crates/parser/src/tests.rs
@@ -1,6 +1,6 @@
 mod sourcegen_inline_tests;
-mod prefix_entries;
 mod top_entries;
+mod prefix_entries;
 
 use std::{
     fmt::Write,
diff --git a/crates/parser/src/tests/top_entries.rs b/crates/parser/src/tests/top_entries.rs
index dcf61b6aca..24e41b46f8 100644
--- a/crates/parser/src/tests/top_entries.rs
+++ b/crates/parser/src/tests/top_entries.rs
@@ -51,6 +51,13 @@ fn source_file() {
 
 #[test]
 fn macro_stmt() {
+    check(
+        TopEntryPoint::MacroStmts,
+        "",
+        expect![[r#"
+            MACRO_STMTS
+        "#]],
+    );
     check(
         TopEntryPoint::MacroStmts,
         "#!/usr/bin/rust",
@@ -94,6 +101,13 @@ fn macro_stmt() {
 
 #[test]
 fn macro_items() {
+    check(
+        TopEntryPoint::MacroItems,
+        "",
+        expect![[r#"
+            MACRO_ITEMS
+        "#]],
+    );
     check(
         TopEntryPoint::MacroItems,
         "#!/usr/bin/rust",
@@ -131,6 +145,14 @@ fn macro_items() {
 
 #[test]
 fn macro_pattern() {
+    check(
+        TopEntryPoint::Pattern,
+        "",
+        expect![[r#"
+            ERROR
+            error 0: expected pattern
+        "#]],
+    );
     check(
         TopEntryPoint::Pattern,
         "Some(_)",
@@ -177,6 +199,15 @@ fn macro_pattern() {
 
 #[test]
 fn type_() {
+    check(
+        TopEntryPoint::Type,
+        "",
+        expect![[r#"
+            ERROR
+            error 0: expected type
+        "#]],
+    );
+
     check(
         TopEntryPoint::Type,
         "Option<!>",
@@ -224,6 +255,54 @@ fn type_() {
     );
 }
 
+#[test]
+fn expr() {
+    check(
+        TopEntryPoint::Expr,
+        "",
+        expect![[r#"
+            ERROR
+            error 0: expected expression
+        "#]],
+    );
+    check(
+        TopEntryPoint::Expr,
+        "2 + 2 == 5",
+        expect![[r#"
+        BIN_EXPR
+          BIN_EXPR
+            LITERAL
+              INT_NUMBER "2"
+            WHITESPACE " "
+            PLUS "+"
+            WHITESPACE " "
+            LITERAL
+              INT_NUMBER "2"
+          WHITESPACE " "
+          EQ2 "=="
+          WHITESPACE " "
+          LITERAL
+            INT_NUMBER "5"
+    "#]],
+    );
+    check(
+        TopEntryPoint::Expr,
+        "let _ = 0;",
+        expect![[r#"
+        ERROR
+          LET_KW "let"
+          WHITESPACE " "
+          UNDERSCORE "_"
+          WHITESPACE " "
+          EQ "="
+          WHITESPACE " "
+          INT_NUMBER "0"
+          SEMICOLON ";"
+        error 0: expected expression
+    "#]],
+    );
+}
+
 #[track_caller]
 fn check(entry: TopEntryPoint, input: &str, expect: expect_test::Expect) {
     let (parsed, _errors) = super::parse(entry, input);