formatter: warn on empty message

README.md

@@ -595,14 +595,17 @@ in `v2` of `testify`.
 #### 4)
 
 Validating the first argument of `msgAndArgs` has `string` type (based on `testify`'s
-maintainer's [feedback](https://github.com/stretchr/testify/issues/1679#issuecomment-2480629257)):
+maintainer's [feedback](https://github.com/stretchr/testify/issues/1679#issuecomment-2480629257)) and this string is
+not empty:
 
 ```go
 ❌
 assert.Equal(t, 1, strings.Count(b.String(), "hello"), tc)
+assert.Equalf(t, want, got, "")
 
 ✅
 assert.Equal(t, 1, strings.Count(b.String(), "hello"), "%+v", tc)
+assert.Equal(t, want, got)
 ```
 
 You can disable this behaviour with the `--formatter.require-string-msg=false` flag.

analyzer/testdata/src/checkers-default/formatter/formatter_test.go.golden

@@ -124,6 +124,25 @@ func TestFormatterChecker_PrintfChecks(t *testing.T) {
 	assert.Equalf(t, 1, 2, "msg with args %s %s", "42") // want "formatter: assert\\.Equalf format %s reads arg #2, but call has 1 arg$"
 }
 
+func TestFormatterChecker_EmptyMessage(t *testing.T) {
+	var want, got any
+	assertObj := assert.New(t)
+	assert.Equal(t, want, got)            // want "formatter: empty message"
+	assertObj.Equal(want, got)            // want "formatter: empty message"
+	assert.Equal(t, want, got)            // want "formatter: empty message"
+	assertObj.Equal(want, got)            // want "formatter: empty message"
+	assert.Equal(t, want, got, "", 1, 2)  // want "formatter: empty message"
+	assertObj.Equal(want, got, "", 1, 2)  // want "formatter: empty message"
+	assert.Equalf(t, want, got, "", 1, 2) // want "formatter: empty message"
+	assertObj.Equalf(want, got, "", 1, 2) // want "formatter: empty message"
+	assert.Equal(t, want, got, "boom!")
+	assertObj.Equal(want, got, "boom!")
+	assert.Equal(t, want, got, "%v != %v", 1, 2)
+	assertObj.Equal(want, got, "%v != %v", 1, 2)
+	assert.Equalf(t, want, got, "%v != %v", 1, 2)
+	assertObj.Equalf(want, got, "%v != %v", 1, 2)
+}
+
 type FormatterCheckerSuite struct {
 	suite.Suite
 }

analyzer/testdata/src/formatter-not-defaults/formatter_not_defaults_test.go.golden

@@ -42,6 +42,25 @@ func TestFormatterChecker(t *testing.T) {
 	assert.Equalf(t, 1, 2, fmt.Sprintf("msg with arg %d", 42), "42")
 }
 
+func TestFormatterChecker_EmptyMessage(t *testing.T) {
+	var want, got any
+	assertObj := assert.New(t)
+	assert.Equal(t, want, got)                    // want "formatter: empty message"
+	assertObj.Equal(want, got)                    // want "formatter: empty message"
+	assert.Equal(t, want, got)                    // want "formatter: empty message"
+	assertObj.Equal(want, got)                    // want "formatter: empty message"
+	assert.Equal(t, want, got, "", 1, 2)          // want "formatter: empty message"
+	assertObj.Equal(want, got, "", 1, 2)          // want "formatter: empty message"
+	assert.Equalf(t, want, got, "", 1, 2)         // want "formatter: empty message"
+	assertObj.Equalf(want, got, "", 1, 2)         // want "formatter: empty message"
+	assert.Equalf(t, want, got, "boom!")          // want "formatter: use assert\\.Equalf"
+	assertObj.Equalf(want, got, "boom!")          // want "formatter: use assertObj\\.Equalf"
+	assert.Equalf(t, want, got, "%v != %v", 1, 2) // want "formatter: use assert\\.Equalf"
+	assertObj.Equalf(want, got, "%v != %v", 1, 2) // want "formatter: use assertObj\\.Equalf"
+	assert.Equalf(t, want, got, "%v != %v", 1, 2)
+	assertObj.Equalf(want, got, "%v != %v", 1, 2)
+}
+
 func TestFormatterChecker_AllAssertions(t *testing.T) {
 	assert.Condition(t, nil)
 	assert.Conditionf(t, nil, "msg") // want "formatter: use assert\\.Conditionf"

internal/checkers/formatter.go

@@ -1,6 +1,7 @@
 package checkers
 
 import (
+	"fmt"
 	"go/types"
 	"strconv"
 	"strings"
@@ -28,7 +29,7 @@ import (
 //	assert.Truef(t, targetTs.Equal(ts), "the timestamp should be as expected (%s) but was %s", targetTs, ts)
 //
 // It also checks that there are no arguments in `msgAndArgs` if the message is not a string,
-// and additionally checks that the first argument of `msgAndArgs` is a string.
+// and additionally checks that the first argument of `msgAndArgs` is a not empty string.
 //
 // Finally, it checks that failure message in Fail and FailNow is not used as a format string (which won't work).
 type Formatter struct {
@@ -100,6 +101,17 @@ func (checker Formatter) checkNotFmtAssertion(pass *analysis.Pass, call *CallMet
 	}
 
 	if hasStringType(pass, msgAndArgs) { //nolint:nestif // This is the best option of code organization :(
+		format, err := strconv.Unquote(analysisutil.NodeString(pass.Fset, msgAndArgs))
+		if nil == err && format == "" { // Unquote failed for not string literals.
+			var fixes []analysis.SuggestedFix
+			if isSingleMsgAndArgElem {
+				fixes = append(fixes, analysis.SuggestedFix{
+					Message:   "Remove empty message",
+					TextEdits: []analysis.TextEdit{newRemoveLastArgTextEdit(pass, call.Args)},
+				})
+			}
+			return newDiagnostic(checker.Name(), call, "empty message", fixes...)
+		}
 		if checker.requireFFuncs {
 			return newUseFunctionDiagnostic(checker.Name(), call, call.Fn.Name+"f")
 		}
@@ -135,25 +147,40 @@ func (checker Formatter) checkFmtAssertion(pass *analysis.Pass, call *CallMeta)
 
 	lastArgPos := len(call.ArgsRaw) - 1
 	msg := call.ArgsRaw[formatPos]
+	noFormatArgs := formatPos == lastArgPos
 
 	if formatPos == lastArgPos {
 		if args, ok := isFmtSprintfCall(pass, msg); ok {
 			return newRemoveSprintfDiagnostic(pass, checker.Name(), call, msg, args)
 		}
 	}
 
+	format, err := strconv.Unquote(analysisutil.NodeString(pass.Fset, msg))
+	if err != nil {
+		// Unreachable, because msg is a string literal in formatted assertion.
+		return nil
+	}
+	if format == "" {
+		var fixes []analysis.SuggestedFix
+		if noFormatArgs {
+			fixes = append(fixes, analysis.SuggestedFix{
+				Message: fmt.Sprintf("Remove empty message and use `%s`", call.Fn.NameFTrimmed),
+				TextEdits: []analysis.TextEdit{
+					newReplaceFnTextEdit(call.Fn, call.Fn.NameFTrimmed),
+					newRemoveLastArgTextEdit(pass, call.Args),
+				},
+			})
+		}
+		return newDiagnostic(checker.Name(), call, "empty message", fixes...)
+	}
+
 	if checker.checkFormatString {
 		report := pass.Report
 		defer func() { pass.Report = report }()
 
 		pass.Report = func(d analysis.Diagnostic) {
 			result = newDiagnostic(checker.Name(), call, d.Message)
 		}
-
-		format, err := strconv.Unquote(analysisutil.NodeString(pass.Fset, msg))
-		if err != nil {
-			return nil
-		}
 		printf.CheckPrintf(pass, call.Call, call.String(), format, formatPos)
 	}
 	return result

internal/checkers/helpers_diagnostic.go

@@ -131,13 +131,23 @@ func newSuggestedFuncReplacement(
 		proposedFn += "f"
 	}
 	return analysis.SuggestedFix{
-		Message: fmt.Sprintf("Replace `%s` with `%s`", call.Fn.Name, proposedFn),
-		TextEdits: append([]analysis.TextEdit{
-			{
-				Pos:     call.Fn.Pos(),
-				End:     call.Fn.End(),
-				NewText: []byte(proposedFn),
-			},
-		}, additionalEdits...),
+		Message:   fmt.Sprintf("Replace `%s` with `%s`", call.Fn.Name, proposedFn),
+		TextEdits: append([]analysis.TextEdit{newReplaceFnTextEdit(call.Fn, proposedFn)}, additionalEdits...),
+	}
+}
+
+func newReplaceFnTextEdit(callFn analysis.Range, proposedFn string) analysis.TextEdit {
+	return analysis.TextEdit{
+		Pos:     callFn.Pos(),
+		End:     callFn.End(),
+		NewText: []byte(proposedFn),
+	}
+}
+
+func newRemoveLastArgTextEdit(pass *analysis.Pass, callArgs []ast.Expr) analysis.TextEdit {
+	return analysis.TextEdit{
+		Pos:     callArgs[0].Pos(),
+		End:     callArgs[len(callArgs)-1].End(),
+		NewText: formatAsCallArgs(pass, callArgs[0:len(callArgs)-1]...),
 	}
 }

internal/testgen/gen_formatter.go

@@ -24,6 +24,8 @@ func (g FormatterTestsGenerator) TemplateData() any {
 			": using msgAndArgs with non-string first element (msg) causes panic"
 		reportFailureMsgIsNotFmtString = checker +
 			": failure message is not a format string, use msgAndArgs instead"
+		reportEmptyMessage = checker +
+			": empty message"
 	)
 
 	baseAssertion := Assertion{Fn: "Equal", Argsf: "1, 2"}
@@ -171,6 +173,29 @@ func (g FormatterTestsGenerator) TemplateData() any {
 		},
 	}
 
+	emptyMsgAssertions := []Assertion{
+		{
+			Fn: "Equal", Argsf: `want, got, ""`,
+			ReportMsgf: reportEmptyMessage, ProposedArgsf: "want, got",
+		},
+		{
+			Fn: "Equalf", Argsf: `want, got, ""`,
+			ReportMsgf: reportEmptyMessage + "%.s%.s", ProposedFn: "Equal", ProposedArgsf: "want, got",
+		},
+		{
+			Fn: "Equal", Argsf: `want, got, "", 1, 2`,
+			ReportMsgf: reportEmptyMessage,
+		},
+		{
+			Fn: "Equalf", Argsf: `want, got, "", 1, 2`,
+			ReportMsgf: reportEmptyMessage,
+		},
+
+		{Fn: "Equal", Argsf: `want, got, "boom!"`},
+		{Fn: "Equal", Argsf: `want, got, "%v != %v", 1, 2`},
+		{Fn: "Equalf", Argsf: `want, got, "%v != %v", 1, 2`},
+	}
+
 	assertions := make([]Assertion, 0, len(allAssertions)*5)
 	for _, a := range allAssertions {
 		assertions = append(assertions,
@@ -204,13 +229,15 @@ func (g FormatterTestsGenerator) TemplateData() any {
 		BaseAssertion          Assertion
 		NonStringMsgAssertions []Assertion
 		SprintfAssertions      []Assertion
+		EmptyMsgAssertions     []Assertion
 		AllAssertions          []Assertion
 		IgnoredAssertions      []Assertion
 	}{
 		CheckerName:            CheckerName(g.Checker().Name()),
 		BaseAssertion:          baseAssertion,
 		NonStringMsgAssertions: nonStringMsgAssertions,
 		SprintfAssertions:      sprintfAssertions,
+		EmptyMsgAssertions:     emptyMsgAssertions,
 		AllAssertions:          assertions,
 		IgnoredAssertions: []Assertion{
 			{Fn: "ObjectsAreEqual", Argsf: "nil, nil"},
@@ -304,6 +331,16 @@ func {{ .CheckerName.AsTestName }}_PrintfChecks(t *testing.T) {
 	assert.Equalf(t, 1, 2, "msg with args %s %s", "42") // want "formatter: assert\\.Equalf format %s reads arg #2, but call has 1 arg$"
 }
 
+func {{ .CheckerName.AsTestName }}_EmptyMessage(t *testing.T) {
+	var want, got any
+	assertObj := assert.New(t)
+
+	{{- range $ai, $assrn := $.EmptyMsgAssertions }}
+		{{ NewAssertionExpander.NotFmtSingleMode.Expand $assrn "assert" "t" nil }}
+		{{ NewAssertionExpander.NotFmtSingleMode.Expand $assrn "assertObj" "" nil }}
+	{{- end }}
+}
+
 {{ $suiteName := .CheckerName.AsSuiteName }}
 
 type {{ $suiteName }} struct {

internal/testgen/gen_formatter_not_defaults.go

@@ -19,6 +19,8 @@ func (g FormatterNotDefaultsTestsGenerator) TemplateData() any {
 			": using msgAndArgs with non-string first element (msg) causes panic"
 		reportFailureMsgIsNotFmtString = checker +
 			": failure message is not a format string, use msgAndArgs instead"
+		reportEmptyMessage = checker +
+			": empty message"
 	)
 
 	baseAssertions := []Assertion{
@@ -130,6 +132,29 @@ func (g FormatterNotDefaultsTestsGenerator) TemplateData() any {
 		},
 	}
 
+	emptyMsgAssertions := []Assertion{
+		{
+			Fn: "Equal", Argsf: `want, got, ""`,
+			ReportMsgf: reportEmptyMessage, ProposedArgsf: "want, got",
+		},
+		{
+			Fn: "Equalf", Argsf: `want, got, ""`,
+			ReportMsgf: reportEmptyMessage + "%.s%.s", ProposedFn: "Equal", ProposedArgsf: "want, got",
+		},
+		{
+			Fn: "Equal", Argsf: `want, got, "", 1, 2`,
+			ReportMsgf: reportEmptyMessage,
+		},
+		{
+			Fn: "Equalf", Argsf: `want, got, "", 1, 2`,
+			ReportMsgf: reportEmptyMessage,
+		},
+
+		{Fn: "Equal", Argsf: `want, got, "boom!"`, ReportMsgf: reportUse, ProposedFn: "Equalf"},
+		{Fn: "Equal", Argsf: `want, got, "%v != %v", 1, 2`, ReportMsgf: reportUse, ProposedFn: "Equalf"},
+		{Fn: "Equalf", Argsf: `want, got, "%v != %v", 1, 2`},
+	}
+
 	assertions := make([]Assertion, 0, len(allAssertions)*5)
 	for _, a := range allAssertions {
 		assertions = append(assertions,
@@ -159,15 +184,17 @@ func (g FormatterNotDefaultsTestsGenerator) TemplateData() any {
 	}
 
 	return struct {
-		CheckerName       CheckerName
-		BaseAssertions    []Assertion
-		SprintfAssertions []Assertion
-		AllAssertions     []Assertion
+		CheckerName        CheckerName
+		BaseAssertions     []Assertion
+		SprintfAssertions  []Assertion
+		EmptyMsgAssertions []Assertion
+		AllAssertions      []Assertion
 	}{
-		CheckerName:       CheckerName(checker),
-		BaseAssertions:    baseAssertions,
-		SprintfAssertions: sprintfAssertions,
-		AllAssertions:     assertions,
+		CheckerName:        CheckerName(checker),
+		BaseAssertions:     baseAssertions,
+		SprintfAssertions:  sprintfAssertions,
+		EmptyMsgAssertions: emptyMsgAssertions,
+		AllAssertions:      assertions,
 	}
 }
 
@@ -210,6 +237,16 @@ func {{ .CheckerName.AsTestName }}(t *testing.T) {
 	{{- end }}
 }
 
+func {{ .CheckerName.AsTestName }}_EmptyMessage(t *testing.T) {
+	var want, got any
+	assertObj := assert.New(t)
+
+	{{- range $ai, $assrn := $.EmptyMsgAssertions }}
+		{{ NewAssertionExpander.NotFmtSingleMode.Expand $assrn "assert" "t" nil }}
+		{{ NewAssertionExpander.NotFmtSingleMode.Expand $assrn "assertObj" "" nil }}
+	{{- end }}
+}
+
 func {{ .CheckerName.AsTestName }}_AllAssertions(t *testing.T) {
 	{{- range $ai, $assrn := $.AllAssertions }}
 		{{ NewAssertionExpander.NotFmtSingleMode.Expand $assrn "assert" "t" nil }}