empty: support empty strings, Zero for strings and len + bubbled useless-assert cases (#129)

* empty: detect more cases

Related to #119

* empty: finalize new cases

* empty: finalize implementation

* useless-assert: support cases moved from empty

* review fixes

* useless-assert: more cases

---------

Co-authored-by: Anton Telyshev <[email protected]>

Author: ccoVeille

README.md

@@ -246,35 +246,60 @@ assert.NotContains(t, a, "abc123")
 ```go
 ❌
 assert.Len(t, arr, 0)
+assert.Zero(t, str)
+assert.Zero(t, len(arr))
 assert.Equal(t, 0, len(arr))
 assert.EqualValues(t, 0, len(arr))
 assert.Exactly(t, 0, len(arr))
 assert.LessOrEqual(t, len(arr), 0)
 assert.GreaterOrEqual(t, 0, len(arr))
-assert.Less(t, len(arr), 0)
-assert.Greater(t, 0, len(arr))
 assert.Less(t, len(arr), 1)
 assert.Greater(t, 1, len(arr))
-assert.Zero(t, len(arr))
-assert.Empty(t, len(arr))
+assert.Equal(t, "", str)
+assert.EqualValues(t, "", str)
+assert.Exactly(t, "", str)
+assert.Equal(t, ``, str)
+assert.EqualValues(t, ``, str)
+assert.Exactly(t, ``, str)
 
+assert.Positive(t, len(arr))
+assert.NotZero(t, str)
+assert.NotZero(t, len(arr))
 assert.NotEqual(t, 0, len(arr))
 assert.NotEqualValues(t, 0, len(arr))
-assert.Less(t, 0, len(arr))
 assert.Greater(t, len(arr), 0)
-assert.Positive(t, len(arr))
-assert.NotZero(t, len(arr))
-assert.NotEmpty(t, len(arr))
+assert.Less(t, 0, len(arr))
+assert.NotEqual(t, "", str)
+assert.NotEqualValues(t, "", str)
+assert.NotEqual(t, ``, str)
+assert.NotEqualValues(t, ``, str)
 
 ✅
 assert.Empty(t, arr)
-assert.NotEmpty(t, err)
+assert.NotEmpty(t, arr)
 ```
 
 **Autofix**: true. <br>
 **Enabled by default**: true. <br>
 **Reason**: More appropriate `testify` API with clearer failure message.
 
+Also `empty` removes extra `len` call in `*Emtpy` assertions:
+
+```go
+❌
+assert.Empty(t, len(arr))
+assert.NotEmpty(t, len(arr))
+
+✅
+assert.Empty(t, arr)
+assert.NotEmpty(t, arr)
+```
+
+P.S. `empty` does not remove the string conversion and keeps as is:
+
+- `string(bytes)` – to keep assert failure message readability;
+- `string(str)` – in favor of [unconvert](https://golangci-lint.run/usage/linters/#unconvert).
+
 ---
 
 ### encoded-compare
@@ -1074,25 +1099,40 @@ assert.False(t, num != num)
 And against these meaningless assertions:
 
 ```go
-assert.Empty(t, "")
-assert.False(t, false)
+assert.Empty(t, "value") // Any string literal.
+assert.Error(t, nil)
+assert.False(t, false) // Any bool literal.
 assert.Implements(t, (*any)(nil), new(Conn))
-assert.Negative(t, -42)
+assert.Negative(t, 42) // Any int literal.
 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.NotEmpty(t, "value") // Any string literal.
+assert.NotImplements(t, (*any)(nil), new(Conn))
+assert.NotNil(t, nil)
+assert.NotZero(t, 42)      // Any int literal.
+assert.NotZero(t, "value") // Any string literal.
+assert.NotZero(t, nil)
+assert.NotZero(t, false) // Any bool literal.
+assert.Positive(t, 42)   // Any int literal.
+assert.True(t, true)     // Any bool literal.
+assert.Zero(t, 42)       // Any int literal.
+assert.Zero(t, "value")  // Any string literal.
 assert.Zero(t, nil)
-
-assert.GreaterOrEqual(t, uintVal, 0)
-assert.LessOrEqual(t, 0, uintVal)
-assert.GreaterOrEqual(t, len(x), 0)
-assert.LessOrEqual(t, 0, len(x))
+assert.Zero(t, false) // Any bool literal.
+
+assert.Negative(len(x))
+assert.Less(len(x), 0)
+assert.Greater(0, len(x))
+assert.Positive(len(x))
+assert.GreaterOrEqual(len(x), 0)
+assert.LessOrEqual(0, len(x))
+
+assert.Negative(uintVal)
+assert.Less(uintVal, 0)
+assert.Greater(0, uintVal)
+assert.Positive(uintVal)
+assert.GreaterOrEqual(uintVal, 0)
+assert.LessOrEqual(0, uintVal)
 ```
 
 **Autofix**: false. <br>

analyzer/testdata/src/checkers-default/empty/empty_test.go

@@ -43,7 +43,7 @@ func (checker BoolCompare) Check(pass *analysis.Pass, call *CallMeta) *analysis.
 	}
 
 	newUseFnDiagnostic := func(proposed string, survivingArg ast.Expr, replaceStart, replaceEnd token.Pos) *analysis.Diagnostic {
-		if !isBuiltinBool(pass, survivingArg) {
+		if !hasBoolType(pass, survivingArg) {
 			if checker.ignoreCustomTypes {
 				return nil
 			}
@@ -65,7 +65,7 @@ func (checker BoolCompare) Check(pass *analysis.Pass, call *CallMeta) *analysis.
 	}
 
 	newNeedSimplifyDiagnostic := func(survivingArg ast.Expr, replaceStart, replaceEnd token.Pos) *analysis.Diagnostic {
-		if !isBuiltinBool(pass, survivingArg) {
+		if !hasBoolType(pass, survivingArg) {
 			if checker.ignoreCustomTypes {
 				return nil
 			}
@@ -103,15 +103,15 @@ func (checker BoolCompare) Check(pass *analysis.Pass, call *CallMeta) *analysis.
 		switch {
 		case xor(t1, t2):
 			survivingArg, _ := anyVal([]bool{t1, t2}, arg2, arg1)
-			if call.Fn.NameFTrimmed == "Exactly" && !isBuiltinBool(pass, survivingArg) {
+			if call.Fn.NameFTrimmed == "Exactly" && !hasBoolType(pass, survivingArg) {
 				// NOTE(a.telyshev): `Exactly` assumes no type conversion.
 				return nil
 			}
 			return newUseTrueDiagnostic(survivingArg, arg1.Pos(), arg2.End())
 
 		case xor(f1, f2):
 			survivingArg, _ := anyVal([]bool{f1, f2}, arg2, arg1)
-			if call.Fn.NameFTrimmed == "Exactly" && !isBuiltinBool(pass, survivingArg) {
+			if call.Fn.NameFTrimmed == "Exactly" && !hasBoolType(pass, survivingArg) {
 				// NOTE(a.telyshev): `Exactly` assumes no type conversion.
 				return nil
 			}

analyzer/testdata/src/checkers-default/empty/empty_test.go.golden

@@ -11,31 +11,41 @@ import (
 
 // Empty detects situations like
 //
-//	assert.Len(t, arr, 0)
-//	assert.Equal(t, 0, len(arr))
-//	assert.EqualValues(t, 0, len(arr))
-//	assert.Exactly(t, 0, len(arr))
-//	assert.LessOrEqual(t, len(arr), 0)
-//	assert.GreaterOrEqual(t, 0, len(arr))
-//	assert.Less(t, len(arr), 0)
-//	assert.Greater(t, 0, len(arr))
-//	assert.Less(t, len(arr), 1)
-//	assert.Greater(t, 1, len(arr))
-//	assert.Zero(t, len(arr))
-//	assert.Empty(t, len(arr))
+// assert.Len(t, arr, 0)
+// assert.Zero(t, str)
+// assert.Zero(t, len(arr))
+// assert.Equal(t, 0, len(arr))
+// assert.EqualValues(t, 0, len(arr))
+// assert.Exactly(t, 0, len(arr))
+// assert.LessOrEqual(t, len(arr), 0)
+// assert.GreaterOrEqual(t, 0, len(arr))
+// assert.Less(t, len(arr), 1)
+// assert.Greater(t, 1, len(arr))
+// assert.Equal(t, "", str)
+// assert.EqualValues(t, "", str)
+// assert.Exactly(t, "", str)
+// assert.Equal(t, “, str)
+// assert.EqualValues(t, “, str)
+// assert.Exactly(t, “, str)
 //
-//	assert.NotEqual(t, 0, len(arr))
-//	assert.NotEqualValues(t, 0, len(arr))
-//	assert.Less(t, 0, len(arr))
-//	assert.Greater(t, len(arr), 0)
-//	assert.Positive(t, len(arr))
-//	assert.NotZero(t, len(arr))
-//	assert.NotEmpty(t, len(arr))
+// assert.Positive(t, len(arr))
+// assert.NotZero(t, str)
+// assert.NotZero(t, len(arr))
+// assert.NotEqual(t, 0, len(arr))
+// assert.NotEqualValues(t, 0, len(arr))
+// assert.Greater(t, len(arr), 0)
+// assert.Less(t, 0, len(arr))
+// assert.NotEqual(t, "", str)
+// assert.NotEqualValues(t, "", str)
+// assert.NotEqual(t, “, str)
+// assert.NotEqualValues(t, “, str)
 //
 // and requires
 //
 //	assert.Empty(t, arr)
 //	assert.NotEmpty(t, arr)
+//
+// Also Empty removes extra `len` call.
 type Empty struct{}
 
 // NewEmpty constructs Empty checker.
@@ -67,6 +77,9 @@ func (checker Empty) checkEmpty(pass *analysis.Pass, call *CallMeta) *analysis.D
 
 	switch call.Fn.NameFTrimmed {
 	case "Zero":
+		if hasStringType(pass, a) {
+			return newUseEmptyDiagnostic(a.Pos(), a.End(), a)
+		}
 		if lenArg, ok := isBuiltinLenCall(pass, a); ok {
 			return newUseEmptyDiagnostic(a.Pos(), a.End(), lenArg)
 		}
@@ -89,6 +102,10 @@ func (checker Empty) checkEmpty(pass *analysis.Pass, call *CallMeta) *analysis.D
 		}
 
 	case "Equal", "EqualValues", "Exactly":
+		if isEmptyStringLit(a) {
+			return newUseEmptyDiagnostic(a.Pos(), b.End(), b)
+		}
+
 		arg1, ok1 := isLenCallAndZero(pass, a, b)
 		arg2, ok2 := isLenCallAndZero(pass, b, a)
 
@@ -136,7 +153,15 @@ func (checker Empty) checkNotEmpty(pass *analysis.Pass, call *CallMeta) *analysi
 	a := call.Args[0]
 
 	switch call.Fn.NameFTrimmed {
-	case "NotZero", "Positive":
+	case "Positive":
+		if lenArg, ok := isBuiltinLenCall(pass, a); ok {
+			return newUseNotEmptyDiagnostic(a.Pos(), a.End(), lenArg)
+		}
+
+	case "NotZero":
+		if hasStringType(pass, a) {
+			return newUseNotEmptyDiagnostic(a.Pos(), a.End(), a)
+		}
 		if lenArg, ok := isBuiltinLenCall(pass, a); ok {
 			return newUseNotEmptyDiagnostic(a.Pos(), a.End(), lenArg)
 		}
@@ -154,6 +179,10 @@ func (checker Empty) checkNotEmpty(pass *analysis.Pass, call *CallMeta) *analysi
 
 	switch call.Fn.NameFTrimmed {
 	case "NotEqual", "NotEqualValues":
+		if isEmptyStringLit(a) {
+			return newUseNotEmptyDiagnostic(a.Pos(), b.End(), b)
+		}
+
 		arg1, ok1 := isLenCallAndZero(pass, a, b)
 		arg2, ok2 := isLenCallAndZero(pass, b, a)
 

analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go

@@ -61,24 +61,14 @@ func isIntNumber(e ast.Expr, rhs int) bool {
 	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 {
+func isStringLit(e ast.Expr) bool {
 	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.STRING && bl.Value == `""`
+	return ok && bl.Kind == token.STRING
 }
 
-func isNotEmptyStringLit(e ast.Expr) bool {
+func isEmptyStringLit(e ast.Expr) bool {
 	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.STRING && bl.Value != `""`
+	return ok && bl.Kind == token.STRING && (bl.Value == `""` || bl.Value == "``")
 }
 
 func isBasicLit(e ast.Expr) bool {

internal/checkers/bool_compare.go

@@ -14,6 +14,10 @@ var (
 	trueObj  = types.Universe.Lookup("true")
 )
 
+func isUntypedBool(pass *analysis.Pass, e ast.Expr) bool {
+	return isUntypedTrue(pass, e) || isUntypedFalse(pass, e)
+}
+
 func isUntypedTrue(pass *analysis.Pass, e ast.Expr) bool {
 	return analysisutil.IsObj(pass.TypesInfo, e, trueObj)
 }
@@ -22,7 +26,7 @@ func isUntypedFalse(pass *analysis.Pass, e ast.Expr) bool {
 	return analysisutil.IsObj(pass.TypesInfo, e, falseObj)
 }
 
-func isBuiltinBool(pass *analysis.Pass, e ast.Expr) bool {
+func hasBoolType(pass *analysis.Pass, e ast.Expr) bool {
 	basicType, ok := pass.TypesInfo.TypeOf(e).(*types.Basic)
 	return ok && basicType.Kind() == types.Bool
 }