From 36f2c52ff3039fee8fe8cf276222c4a03ff7a4f5 Mon Sep 17 00:00:00 2001
From: Camden Cheek <camden@ccheek.com>
Date: Thu, 9 Jul 2020 09:52:51 -0400
Subject: [PATCH] Add error for regex with no named groups

---
 plugin/builtin/parser/regex.go      | 14 ++++++++++++
 plugin/builtin/parser/regex_test.go | 34 +++++++++++++++++------------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/plugin/builtin/parser/regex.go b/plugin/builtin/parser/regex.go
index 05869a7dc..ebf259d1d 100644
--- a/plugin/builtin/parser/regex.go
+++ b/plugin/builtin/parser/regex.go
@@ -6,6 +6,7 @@ import (
 	"regexp"
 
 	"github.com/observiq/carbon/entry"
+	"github.com/observiq/carbon/errors"
 	"github.com/observiq/carbon/plugin"
 	"github.com/observiq/carbon/plugin/helper"
 )
@@ -37,6 +38,19 @@ func (c RegexParserConfig) Build(context plugin.BuildContext) (plugin.Plugin, er
 		return nil, fmt.Errorf("compiling regex: %s", err)
 	}
 
+	namedCaptureGroups := 0
+	for _, groupName := range r.SubexpNames() {
+		if groupName != "" {
+			namedCaptureGroups++
+		}
+	}
+	if namedCaptureGroups == 0 {
+		return nil, errors.NewError(
+			"no named capture groups in regex pattern",
+			"use named capture groups like '^(?P<my_key>.*)$' to specify the key name for the parsed field",
+		)
+	}
+
 	regexParser := &RegexParser{
 		ParserPlugin: parserPlugin,
 		regexp:       r,
diff --git a/plugin/builtin/parser/regex_test.go b/plugin/builtin/parser/regex_test.go
index 40321dbdf..03bebe095 100644
--- a/plugin/builtin/parser/regex_test.go
+++ b/plugin/builtin/parser/regex_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/observiq/carbon/plugin/helper"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/zap/zaptest"
 )
 
 func newFakeRegexParser() (*RegexParser, *testutil.Plugin) {
@@ -99,36 +98,43 @@ func TestBuildParserRegex(t *testing.T) {
 					},
 				},
 			},
-			Regex: ".*",
+			Regex: "(?P<all>.*)",
 		}
 	}
 
 	t.Run("BasicConfig", func(t *testing.T) {
 		c := newBasicRegexParser()
-		buildContext := plugin.BuildContext{
-			Logger: zaptest.NewLogger(t).Sugar(),
-		}
-		_, err := c.Build(buildContext)
+		_, err := c.Build(testutil.NewBuildContext(t))
 		require.NoError(t, err)
 	})
 
 	t.Run("MissingRegexField", func(t *testing.T) {
 		c := newBasicRegexParser()
 		c.Regex = ""
-		buildContext := plugin.BuildContext{
-			Logger: zaptest.NewLogger(t).Sugar(),
-		}
-		_, err := c.Build(buildContext)
+		_, err := c.Build(testutil.NewBuildContext(t))
 		require.Error(t, err)
 	})
 
 	t.Run("InvalidRegexField", func(t *testing.T) {
 		c := newBasicRegexParser()
 		c.Regex = "())()"
-		buildContext := plugin.BuildContext{
-			Logger: zaptest.NewLogger(t).Sugar(),
-		}
-		_, err := c.Build(buildContext)
+		_, err := c.Build(testutil.NewBuildContext(t))
+		require.Error(t, err)
+	})
+
+	t.Run("NoNamedGroups", func(t *testing.T) {
+		c := newBasicRegexParser()
+		c.Regex = ".*"
+		_, err := c.Build(testutil.NewBuildContext(t))
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "no named capture groups")
+	})
+
+	t.Run("NoNamedGroups", func(t *testing.T) {
+		c := newBasicRegexParser()
+		c.Regex = "(.*)"
+		_, err := c.Build(testutil.NewBuildContext(t))
 		require.Error(t, err)
+		require.Contains(t, err.Error(), "no named capture groups")
 	})
 }