compares: support Same/NotSame (#96)

* compares: support Same/NotSame

* compares: simplify the code

Author: Antonboom

README.md

@@ -191,6 +191,10 @@ To turn off this behavior use the `--bool-compare.ignore-custom-types` flag.
 **Enabled by default**: true. <br>
 **Reason**: More appropriate `testify` API with clearer failure message.
 
+If `a` and `b` are pointers then `assert.Same`/`NotSame` is required instead,
+due to the inappropriate recursive nature of `assert.Equal` (based on
+[reflect.DeepEqual](https://pkg.go.dev/reflect#DeepEqual)).
+
 ---
 
 ### empty

analyzer/testdata/src/checkers-default/compares/compares_test.go

@@ -11,6 +11,7 @@ import (
 func TestComparesChecker(t *testing.T) {
 	var a, b int
 	var c, d bool
+	var ptrA, ptrB *int
 
 	// Invalid.
 	{
@@ -38,6 +39,14 @@ func TestComparesChecker(t *testing.T) {
 		assert.GreaterOrEqualf(t, a, b, "msg with args %d %s", 42, "42") // want "compares: use assert\\.GreaterOrEqualf"
 		assert.Greater(t, a, b)                                          // want "compares: use assert\\.Greater"
 		assert.Greaterf(t, a, b, "msg with args %d %s", 42, "42")        // want "compares: use assert\\.Greaterf"
+		assert.Same(t, ptrA, ptrB)                                       // want "compares: use assert\\.Same"
+		assert.Samef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")     // want "compares: use assert\\.Samef"
+		assert.NotSame(t, ptrA, ptrB)                                    // want "compares: use assert\\.NotSame"
+		assert.NotSamef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")  // want "compares: use assert\\.NotSamef"
+		assert.NotSame(t, ptrA, ptrB)                                    // want "compares: use assert\\.NotSame"
+		assert.NotSamef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")  // want "compares: use assert\\.NotSamef"
+		assert.Same(t, ptrA, ptrB)                                       // want "compares: use assert\\.Same"
+		assert.Samef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")     // want "compares: use assert\\.Samef"
 	}
 
 	// Valid.
@@ -54,6 +63,10 @@ func TestComparesChecker(t *testing.T) {
 		assert.Lessf(t, a, b, "msg with args %d %s", 42, "42")
 		assert.LessOrEqual(t, a, b)
 		assert.LessOrEqualf(t, a, b, "msg with args %d %s", 42, "42")
+		assert.Same(t, ptrA, ptrB)
+		assert.Samef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")
+		assert.NotSame(t, ptrA, ptrB)
+		assert.NotSamef(t, ptrA, ptrB, "msg with args %d %s", 42, "42")
 	}
 
 	// Ignored.

analyzer/testdata/src/checkers-default/compares/compares_test.go.golden

@@ -0,0 +1,28 @@
+package debug
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSameNotSameReplacements(t *testing.T) {
+	var tm tMock
+
+	a := new(os.PathError)
+	b := new(os.PathError)
+	c := &os.PathError{Path: "/tmp"}
+
+	// Invalid.
+	assert.Equal(t, assert.True(tm, a == b), assert.Equal(tm, a, b))
+	assert.Equal(t, assert.True(tm, a != b), assert.NotEqual(tm, a, b))
+	assert.Equal(t, assert.True(tm, a == c), assert.Equal(tm, a, c))
+	assert.Equal(t, assert.True(tm, a != c), assert.NotEqual(tm, a, c))
+
+	// Valid.
+	assert.Equal(t, assert.True(tm, a == b), assert.Same(tm, a, b))
+	assert.Equal(t, assert.True(tm, a != b), assert.NotSame(tm, a, b))
+	assert.Equal(t, assert.True(tm, a == c), assert.Same(tm, a, c))
+	assert.Equal(t, assert.True(tm, a != c), assert.NotSame(tm, a, c))
+}

analyzer/testdata/src/debug/compares_replacements_test.go

@@ -26,6 +26,9 @@ import (
 //	assert.GreaterOrEqual(t, a, b)
 //	assert.Less(t, a, b)
 //	assert.LessOrEqual(t, a, b)
+//
+// If `a` and `b` are pointers then `assert.Same`/`NotSame` is required instead,
+// due to the inappropriate recursive nature of `assert.Equal` (based on `reflect.DeepEqual`).
 type Compares struct{}
 
 // NewCompares constructs Compares checker.
@@ -53,17 +56,28 @@ func (checker Compares) Check(pass *analysis.Pass, call *CallMeta) *analysis.Dia
 		return nil
 	}
 
-	if proposedFn, ok := tokenToProposedFn[be.Op]; ok {
-		a, b := be.X, be.Y
-		return newUseFunctionDiagnostic(checker.Name(), call, proposedFn,
-			newSuggestedFuncReplacement(call, proposedFn, analysis.TextEdit{
-				Pos:     be.X.Pos(),
-				End:     be.Y.End(),
-				NewText: formatAsCallArgs(pass, a, b),
-			}),
-		)
+	proposedFn, ok := tokenToProposedFn[be.Op]
+	if !ok {
+		return nil
+	}
+
+	if isPointer(pass, be.X) && isPointer(pass, be.Y) {
+		switch proposedFn {
+		case "Equal":
+			proposedFn = "Same"
+		case "NotEqual":
+			proposedFn = "NotSame"
+		}
 	}
-	return nil
+
+	a, b := be.X, be.Y
+	return newUseFunctionDiagnostic(checker.Name(), call, proposedFn,
+		newSuggestedFuncReplacement(call, proposedFn, analysis.TextEdit{
+			Pos:     be.X.Pos(),
+			End:     be.Y.End(),
+			NewText: formatAsCallArgs(pass, a, b),
+		}),
+	)
 }
 
 var tokenToProposedFnInsteadOfTrue = map[token.Token]string{

internal/checkers/compares.go

@@ -54,3 +54,8 @@ func isFloat(pass *analysis.Pass, expr ast.Expr) bool {
 	bt, ok := t.Underlying().(*types.Basic)
 	return ok && (bt.Info()&types.IsFloat > 0)
 }
+
+func isPointer(pass *analysis.Pass, expr ast.Expr) bool {
+	_, ok := pass.TypesInfo.TypeOf(expr).(*types.Pointer)
+	return ok
+}

internal/checkers/helpers_basic_type.go

@@ -51,6 +51,11 @@ func (g ComparesTestsGenerator) TemplateData() any {
 			{Fn: "False", Argsf: "a >= b", ReportMsgf: report, ProposedFn: "Less", ProposedArgsf: "a, b"},
 			{Fn: "False", Argsf: "a < b", ReportMsgf: report, ProposedFn: "GreaterOrEqual", ProposedArgsf: "a, b"},
 			{Fn: "False", Argsf: "a <= b", ReportMsgf: report, ProposedFn: "Greater", ProposedArgsf: "a, b"},
+
+			{Fn: "True", Argsf: "ptrA == ptrB", ReportMsgf: report, ProposedFn: "Same", ProposedArgsf: "ptrA, ptrB"},
+			{Fn: "True", Argsf: "ptrA != ptrB", ReportMsgf: report, ProposedFn: "NotSame", ProposedArgsf: "ptrA, ptrB"},
+			{Fn: "False", Argsf: "ptrA == ptrB", ReportMsgf: report, ProposedFn: "NotSame", ProposedArgsf: "ptrA, ptrB"},
+			{Fn: "False", Argsf: "ptrA != ptrB", ReportMsgf: report, ProposedFn: "Same", ProposedArgsf: "ptrA, ptrB"},
 		},
 		ValidAssertions: []Assertion{
 			{Fn: "Equal", Argsf: "a, b"},
@@ -59,6 +64,9 @@ func (g ComparesTestsGenerator) TemplateData() any {
 			{Fn: "GreaterOrEqual", Argsf: "a, b"},
 			{Fn: "Less", Argsf: "a, b"},
 			{Fn: "LessOrEqual", Argsf: "a, b"},
+
+			{Fn: "Same", Argsf: "ptrA, ptrB"},
+			{Fn: "NotSame", Argsf: "ptrA, ptrB"},
 		},
 		IgnoredAssertions: ignored,
 	}
@@ -89,6 +97,7 @@ import (
 func {{ .CheckerName.AsTestName }}(t *testing.T) {
 	var a, b int
 	var c, d bool
+	var ptrA, ptrB *int
 
 	// Invalid.
 	{