Improve `match` parsing diagnostics (#4934)

Adds diagnostics for missing `case` guard parenthesis.
Adds test for which `lex` parenthesis check succeeds but errors are
reported in `parse` phase.

toolchain/diagnostics/diagnostic_kind.def

@@ -90,7 +90,6 @@ CARBON_DIAGNOSTIC_KIND(ExpectedVarAfterReturned)
 CARBON_DIAGNOSTIC_KIND(ExpectedVariableDecl)
 CARBON_DIAGNOSTIC_KIND(ExpectedChoiceDefinition)
 CARBON_DIAGNOSTIC_KIND(ExpectedChoiceAlternativeName)
-CARBON_DIAGNOSTIC_KIND(UnreachableMatchCase)
 CARBON_DIAGNOSTIC_KIND(OperatorRequiresParentheses)
 CARBON_DIAGNOSTIC_KIND(StatementOperatorAsSubExpr)
 CARBON_DIAGNOSTIC_KIND(UnaryOperatorRequiresParentheses)
@@ -101,12 +100,14 @@ CARBON_DIAGNOSTIC_KIND(UnrecognizedDecl)
 CARBON_DIAGNOSTIC_KIND(UnexpectedTokenInCompoundMemberAccess)
 
 // Match diagnostics.
-CARBON_DIAGNOSTIC_KIND(UnexpectedTokenInMatchCasesBlock)
-CARBON_DIAGNOSTIC_KIND(ExpectedMatchCases)
-CARBON_DIAGNOSTIC_KIND(ExpectedMatchCasesBlock)
 CARBON_DIAGNOSTIC_KIND(ExpectedMatchCaseArrow)
 CARBON_DIAGNOSTIC_KIND(ExpectedMatchCaseBlock)
-CARBON_DIAGNOSTIC_KIND(MatchParseTODO)
+CARBON_DIAGNOSTIC_KIND(ExpectedMatchCaseGuardCloseParen)
+CARBON_DIAGNOSTIC_KIND(ExpectedMatchCaseGuardOpenParen)
+CARBON_DIAGNOSTIC_KIND(ExpectedMatchCases)
+CARBON_DIAGNOSTIC_KIND(ExpectedMatchCasesBlock)
+CARBON_DIAGNOSTIC_KIND(UnexpectedTokenInMatchCasesBlock)
+CARBON_DIAGNOSTIC_KIND(UnreachableMatchCase)
 
 // Package-related diagnostics.
 CARBON_DIAGNOSTIC_KIND(FirstDecl)

toolchain/parse/handle_match.cpp

@@ -7,15 +7,6 @@
 
 namespace Carbon::Parse {
 
-// TODO: Some handling is producing erroneous nodes, but wasn't diagnosing and
-// needed to. A more specific diagnostic could be added, but we should probably
-// have a larger look at this code and how it produces parse errors. This may be
-// good to re-examine when someone is working on implementing match checking.
-static auto DiagnoseMatchParseTODO(Context& context) -> void {
-  CARBON_DIAGNOSTIC(MatchParseTODO, Error, "TODO: improve match parsing");
-  context.emitter().Emit(*context.position(), MatchParseTODO);
-}
-
 static auto HandleStatementsBlockStart(Context& context, State finish,
                                        NodeKind equal_greater, NodeKind starter,
                                        NodeKind complete) -> void {
@@ -171,11 +162,14 @@ auto HandleMatchCaseAfterPattern(Context& context) -> void {
       context.PushState(State::Expr);
     } else {
       if (!state.has_error) {
-        DiagnoseMatchParseTODO(context);
+        CARBON_DIAGNOSTIC(ExpectedMatchCaseGuardOpenParen, Error,
+                          "expected `(` after `if`");
+        context.emitter().Emit(*context.position(),
+                               ExpectedMatchCaseGuardOpenParen);
       }
 
       context.AddLeafNode(NodeKind::MatchCaseGuardStart, *context.position(),
-                          true);
+                          /*has_error=*/true);
       context.AddInvalidParse(*context.position());
       state = context.PopState();
       context.AddNode(NodeKind::MatchCaseGuard, *context.position(),
@@ -199,7 +193,10 @@ auto HandleMatchCaseGuardFinish(Context& context) -> void {
     context.AddNode(NodeKind::MatchCaseGuard, *close_paren, state.has_error);
   } else {
     if (!state.has_error) {
-      DiagnoseMatchParseTODO(context);
+      CARBON_DIAGNOSTIC(ExpectedMatchCaseGuardCloseParen, Error,
+                        "expected `)`");
+      context.emitter().Emit(*context.position(),
+                             ExpectedMatchCaseGuardCloseParen);
     }
 
     context.AddNode(NodeKind::MatchCaseGuard, *context.position(),

toolchain/parse/testdata/match/fail_missing_guard_close_paren.carbon

@@ -14,7 +14,7 @@ fn f() -> i32 {
     // CHECK:STDERR:     case x: bool if (false => { return 1; }
     // CHECK:STDERR:                     ^
     // CHECK:STDERR:
-    // CHECK:STDERR: fail_missing_guard_close_paren.carbon:[[@LINE+4]]:28: error: TODO: improve match parsing [MatchParseTODO]
+    // CHECK:STDERR: fail_missing_guard_close_paren.carbon:[[@LINE+4]]:28: error: expected `)` [ExpectedMatchCaseGuardCloseParen]
     // CHECK:STDERR:     case x: bool if (false => { return 1; }
     // CHECK:STDERR:                            ^~
     // CHECK:STDERR:

toolchain/parse/testdata/match/fail_missing_guard_open_paren.carbon

@@ -10,7 +10,7 @@
 
 fn f() -> i32 {
   match (true) {
-    // CHECK:STDERR: fail_missing_guard_open_paren.carbon:[[@LINE+8]]:21: error: TODO: improve match parsing [MatchParseTODO]
+    // CHECK:STDERR: fail_missing_guard_open_paren.carbon:[[@LINE+8]]:21: error: expected `(` after `if` [ExpectedMatchCaseGuardOpenParen]
     // CHECK:STDERR:     case x: bool if false) => { return 1; }
     // CHECK:STDERR:                     ^~~~~
     // CHECK:STDERR:

toolchain/parse/testdata/match/fail_missing_guard_parens_only_parse_errors.carbon

@@ -0,0 +1,72 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/match/fail_missing_guard_parens_only_parse_errors.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/match/fail_missing_guard_parens_only_parse_errors.carbon
+
+fn f() -> i32 {
+  match (true) {
+    // CHECK:STDERR: fail_missing_guard_parens_only_parse_errors.carbon:[[@LINE+4]]:33: error: expected `)` [ExpectedMatchCaseGuardCloseParen]
+    // CHECK:STDERR:     case x: bool if (x == false => { return 1; }
+    // CHECK:STDERR:                                 ^~
+    // CHECK:STDERR:
+    case x: bool if (x == false => { return 1; }
+    case y: bool if (not y) => { return 2; }
+    // CHECK:STDERR: fail_missing_guard_parens_only_parse_errors.carbon:[[@LINE+4]]:21: error: expected `(` after `if` [ExpectedMatchCaseGuardOpenParen]
+    // CHECK:STDERR:     case z: bool if true or false) => { return 3; }
+    // CHECK:STDERR:                     ^~~~
+    // CHECK:STDERR:
+    case z: bool if true or false) => { return 3; }
+  }
+  return 0;
+}
+
+// CHECK:STDOUT: - filename: fail_missing_guard_parens_only_parse_errors.carbon
+// CHECK:STDOUT:   parse_tree: [
+// CHECK:STDOUT:     {kind: 'FileStart', text: ''},
+// CHECK:STDOUT:         {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:         {kind: 'IdentifierNameBeforeParams', text: 'f'},
+// CHECK:STDOUT:           {kind: 'TuplePatternStart', text: '('},
+// CHECK:STDOUT:         {kind: 'TuplePattern', text: ')', subtree_size: 2},
+// CHECK:STDOUT:           {kind: 'IntTypeLiteral', text: 'i32'},
+// CHECK:STDOUT:         {kind: 'ReturnType', text: '->', subtree_size: 2},
+// CHECK:STDOUT:       {kind: 'FunctionDefinitionStart', text: '{', subtree_size: 7},
+// CHECK:STDOUT:           {kind: 'MatchIntroducer', text: 'match'},
+// CHECK:STDOUT:             {kind: 'MatchConditionStart', text: '('},
+// CHECK:STDOUT:             {kind: 'BoolLiteralTrue', text: 'true'},
+// CHECK:STDOUT:           {kind: 'MatchCondition', text: ')', subtree_size: 3},
+// CHECK:STDOUT:         {kind: 'MatchStatementStart', text: '{', subtree_size: 5},
+// CHECK:STDOUT:             {kind: 'MatchCaseIntroducer', text: 'case'},
+// CHECK:STDOUT:               {kind: 'IdentifierNameNotBeforeParams', text: 'x'},
+// CHECK:STDOUT:               {kind: 'BoolTypeLiteral', text: 'bool'},
+// CHECK:STDOUT:             {kind: 'LetBindingPattern', text: ':', subtree_size: 3},
+// CHECK:STDOUT:               {kind: 'MatchCaseGuardIntroducer', text: 'if'},
+// CHECK:STDOUT:               {kind: 'MatchCaseGuardStart', text: '('},
+// CHECK:STDOUT:                 {kind: 'IdentifierNameExpr', text: 'x'},
+// CHECK:STDOUT:                 {kind: 'BoolLiteralFalse', text: 'false'},
+// CHECK:STDOUT:               {kind: 'InfixOperatorEqualEqual', text: '==', subtree_size: 3},
+// CHECK:STDOUT:             {kind: 'MatchCaseGuard', text: '=>', has_error: yes, subtree_size: 6},
+// CHECK:STDOUT:             {kind: 'MatchCaseEqualGreater', text: 'case', has_error: yes},
+// CHECK:STDOUT:           {kind: 'MatchCaseStart', text: 'case', has_error: yes, subtree_size: 12},
+// CHECK:STDOUT:         {kind: 'MatchCase', text: 'case', has_error: yes, subtree_size: 13},
+// CHECK:STDOUT:             {kind: 'MatchCaseIntroducer', text: 'case'},
+// CHECK:STDOUT:               {kind: 'IdentifierNameNotBeforeParams', text: 'z'},
+// CHECK:STDOUT:               {kind: 'BoolTypeLiteral', text: 'bool'},
+// CHECK:STDOUT:             {kind: 'LetBindingPattern', text: ':', subtree_size: 3},
+// CHECK:STDOUT:               {kind: 'MatchCaseGuardIntroducer', text: 'if'},
+// CHECK:STDOUT:               {kind: 'MatchCaseGuardStart', text: 'true', has_error: yes},
+// CHECK:STDOUT:               {kind: 'InvalidParse', text: 'true', has_error: yes},
+// CHECK:STDOUT:             {kind: 'MatchCaseGuard', text: 'true', has_error: yes, subtree_size: 4},
+// CHECK:STDOUT:           {kind: 'MatchCaseStart', text: 'true', has_error: yes, subtree_size: 9},
+// CHECK:STDOUT:         {kind: 'MatchCase', text: 'true', has_error: yes, subtree_size: 10},
+// CHECK:STDOUT:       {kind: 'MatchStatement', text: '}', subtree_size: 29},
+// CHECK:STDOUT:         {kind: 'ReturnStatementStart', text: 'return'},
+// CHECK:STDOUT:         {kind: 'IntLiteral', text: '0'},
+// CHECK:STDOUT:       {kind: 'ReturnStatement', text: ';', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'FunctionDefinition', text: '}', subtree_size: 40},
+// CHECK:STDOUT:     {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT:   ]