From 906c5445b225286652c33ad5907e0de0df84cbfa Mon Sep 17 00:00:00 2001
From: Joseph Kato <joseph@jdkato.io>
Date: Fri, 19 Jul 2024 16:32:26 -0700
Subject: [PATCH] refactor: support capture groups in `substitution`

---
 internal/check/substitution.go      | 27 +++++++++++++--------------
 internal/check/substitution_test.go | 12 ++++++++++++
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/internal/check/substitution.go b/internal/check/substitution.go
index 7f429b52..c5b6bbcf 100644
--- a/internal/check/substitution.go
+++ b/internal/check/substitution.go
@@ -25,6 +25,7 @@ type Substitution struct {
 	Vocab      bool
 	Capitalize bool
 
+	msgMap map[string]string
 	// Deprecated
 	POS string
 }
@@ -67,20 +68,13 @@ func NewSubstitution(cfg *core.Config, generic baseCheck, path string) (Substitu
 
 		opens := strings.Count(regexstr, "(")
 		if opens != strings.Count(regexstr, "(?")+strings.Count(regexstr, `\(`) {
-			// We rely on manually-added capture groups to associate a match
-			// with its replacement -- e.g.,
-			//
-			//    `(foo)|(bar)`, [replacement1, replacement2]
-			//
-			// where the first capture group ("foo") corresponds to the first
-			// element of the replacements slice ("replacement1"). This means
-			// that we can only accept non-capture groups from the user (the
-			// indexing would be mixed up otherwise).
-			//
-			// TODO: Should we change this? Perhaps by creating a map of regex
-			// to replacements?
-			return rule, core.NewE201FromTarget(
-				"capture group not supported; use '(?:' instead of '('", regexstr, path)
+			old := regexstr
+			// We have a capture group, so we need to make it non-capturing.
+			regexstr, err = convertCaptureGroups(regexstr)
+			if err != nil {
+				return rule, core.NewE201FromTarget(err.Error(), regexstr, path)
+			}
+			rule.msgMap[regexstr] = old
 		}
 		tokens += `(` + regexstr + `)|`
 		replacements = append(replacements, replacement)
@@ -181,3 +175,8 @@ func convertMessage(s string) string {
 	}
 	return s
 }
+
+func convertCaptureGroups(msg string) (string, error) {
+	captureOpen := regexp2.MustCompileStd(`(?<!\\)\((?!\?)`)
+	return captureOpen.Replace(msg, "(?:", -1, -1)
+}
diff --git a/internal/check/substitution_test.go b/internal/check/substitution_test.go
index 1231f6ec..9c7ebb53 100644
--- a/internal/check/substitution_test.go
+++ b/internal/check/substitution_test.go
@@ -21,6 +21,18 @@ func makeSubstitution(def baseCheck) (*Substitution, error) {
 	return &rule, nil
 }
 
+func TestConvertGroups(t *testing.T) {
+	converted, err := convertCaptureGroups("change in(?: )?to the (.*) directory")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := "change in(?: )?to the (?:.*) directory"
+	if converted != expected {
+		t.Fatalf("Expected '%s', got '%s'", expected, converted)
+	}
+}
+
 func TestIsDeterministic(t *testing.T) {
 	swap := map[string]interface{}{
 		"extends":    "substitution",