From dec0cca5ad26ef846174cc2641f7c8914f4ddafd Mon Sep 17 00:00:00 2001
From: Brandur <brandur@brandur.org>
Date: Sat, 26 Apr 2025 01:47:44 -0700
Subject: [PATCH] sqlctemplate: Error on template-like artifacts that are
 probably mistakes

While implementing #848 I made a couple typos in SQL queries at a few
points where I made some minor problem in a template tag like putting in
`/* TEMPLATE schema */` instead of `/* TEMPLATE: schema */`.

sqlctemplate isn't a real compiler, so instead of these problems getting
flagged, they're silently ignored, and this is made even worse because
sqlc strips comments from queries so we often end up with absolutely
nothing to debug with. This results in a long, frustrating debugging
experience.

Here, perform a post-replace match to check for obvious problems and
tell the caller about them immediately. It's not perfect, but will save
time in diagnosing common problems.
---
 rivershared/sqlctemplate/sqlc_template.go     | 11 +++++
 .../sqlctemplate/sqlc_template_test.go        | 47 +++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/rivershared/sqlctemplate/sqlc_template.go b/rivershared/sqlctemplate/sqlc_template.go
index e0a02d6a..965ff46d 100644
--- a/rivershared/sqlctemplate/sqlc_template.go
+++ b/rivershared/sqlctemplate/sqlc_template.go
@@ -97,6 +97,12 @@ var (
 	templateRE         = regexp.MustCompile(`/\* TEMPLATE: (.*?) \*/`)
 )
 
+// Regex to search for in SQL after replacement has occurred and which probably
+// represents a syntax error. sqlctemplate isn't a true compiler so if template
+// REs don't match, we can be left with some subtle bugs where there's some
+// minor problem like a missing semicolon that are hard to debug.
+var postReplaceMistakeRE = regexp.MustCompile(`\/\*\s*TEMPLATE([A-Z0-9_]+)?`) // also finds "/* TEMPLATE_BEGIN"
+
 // Run replaces any tempates in input SQL with values from context added via
 // WithReplacements.
 //
@@ -189,6 +195,11 @@ func (r *Replacer) RunSafely(ctx context.Context, sql string, args []any) (strin
 		return "", nil, errors.New("sqlctemplate params present in SQL but missing in context: " + strings.Join(templatesMissing, ", "))
 	}
 
+	probableMistakes := postReplaceMistakeRE.FindAllString(updatedSQL, -1)
+	if len(probableMistakes) > 0 {
+		return "", nil, errors.New("sqlctemplate found template-like tag after replacements; probably syntax error or missing end tag: " + strings.Join(probableMistakes, ", "))
+	}
+
 	if len(container.NamedArgs) > 0 {
 		placeholderNum := len(args)
 
diff --git a/rivershared/sqlctemplate/sqlc_template_test.go b/rivershared/sqlctemplate/sqlc_template_test.go
index 5fd0c612..38213b55 100644
--- a/rivershared/sqlctemplate/sqlc_template_test.go
+++ b/rivershared/sqlctemplate/sqlc_template_test.go
@@ -115,6 +115,53 @@ func TestReplacer(t *testing.T) {
 		`, updatedSQL)
 	})
 
+	t.Run("SyntaxMistakes", func(t *testing.T) {
+		t.Parallel()
+
+		replacer, _ := setup(t)
+
+		// Missing colon after "TEMPLATE".
+		{
+			ctx := WithReplacements(ctx, map[string]Replacement{}, nil)
+
+			_, _, err := replacer.RunSafely(ctx, `
+			-- name: JobCountByState :one
+			SELECT count(*)
+			FROM /* TEMPLATE schema */river_job
+			WHERE state = @state;
+		`, nil)
+			require.EqualError(t, err, "sqlctemplate found template-like tag after replacements; probably syntax error or missing end tag: /* TEMPLATE")
+		}
+
+		// Missing "TEMPLATE_END".
+		{
+			ctx := WithReplacements(ctx, map[string]Replacement{}, nil)
+
+			_, _, err := replacer.RunSafely(ctx, `
+			-- name: JobCountByState :one
+			SELECT count(*)
+			FROM /* TEMPLATE_BEGIN: schema */river_job
+			WHERE state = @state;
+		`, nil)
+			require.EqualError(t, err, "sqlctemplate found template-like tag after replacements; probably syntax error or missing end tag: /* TEMPLATE_BEGIN")
+		}
+
+		// Extra whitespace before "TEMPLATE".
+		{
+			ctx := WithReplacements(ctx, map[string]Replacement{
+				"state": {Value: "'available'"},
+			}, nil)
+
+			_, _, err := replacer.RunSafely(ctx, `
+			-- name: JobCountByState :one
+			SELECT count(*)
+			FROM /*   TEMPLATE schema */river_job
+			WHERE state = /* TEMPLATE_BEGIN: state */ 'available' /* TEMPLATE_END */ -- need to have one valid template to get to the right error
+		`, nil)
+			require.EqualError(t, err, "sqlctemplate found template-like tag after replacements; probably syntax error or missing end tag: /*   TEMPLATE")
+		}
+	})
+
 	t.Run("RepeatedTemplate", func(t *testing.T) {
 		t.Parallel()