Better parser error message headers (ExpectedKind enum)

crates/hir/src/lower/parse.rs

@@ -47,7 +47,7 @@ impl DiagnosticVoucher for ParserError {
             self.error.msg(),
             vec![SubDiagnostic::new(
                 LabelStyle::Primary,
-                self.error.msg(),
+                self.error.label(),
                 Some(span),
             )],
             vec![],

crates/hir/src/visitor.rs

@@ -2171,8 +2171,8 @@ mod tests {
     fn visitor() {
         let mut db = TestDb::default();
         let text = r#"
-            #[attr1]
-            #[attr2]
+            #attr1
+            #attr2
             fn foo<T: 'static, V: Add>() {
                 1
                 "foo"
@@ -2197,8 +2197,8 @@ mod tests {
         );
 
         assert_eq!(visitor.attributes.len(), 2);
-        assert_eq!("#[attr1]", db.text_at(top_mod, &visitor.attributes[0]));
-        assert_eq!("#[attr2]", db.text_at(top_mod, &visitor.attributes[1]));
+        assert_eq!("#attr1", db.text_at(top_mod, &visitor.attributes[0]));
+        assert_eq!("#attr2", db.text_at(top_mod, &visitor.attributes[1]));
 
         assert_eq!(visitor.lit_ints.len(), 2);
         assert_eq!("1", db.text_at(top_mod, &visitor.lit_ints[0]));

crates/parser2/src/ast/expr.rs

@@ -913,7 +913,6 @@ mod tests {
 
         let if_expr: IfExpr = parse_expr("if { true } { return } else { continue }");
         if let ExprKind::Block(stmts) = if_expr.cond().unwrap().kind() {
-            dbg!(&stmts);
             assert!(matches!(
                 stmts.into_iter().next().unwrap().kind(),
                 crate::ast::StmtKind::Expr(_)

crates/parser2/src/ast/item.rs

@@ -549,7 +549,6 @@ mod tests {
                 }
             "#;
         let s: Struct = parse_item(source);
-        dbg!(&s);
         assert_eq!(s.name().unwrap().text(), "Foo");
         let mut count = 0;
         for field in s.fields().unwrap() {

crates/parser2/src/lib.rs

@@ -26,25 +26,51 @@ pub fn parse_source_file(text: &str) -> (GreenNode, Vec<ParseError>) {
 /// An parse error which is accumulated in the [`parser::Parser`] while parsing.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum ParseError {
-    Expected(SmallVec<[SyntaxKind; 2]>, Option<String>, TextSize),
-    Unexpected(TextRange),
-
+    Expected(SmallVec<[SyntaxKind; 2]>, ExpectedKind, TextSize),
+    Unexpected(String, TextRange),
     Msg(String, TextRange),
 }
 
 impl ParseError {
-    pub fn expected(tokens: &[SyntaxKind], msg: Option<&str>, pos: TextSize) -> Self {
+    pub fn expected(tokens: &[SyntaxKind], kind: Option<ExpectedKind>, pos: TextSize) -> Self {
         ParseError::Expected(
             SmallVec::from_slice(tokens),
-            msg.map(|s| s.to_string()),
+            kind.unwrap_or(ExpectedKind::Unspecified),
             pos,
         )
     }
 
     pub fn msg(&self) -> String {
         match self {
-            ParseError::Expected(_, Some(msg), _) => msg.clone(),
-            ParseError::Expected(tokens, None, _) => {
+            ParseError::Expected(_, exp, _) => match exp {
+                ExpectedKind::Body(kind) => format!("{} requires a body", kind.describe()),
+                ExpectedKind::Name(kind) => format!("expected name for {}", kind.describe()),
+                ExpectedKind::ClosingBracket { bracket, parent } => format!(
+                    "missing closing {} for {}",
+                    bracket.describe(),
+                    parent.describe()
+                ),
+                ExpectedKind::Separator { separator, element } => {
+                    format!(
+                        "expected {} separator after {}",
+                        separator.describe(),
+                        element.describe()
+                    )
+                }
+                ExpectedKind::TypeSpecifier(kind) => {
+                    format!("missing type bound for {}", kind.describe())
+                }
+                ExpectedKind::Syntax(kind) => format!("expected {}", kind.describe()),
+                ExpectedKind::Unspecified => self.label(),
+            },
+            ParseError::Unexpected(m, _) => m.clone(),
+            ParseError::Msg(m, _) => m.clone(),
+        }
+    }
+
+    pub fn label(&self) -> String {
+        match self {
+            ParseError::Expected(tokens, _, _) => {
                 if tokens.len() == 1 {
                     return format!("expected {}", tokens[0].describe());
                 }
@@ -59,15 +85,35 @@ impl ParseError {
                 }
                 s
             }
-            ParseError::Unexpected(_) => "unexpected syntax".into(),
-            ParseError::Msg(m, _) => m.clone(),
+            ParseError::Unexpected(_, _) => "unexpected".into(),
+            ParseError::Msg(msg, _) => msg.clone(),
         }
     }
 
     pub fn range(&self) -> TextRange {
         match self {
             ParseError::Expected(_, _, pos) => TextRange::empty(*pos),
-            ParseError::Unexpected(r) | ParseError::Msg(_, r) => *r,
+            ParseError::Unexpected(_, r) | ParseError::Msg(_, r) => *r,
         }
     }
 }
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ExpectedKind {
+    Body(SyntaxKind),
+    Name(SyntaxKind),
+    ClosingBracket {
+        bracket: SyntaxKind,
+        parent: SyntaxKind,
+    },
+    TypeSpecifier(SyntaxKind),
+    Separator {
+        separator: SyntaxKind,
+        element: SyntaxKind,
+    },
+    Syntax(SyntaxKind),
+    Unspecified,
+    // TODO:
+    //  - newline after attribute in attrlistscope
+    //
+}

crates/parser2/src/parser/attr.rs

@@ -5,7 +5,7 @@ use super::{
     define_scope, parse_list, token_stream::TokenStream, Checkpoint, ErrProof, Parser, Recovery,
 };
 
-use crate::SyntaxKind;
+use crate::{ExpectedKind, SyntaxKind};
 
 pub(super) fn parse_attr_list<S: TokenStream>(
     parser: &mut Parser<S>,
@@ -40,7 +40,13 @@ impl super::Parse for AttrListScope {
                 _ => break,
             };
             parser.set_newline_as_trivia(false);
-            if parser.find(SyntaxKind::Newline, None)? {
+            if parser.find(
+                SyntaxKind::Newline,
+                ExpectedKind::Separator {
+                    separator: SyntaxKind::Newline,
+                    element: SyntaxKind::Attr,
+                },
+            )? {
                 parser.bump();
             }
         }
@@ -61,10 +67,7 @@ impl super::Parse for AttrScope {
         parser.bump_expected(SyntaxKind::Pound);
 
         parser.set_scope_recovery_stack(&[SyntaxKind::LParen]);
-        if parser.find(
-            SyntaxKind::Ident,
-            Some("expected an attribute name after `#`"),
-        )? {
+        if parser.find(SyntaxKind::Ident, ExpectedKind::Name(SyntaxKind::Attr))? {
             parser.bump()
         }
 
@@ -89,6 +92,7 @@ impl super::Parse for AttrArgListScope {
         parse_list(
             parser,
             false,
+            SyntaxKind::AttrArgList,
             (SyntaxKind::LParen, SyntaxKind::RParen),
             |parser| parser.parse(AttrArgScope::default()),
         )
@@ -104,16 +108,16 @@ impl super::Parse for AttrArgScope {
     type Error = Recovery<ErrProof>;
 
     fn parse<S: TokenStream>(&mut self, parser: &mut Parser<S>) -> Result<(), Self::Error> {
-        let msg = Some("expected attribute argument in the form `key: value`");
+        let expected_err = ExpectedKind::Syntax(SyntaxKind::AttrArg);
 
         parser.set_scope_recovery_stack(&[SyntaxKind::Ident, SyntaxKind::Colon]);
-        if parser.find_and_pop(SyntaxKind::Ident, msg)? {
+        if parser.find_and_pop(SyntaxKind::Ident, expected_err)? {
             parser.bump();
         }
-        if parser.find_and_pop(SyntaxKind::Colon, msg)? {
+        if parser.find_and_pop(SyntaxKind::Colon, expected_err)? {
             parser.bump();
         }
-        if parser.find(SyntaxKind::Ident, msg)? {
+        if parser.find(SyntaxKind::Ident, expected_err)? {
             parser.bump();
         }
         Ok(())

crates/parser2/src/parser/expr.rs

@@ -1,13 +1,14 @@
-use std::convert::Infallible;
+use std::convert::{identity, Infallible};
 use unwrap_infallible::UnwrapInfallible;
 
 use super::{
-    define_scope, expr_atom,
+    define_scope,
+    expr_atom::{self, is_expr_atom_head},
     param::{CallArgListScope, GenericArgListScope},
     token_stream::TokenStream,
     Checkpoint, ErrProof, Parser, Recovery,
 };
-use crate::SyntaxKind;
+use crate::{ExpectedKind, SyntaxKind};
 
 /// Parses expression.
 pub fn parse_expr<S: TokenStream>(parser: &mut Parser<S>) -> Result<(), Recovery<ErrProof>> {
@@ -112,8 +113,10 @@ fn parse_expr_atom<S: TokenStream>(
         Some(kind) if prefix_binding_power(kind).is_some() => {
             parser.parse_cp(UnExprScope::default(), None)
         }
-        Some(_) => expr_atom::parse_expr_atom(parser, allow_struct_init),
-        None => parser
+        Some(kind) if is_expr_atom_head(kind) => {
+            expr_atom::parse_expr_atom(parser, allow_struct_init)
+        }
+        _ => parser
             .error_and_recover("expected expression")
             .map(|_| parser.checkpoint()),
     }
@@ -273,7 +276,10 @@ impl super::Parse for IndexExprScope {
 
         if parser.find(
             SyntaxKind::RBracket,
-            Some("missing closing `]` in index expression"),
+            ExpectedKind::ClosingBracket {
+                bracket: SyntaxKind::RBracket,
+                parent: SyntaxKind::IndexExpr,
+            },
         )? {
             parser.bump();
         }
@@ -293,7 +299,10 @@ impl super::Parse for CallExprScope {
             parser.parse(GenericArgListScope::default())?;
         }
 
-        if parser.find_and_pop(SyntaxKind::LParen, None)? {
+        if parser.find_and_pop(
+            SyntaxKind::LParen,
+            ExpectedKind::Syntax(SyntaxKind::CallArgList),
+        )? {
             parser.parse(CallArgListScope::default())?;
         }
         Ok(())
@@ -309,7 +318,10 @@ impl super::Parse for MethodExprScope {
         parser.set_newline_as_trivia(false);
 
         parser.set_scope_recovery_stack(&[SyntaxKind::Ident, SyntaxKind::Lt, SyntaxKind::LParen]);
-        if parser.find_and_pop(SyntaxKind::Ident, None)? {
+        if parser.find_and_pop(
+            SyntaxKind::Ident,
+            ExpectedKind::Name(SyntaxKind::MethodCallExpr),
+        )? {
             parser.bump();
         }
 
@@ -318,7 +330,10 @@ impl super::Parse for MethodExprScope {
             parser.parse(GenericArgListScope::default())?;
         }
 
-        if parser.find_and_pop(SyntaxKind::LParen, None)? {
+        if parser.find_and_pop(
+            SyntaxKind::LParen,
+            ExpectedKind::Syntax(SyntaxKind::CallArgList),
+        )? {
             parser.parse(CallArgListScope::default())?;
         }
         Ok(())
@@ -472,14 +487,18 @@ fn is_call_expr<S: TokenStream>(parser: &mut Parser<S>) -> bool {
 
         let mut is_call = true;
         if parser.current_kind() == Some(SyntaxKind::Lt) {
-            // xxx `call` error recovery test: "without error" should only apply to base scope
-            is_call &= parser.parses_without_error(GenericArgListScope::default())
+            is_call &= parser
+                .parse_ok(GenericArgListScope::default())
+                .is_ok_and(identity)
         }
 
         if parser.current_kind() != Some(SyntaxKind::LParen) {
             false
         } else {
-            is_call && parser.parses_without_error(CallArgListScope::default())
+            is_call
+                && parser
+                    .parse_ok(CallArgListScope::default())
+                    .is_ok_and(identity)
         }
     })
 }
@@ -496,7 +515,9 @@ fn is_method_call<S: TokenStream>(parser: &mut Parser<S>) -> bool {
         }
 
         if parser.current_kind() == Some(SyntaxKind::Lt)
-            && !parser.parses_without_error(GenericArgListScope::default())
+            && !parser
+                .parse_ok(GenericArgListScope::default())
+                .is_ok_and(identity)
         {
             return false;
         }
@@ -505,7 +526,9 @@ fn is_method_call<S: TokenStream>(parser: &mut Parser<S>) -> bool {
             false
         } else {
             parser.set_newline_as_trivia(is_trivia);
-            parser.parses_without_error(CallArgListScope::default())
+            parser
+                .parse_ok(CallArgListScope::default())
+                .is_ok_and(identity)
         }
     });
     parser.set_newline_as_trivia(is_trivia);