fix: unterminated up statement is ignored (pressly#558)

CHANGELOG.md

@@ -7,7 +7,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
-- Fix pre-built binary versioning and make small improvements to the build process.
+- Fix pre-built binary versioning and make small improvements to GoReleaser config.
+- Fix an edge case in the `sqlparser` where the last up statement may be ignored if it's
+  unterminated and followed by a `-- +goose Down` annotation.
 
 ## [v3.13.1] - 2023-07-03
 

internal/sqlparser/parser.go

@@ -127,6 +127,12 @@ func ParseSQLMigration(r io.Reader, direction Direction, debug bool) (stmts []st
 			case "+goose Down":
 				switch stateMachine.get() {
 				case gooseUp, gooseStatementEndUp:
+					// If we hit a down annotation, but the buffer is not empty, we have an unfinished SQL query from a
+					// previous up annotation. This is an error, because we expect the SQL query to be terminated by a semicolon
+					// and the buffer to have been reset.
+					if bufferRemaining := strings.TrimSpace(buf.String()); len(bufferRemaining) > 0 {
+						return nil, false, missingSemicolonError(stateMachine.state, direction, bufferRemaining)
+					}
 					stateMachine.set(gooseDown)
 				default:
 					return nil, false, fmt.Errorf("must start with '-- +goose Up' annotation, stateMachine=%d, see https://github.com/pressly/goose#sql-migrations", stateMachine.state)
@@ -238,12 +244,20 @@ func ParseSQLMigration(r io.Reader, direction Direction, debug bool) (stmts []st
 	}
 
 	if bufferRemaining := strings.TrimSpace(buf.String()); len(bufferRemaining) > 0 {
-		return nil, false, fmt.Errorf("failed to parse migration: state %d, direction: %v: unexpected unfinished SQL query: %q: missing semicolon?", stateMachine.state, direction, bufferRemaining)
+		return nil, false, missingSemicolonError(stateMachine.state, direction, bufferRemaining)
 	}
 
 	return stmts, useTx, nil
 }
 
+func missingSemicolonError(state parserState, direction Direction, s string) error {
+	return fmt.Errorf("failed to parse migration: state %d, direction: %v: unexpected unfinished SQL query: %q: missing semicolon?",
+		state,
+		direction,
+		s,
+	)
+}
+
 // cleanupStatement attempts to find the last semicolon and trims
 // the remaining chars from the input string. This is useful for cleaning
 // up a statement containing trailing comments or empty lines.

internal/sqlparser/parser_test.go

@@ -86,6 +86,22 @@ func TestSplitStatements(t *testing.T) {
 	}
 }
 
+func TestInvalidUp(t *testing.T) {
+	t.Parallel()
+
+	testdataDir := filepath.Join("testdata", "invalid", "up")
+	entries, err := os.ReadDir(testdataDir)
+	check.NoError(t, err)
+	check.NumberNotZero(t, len(entries))
+
+	for _, entry := range entries {
+		by, err := os.ReadFile(filepath.Join(testdataDir, entry.Name()))
+		check.NoError(t, err)
+		_, _, err = ParseSQLMigration(strings.NewReader(string(by)), DirectionUp, false)
+		check.HasError(t, err)
+	}
+}
+
 func TestUseTransactions(t *testing.T) {
 	t.Parallel()
 

internal/sqlparser/testdata/invalid/up/a.sql

@@ -0,0 +1,5 @@
+-- +goose Up
+SELECT * FROM foo;
+SELECT * FROM bar
+-- +goose Down
+SELECT * FROM baz;

internal/sqlparser/testdata/invalid/up/b.sql

@@ -0,0 +1,4 @@
+-- +goose Up
+SELECT * FROM bar
+-- +goose Down
+SELECT * FROM baz;

internal/sqlparser/testdata/invalid/up/c.sql

@@ -0,0 +1,3 @@
+-- +goose Up
+SELECT * FROM bar
+-- +goose Down

internal/sqlparser/testdata/invalid/up/d.sql

@@ -0,0 +1,2 @@
+-- +goose Up
+SELECT * FROM bar