Fail parsing checks if var def regexp is bad

e.g. bad pattern [[FOO:?]]
The question mark is a quantifier that doesn't quantify anything.

BUG=137632977

2 files changed, 17 insertions, 1 deletions

diff --git a/effcee/check.cc b/effcee/check.cc
index 08f7b7a..c078eff 100644
--- a/effcee/check.cc
+++ b/effcee/check.cc
@@ -188,6 +188,14 @@ std::pair<Result, Check::Parts> PartsForPattern(StringPiece pattern) {
           StringPiece expression = var.substr(colon + 1, StringPiece::npos);
           parts.emplace_back(
               make_unique<Check::Part>(Type::VarDef, var, name, expression));
+          if (parts.back()->NumCapturingGroups() < 0) {
+            return std::make_pair(
+                Result(
+                    Result::Status::BadRule,
+                    std::string("invalid regex in variable definition for ") +
+                        ToString(name) + ": " + ToString(expression)),
+                Check::Parts());
+          }
         }
       }
     } else {
diff --git a/effcee/check_test.cc b/effcee/check_test.cc
index f7089ef..d5c118a 100644
--- a/effcee/check_test.cc
+++ b/effcee/check_test.cc
@@ -246,13 +246,21 @@ INSTANTIATE_TEST_SUITE_P(AllCheckTypes, ParseChecksTypeFailTest,
                                                                    "FOO"}),
                                  ValuesIn(AllCheckTypesAsPairs())));
 
-TEST(ParseChecks, BadRegexpFails) {
+TEST(ParseChecks, BadRegexpMatchTrailingSlashFails) {
   const auto parsed = ParseChecks("CHECK: {{\\}}", Options());
   EXPECT_THAT(parsed.first.status(), Eq(Status::BadRule));
   EXPECT_THAT(parsed.first.message(), HasSubstr("invalid regex: \\"));
   EXPECT_THAT(parsed.second, Eq(CheckList({})));
 }
 
+TEST(ParseChecks, BadRegexpVardefUnboundOptionalFails) {
+  const auto parsed = ParseChecks("CHECK: [[VAR:?]]", Options());
+  EXPECT_THAT(parsed.first.status(), Eq(Status::BadRule));
+  EXPECT_THAT(parsed.first.message(),
+              HasSubstr("invalid regex in variable definition for VAR: ?"));
+  EXPECT_THAT(parsed.second, Eq(CheckList({})));
+}
+
 TEST(ParseChecks, CheckSameCantBeFirst) {
   const auto parsed = ParseChecks("CHECK-SAME: now", Options());
   EXPECT_THAT(parsed.first.status(), Eq(Status::BadRule));