useless-assert: more cases (#182)

* useless-assert: more cases

* useless-assert: add NotZero

CONTRIBUTING.md

@@ -142,7 +142,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
 - [http-sugar](#http-sugar)
 - [require-len](#require-len)
 - [suite-test-name](#suite-test-name)
-- [useless-assert](#useless-assert)
 
 ---
 
@@ -367,26 +366,5 @@ func (s *HandlersSuite) Test_UsecaseSuccess()
 
 ---
 
-### useless-assert
-
-Support more complex cases, e.g.
-
-```go
-body, err := io.ReadAll(rr.Body)
-require.NoError(t, err)
-require.NoError(t, err) ❌
-
-expectedJSON, err := json.Marshal(expected)
-require.NoError(t, err)
-require.JSONEq(t, string(expectedJSON), string(body))
-```
-
-```go
-require.NoError(t, err)
-assert.ErrorContains(t, err, "user") ❌
-```
-
----
-
 Any other figments of your imagination are welcome 🙏<br>
 List of `testify` functions [here](https://pkg.go.dev/github.com/stretchr/testify@master/assert#pkg-functions).

README.md

@@ -880,22 +880,51 @@ a [checkers.AdvancedChecker](https://github.com/Antonboom/testifylint/blob/67632
 
 ### useless-assert
 
-Currently the checker guards against assertion of the same variable:
+The checker guards against assertion of the same variable:
 
 ```go
-❌
+assert.Contains(t, tt.value, tt.value)
+assert.ElementsMatch(t, tt.value, tt.value)
 assert.Equal(t, tt.value, tt.value)
-assert.ElementsMatch(t, users, users)
-// And so on...
+assert.EqualExportedValues(t, tt.value, tt.value)
+// And other assert functions...
+
 assert.True(t, num > num)
+assert.True(t, num < num)
+assert.True(t, num >= num)
+assert.True(t, num <= num)
+assert.True(t, num == num)
+assert.True(t, num != num)
+
+assert.False(t, num > num)
+assert.False(t, num < num)
+assert.False(t, num >= num)
+assert.False(t, num <= num)
 assert.False(t, num == num)
+assert.False(t, num != num)
 ```
 
-More complex cases are [open for contribution](CONTRIBUTING.md#useless-assert).
+And against these meaningless assertions:
+```go
+assert.Empty(t, "")
+assert.False(t, false)
+assert.Implements(t, (*any)(nil), new(Conn))
+assert.Negative(t, -42)
+assert.Nil(t, nil)
+assert.NoError(t, nil)
+assert.NotEmpty(t, "value")
+assert.NotZero(t, 42)
+assert.NotZero(t, "value")
+assert.Positive(t, 42)
+assert.True(t, true)
+assert.Zero(t, 0)
+assert.Zero(t, "")
+assert.Zero(t, nil)
+```
 
 **Autofix**: false. <br>
 **Enabled by default**: true. <br>
-**Reason**: Protection from bugs and possible dead code.
+**Reason**: Protection from bugs and dead code.
 
 ---
 

analyzer/testdata/src/debug/useless_assert_test.go

@@ -0,0 +1,25 @@
+package debug
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUselessAsserts(t *testing.T) {
+	assert.Empty(t, "")
+	assert.False(t, false)
+	assert.Implements(t, (*any)(nil), new(testing.T))
+	assert.Negative(t, -42)
+	assert.Nil(t, nil)
+	assert.NoError(t, nil)
+	assert.NotEmpty(t, "value")
+	assert.NotZero(t, 42)
+	assert.NotZero(t, "value")
+	assert.Zero(t, nil)
+	assert.Positive(t, 42)
+	assert.True(t, true)
+	assert.Zero(t, 0)
+	assert.Zero(t, "")
+	assert.Zero(t, nil)
+}

internal/checkers/compares.go

@@ -61,7 +61,9 @@ func (checker Compares) Check(pass *analysis.Pass, call *CallMeta) *analysis.Dia
 		return nil
 	}
 
-	if isPointer(pass, be.X) && isPointer(pass, be.Y) {
+	_, xp := isPointer(pass, be.X)
+	_, yp := isPointer(pass, be.Y)
+	if xp && yp {
 		switch proposedFn {
 		case "Equal":
 			proposedFn = "Same"

internal/checkers/helpers_basic_type.go

@@ -1,10 +1,10 @@
 package checkers
 
 import (
-	"fmt"
 	"go/ast"
 	"go/token"
 	"go/types"
+	"strconv"
 
 	"golang.org/x/tools/go/analysis"
 )
@@ -56,19 +56,57 @@ func isTypedIntNumber(e ast.Expr, v int, types ...string) bool {
 	return false
 }
 
-func isIntNumber(e ast.Expr, v int) bool {
+func isIntNumber(e ast.Expr, rhs int) bool {
+	lhs, ok := isIntBasicLit(e)
+	return ok && (lhs == rhs)
+}
+
+func isNegativeIntNumber(e ast.Expr) bool {
+	v, ok := isIntBasicLit(e)
+	return ok && v < 0
+}
+
+func isPositiveIntNumber(e ast.Expr) bool {
+	v, ok := isIntBasicLit(e)
+	return ok && v > 0
+}
+
+func isEmptyStringLit(e ast.Expr) bool {
 	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.INT && bl.Value == fmt.Sprintf("%d", v)
+	return ok && bl.Kind == token.STRING && bl.Value == `""`
+}
+
+func isNotEmptyStringLit(e ast.Expr) bool {
+	bl, ok := e.(*ast.BasicLit)
+	return ok && bl.Kind == token.STRING && bl.Value != `""`
 }
 
 func isBasicLit(e ast.Expr) bool {
 	_, ok := e.(*ast.BasicLit)
 	return ok
 }
 
-func isIntBasicLit(e ast.Expr) bool {
+func isIntBasicLit(e ast.Expr) (int, bool) {
+	if un, ok := e.(*ast.UnaryExpr); ok {
+		if un.Op == token.SUB {
+			v, ok := isIntBasicLit(un.X)
+			return -1 * v, ok
+		}
+	}
+
 	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.INT
+	if !ok {
+		return 0, false
+	}
+	if bl.Kind != token.INT {
+		return 0, false
+	}
+
+	v, err := strconv.Atoi(bl.Value)
+	if err != nil {
+		return 0, false
+	}
+	return v, true
 }
 
 func isUntypedConst(pass *analysis.Pass, e ast.Expr) bool {
@@ -98,9 +136,12 @@ func isUnderlying(pass *analysis.Pass, e ast.Expr, flag types.BasicInfo) bool {
 	return ok && (bt.Info()&flag > 0)
 }
 
-func isPointer(pass *analysis.Pass, e ast.Expr) bool {
-	_, ok := pass.TypesInfo.TypeOf(e).(*types.Pointer)
-	return ok
+func isPointer(pass *analysis.Pass, e ast.Expr) (types.Type, bool) {
+	ptr, ok := pass.TypesInfo.TypeOf(e).(*types.Pointer)
+	if !ok {
+		return nil, false
+	}
+	return ptr.Elem(), true
 }
 
 // untype returns v from type(v) expression or v itself if there is no type cast.

internal/checkers/helpers_interface.go

@@ -15,8 +15,11 @@ func isEmptyInterface(pass *analysis.Pass, expr ast.Expr) bool {
 	if !ok {
 		return false
 	}
+	return isEmptyInterfaceType(t.Type)
+}
 
-	iface, ok := t.Type.Underlying().(*types.Interface)
+func isEmptyInterfaceType(t types.Type) bool {
+	iface, ok := t.Underlying().(*types.Interface)
 	return ok && iface.NumMethods() == 0
 }
 

internal/checkers/len.go

@@ -31,7 +31,7 @@ func (checker Len) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnost
 		a, b := call.Args[0], call.Args[1]
 
 		if lenArg, expectedLen, ok := xorLenCall(pass, a, b); ok {
-			if expectedLen == b && !isIntBasicLit(expectedLen) {
+			if _, ok := isIntBasicLit(expectedLen); (expectedLen == b) && !ok {
 				// https://github.com/Antonboom/testifylint/issues/9
 				return nil
 			}
@@ -50,7 +50,10 @@ func (checker Len) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnost
 		}
 		expr := call.Args[0]
 
-		if lenArg, expectedLen, ok := isLenEquality(pass, expr); ok && isIntBasicLit(expectedLen) {
+		if lenArg, expectedLen, ok := isLenEquality(pass, expr); ok {
+			if _, ok := isIntBasicLit(expectedLen); !ok {
+				return nil
+			}
 			return newUseFunctionDiagnostic(checker.Name(), call, proposedFn,
 				newSuggestedFuncReplacement(call, proposedFn, analysis.TextEdit{
 					Pos:     expr.Pos(),

internal/checkers/useless_assert.go

@@ -10,22 +10,99 @@ import (
 
 // UselessAssert detects useless asserts like
 //
-// 1) Asserting of the same variable
-//
+//	assert.Contains(t, tt.value, tt.value)
+//	assert.ElementsMatch(t, tt.value, tt.value)
 //	assert.Equal(t, tt.value, tt.value)
-//	assert.ElementsMatch(t, users, users)
+//	assert.EqualExportedValues(t, tt.value, tt.value)
 //	...
+//
 //	assert.True(t, num > num)
+//	assert.True(t, num < num)
+//	assert.True(t, num >= num)
+//	assert.True(t, num <= num)
+//	assert.True(t, num == num)
+//	assert.True(t, num != num)
+//
+//	assert.False(t, num > num)
+//	assert.False(t, num < num)
+//	assert.False(t, num >= num)
+//	assert.False(t, num <= num)
 //	assert.False(t, num == num)
+//	assert.False(t, num != num)
 //
-// 2) Open for contribution...
+//	assert.Empty(t, "")
+//	assert.False(t, false)
+//	assert.Implements(t, (*any)(nil), new(Conn))
+//	assert.Negative(t, -42)
+//	assert.Nil(t, nil)
+//	assert.NoError(t, nil)
+//	assert.NotEmpty(t, "value")
+//	assert.NotZero(t, 42)
+//	assert.NotZero(t, "value")
+//	assert.Positive(t, 42)
+//	assert.True(t, true)
+//	assert.Zero(t, 0)
+//	assert.Zero(t, "")
+//	assert.Zero(t, nil)
 type UselessAssert struct{}
 
 // NewUselessAssert constructs UselessAssert checker.
 func NewUselessAssert() UselessAssert { return UselessAssert{} }
 func (UselessAssert) Name() string    { return "useless-assert" }
 
 func (checker UselessAssert) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic {
+	if d := checker.checkSameVars(pass, call); d != nil {
+		return d
+	}
+
+	var isMeaningless bool
+	switch call.Fn.NameFTrimmed {
+	case "Empty":
+		isMeaningless = (len(call.Args) >= 1) && isEmptyStringLit(call.Args[0])
+
+	case "False":
+		isMeaningless = (len(call.Args) >= 1) && isUntypedFalse(pass, call.Args[0])
+
+	case "Implements":
+		if len(call.Args) < 2 {
+			return nil
+		}
+
+		elem, ok := isPointer(pass, call.Args[0])
+		isMeaningless = ok && isEmptyInterfaceType(elem)
+
+	case "Negative":
+		isMeaningless = (len(call.Args) >= 1) && isNegativeIntNumber(call.Args[0])
+
+	case "Nil", "NoError":
+		isMeaningless = (len(call.Args) >= 1) && isNil(call.Args[0])
+
+	case "NotEmpty":
+		isMeaningless = (len(call.Args) >= 1) && isNotEmptyStringLit(call.Args[0])
+
+	case "NotZero":
+		isMeaningless = (len(call.Args) >= 1) &&
+			(isNotEmptyStringLit(call.Args[0]) ||
+				isNegativeIntNumber(call.Args[0]) || isPositiveIntNumber(call.Args[0]))
+
+	case "Positive":
+		isMeaningless = (len(call.Args) >= 1) && isPositiveIntNumber(call.Args[0])
+
+	case "True":
+		isMeaningless = (len(call.Args) >= 1) && isUntypedTrue(pass, call.Args[0])
+
+	case "Zero":
+		isMeaningless = (len(call.Args) >= 1) &&
+			(isZero(call.Args[0]) || isEmptyStringLit(call.Args[0]) || isNil(call.Args[0]))
+	}
+
+	if isMeaningless {
+		return newDiagnostic(checker.Name(), call, "meaningless assertion", nil)
+	}
+	return nil
+}
+
+func (checker UselessAssert) checkSameVars(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic {
 	var first, second ast.Node
 
 	switch call.Fn.NameFTrimmed {