From 06dde215c8d0bde2b2364597190729a160e536a1 Mon Sep 17 00:00:00 2001
From: Vladimir Mihailenco <vladimir.webdev@gmail.com>
Date: Mon, 18 Oct 2021 12:51:50 +0300
Subject: [PATCH] feat: warn when there are args but no placeholders

---
 internal/dbtest/query_test.go                       |  3 +++
 .../dbtest/testdata/snapshots/TestQuery-mysql5-96   |  1 +
 .../dbtest/testdata/snapshots/TestQuery-mysql8-96   |  1 +
 internal/dbtest/testdata/snapshots/TestQuery-pg-96  |  1 +
 internal/dbtest/testdata/snapshots/TestQuery-pgx-96 |  1 +
 .../dbtest/testdata/snapshots/TestQuery-sqlite-96   |  1 +
 query_base.go                                       |  8 +++++---
 query_insert.go                                     |  2 +-
 schema/sqlfmt.go                                    | 13 ++++++++++++-
 9 files changed, 26 insertions(+), 5 deletions(-)
 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql5-96
 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql8-96
 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pg-96
 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pgx-96
 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-sqlite-96

diff --git a/internal/dbtest/query_test.go b/internal/dbtest/query_test.go
index 210ade30d..5010f9cdf 100644
--- a/internal/dbtest/query_test.go
+++ b/internal/dbtest/query_test.go
@@ -594,6 +594,9 @@ func TestQuery(t *testing.T) {
 		func(db *bun.DB) schema.QueryAppender {
 			return db.NewInsert().Model(new(SoftDelete2)).On("CONFLICT DO NOTHING")
 		},
+		func(db *bun.DB) schema.QueryAppender {
+			return db.NewInsert().Model(&Model{}).Returning("")
+		},
 	}
 
 	timeRE := regexp.MustCompile(`'\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+(\+\d{2}:\d{2})?'`)
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-96 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-96
new file mode 100644
index 000000000..19d9c1a4b
--- /dev/null
+++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-96
@@ -0,0 +1 @@
+INSERT INTO `models` (`id`, `str`) VALUES (DEFAULT, '')
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-96 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-96
new file mode 100644
index 000000000..19d9c1a4b
--- /dev/null
+++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-96
@@ -0,0 +1 @@
+INSERT INTO `models` (`id`, `str`) VALUES (DEFAULT, '')
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-96 b/internal/dbtest/testdata/snapshots/TestQuery-pg-96
new file mode 100644
index 000000000..817f77065
--- /dev/null
+++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-96
@@ -0,0 +1 @@
+INSERT INTO "models" ("id", "str") VALUES (DEFAULT, '')
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-96 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-96
new file mode 100644
index 000000000..817f77065
--- /dev/null
+++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-96
@@ -0,0 +1 @@
+INSERT INTO "models" ("id", "str") VALUES (DEFAULT, '')
diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-96 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-96
new file mode 100644
index 000000000..2428f754c
--- /dev/null
+++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-96
@@ -0,0 +1 @@
+INSERT INTO "models" ("str") VALUES ('')
diff --git a/query_base.go b/query_base.go
index 7ed294c05..9739d93e2 100644
--- a/query_base.go
+++ b/query_base.go
@@ -819,9 +819,11 @@ func (q *returningQuery) addReturningField(field *schema.Field) {
 
 func (q *returningQuery) hasReturning() bool {
 	if len(q.returning) == 1 {
-		switch q.returning[0].Query {
-		case "null", "NULL":
-			return false
+		if ret := q.returning[0]; len(ret.Args) == 0 {
+			switch ret.Query {
+			case "", "null", "NULL":
+				return false
+			}
 		}
 	}
 	return len(q.returning) > 0 || len(q.returningFields) > 0
diff --git a/query_insert.go b/query_insert.go
index 4cfdf3cb7..8ef644655 100644
--- a/query_insert.go
+++ b/query_insert.go
@@ -118,7 +118,7 @@ func (q *InsertQuery) WhereOr(query string, args ...interface{}) *InsertQuery {
 
 // Returning adds a RETURNING clause to the query.
 //
-// To suppress the auto-generated RETURNING clause, use `Returning("NULL")`.
+// To suppress the auto-generated RETURNING clause, use `Returning("")`.
 func (q *InsertQuery) Returning(query string, args ...interface{}) *InsertQuery {
 	q.addReturning(schema.SafeQuery(query, args))
 	return q
diff --git a/schema/sqlfmt.go b/schema/sqlfmt.go
index bbdb0a01f..93a801c86 100644
--- a/schema/sqlfmt.go
+++ b/schema/sqlfmt.go
@@ -1,5 +1,11 @@
 package schema
 
+import (
+	"strings"
+
+	"github.com/uptrace/bun/internal"
+)
+
 type QueryAppender interface {
 	AppendQuery(fmter Formatter, b []byte) ([]byte, error)
 }
@@ -42,8 +48,13 @@ var _ QueryAppender = QueryWithArgs{}
 func SafeQuery(query string, args []interface{}) QueryWithArgs {
 	if args == nil {
 		args = make([]interface{}, 0)
+	} else if len(query) > 0 && strings.IndexByte(query, '?') == -1 {
+		internal.Warn.Printf("query %q has args %v, but no placeholders", query, args)
+	}
+	return QueryWithArgs{
+		Query: query,
+		Args:  args,
 	}
-	return QueryWithArgs{Query: query, Args: args}
 }
 
 func UnsafeIdent(ident string) QueryWithArgs {