negative-positive: support typed 0 (#126)

* negative-positive: support typed 0

Fixes #94

* negative-positive: support signed zeros only

* negative-positive: refactor debug tests

* negative-positive: support typed 0

Fixes #94

* negative-positive: support signed zeros only

* negative-positive: refactor debug tests

* negative-positive: support Positive for uints

---------

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

CONTRIBUTING.md

@@ -30,7 +30,7 @@ type TimeCompare struct{}
 
 // NewTimeCompare constructs TimeCompare checker.
 func NewTimeCompare() TimeCompare { return TimeCompare{} }
-func (TimeCompare) Name() string  { return "TimeCompare" }
+func (TimeCompare) Name() string  { return "time-compare" }
 ```
 
 The above code is enough to satisfy the `checkers.Checker` interface.
@@ -121,6 +121,12 @@ Install...
 
 Fix linter issues and broken tests (probably related to the checkers registry).
 
+To run checker default tests you can use `task test:checker -- {checker-name}`, e.g.
+
+```bash
+$ task test:checker -- time-compare
+```
+
 ### 10) Update `README.md`, commit the changes and submit a pull request 🔥
 
 Describe a new checker in [checkers section](./README.md#checkers).

README.md

@@ -589,6 +589,8 @@ assert.Positive(t, a)
 **Enabled by default**: true <br>
 **Reason**: More appropriate `testify` API with clearer failure message.
 
+Typed zeros (like `int8(0)`, ..., `uint64(0)`) are also supported.
+
 ---
 
 ### nil-compare

Taskfile.yml

@@ -46,6 +46,12 @@ tasks:
       - echo "Test..."
       - go test ./...
 
+  # task test:checker -- negative-positive
+  test:checker:
+    deps: [ test:gen ]
+    cmds:
+      - go test -count 1 -run TestTestifyLint_CheckersDefault/{{.CLI_ARGS}} ./analyzer
+
   test:coverage:
     env:
       GOEXPERIMENT: nocoverageredesign # https://github.com/golang/go/issues/65653#issuecomment-1955872134

analyzer/testdata/src/debug/negative_positive_replacements_test.go

@@ -10,7 +10,7 @@ import (
 func TestNegativeReplacements(t *testing.T) {
 	var tm tMock
 
-	negativeAssertions := []func(v int) bool{
+	negativeAssertionsInt := []func(v int) bool{
 		func(v int) bool { return assert.Less(tm, v, 0) },
 		func(v int) bool { return assert.Greater(tm, 0, v) },
 		func(v int) bool { return assert.True(tm, v < 0) },
@@ -19,8 +19,8 @@ func TestNegativeReplacements(t *testing.T) {
 		func(v int) bool { return assert.False(tm, 0 <= v) },
 	}
 
-	t.Run("assert.Negative", func(t *testing.T) {
-		for i, original := range negativeAssertions {
+	t.Run("int", func(t *testing.T) {
+		for i, original := range negativeAssertionsInt {
 			for _, v := range []int{-1, 0, 1} {
 				t.Run(fmt.Sprintf("%d_%d", i, v), func(t *testing.T) {
 					replacement := assert.Negative(tm, v)
@@ -29,12 +29,32 @@ func TestNegativeReplacements(t *testing.T) {
 			}
 		}
 	})
+
+	negativeAssertionsUint := []func(v uint8) bool{
+		func(v uint8) bool { return assert.Less(tm, v, uint8(0)) },
+		func(v uint8) bool { return assert.Greater(tm, uint8(0), v) },
+		func(v uint8) bool { return assert.True(tm, v < uint8(0)) },
+		func(v uint8) bool { return assert.True(tm, uint8(0) > v) },
+		func(v uint8) bool { return assert.False(tm, v >= uint8(0)) },
+		func(v uint8) bool { return assert.False(tm, uint8(0) <= v) },
+	}
+
+	t.Run("uint", func(t *testing.T) {
+		for i, original := range negativeAssertionsUint {
+			for _, v := range []uint8{ /* -1, */ 0, 1} { // constant -1 overflows uint8
+				t.Run(fmt.Sprintf("%d_%d", i, v), func(t *testing.T) {
+					replacement := assert.Negative(tm, v)
+					assert.Equal(t, original(v), replacement, "not an equivalent replacement")
+				})
+			}
+		}
+	})
 }
 
 func TestPositiveReplacements(t *testing.T) {
 	var tm tMock
 
-	positiveAssertions := []func(v int) bool{
+	positiveAssertionsInt := []func(v int) bool{
 		func(v int) bool { return assert.Greater(tm, v, 0) },
 		func(v int) bool { return assert.Less(tm, 0, v) },
 		func(v int) bool { return assert.True(tm, v > 0) },
@@ -43,8 +63,8 @@ func TestPositiveReplacements(t *testing.T) {
 		func(v int) bool { return assert.False(tm, 0 >= v) },
 	}
 
-	t.Run("assert.Positive", func(t *testing.T) {
-		for i, original := range positiveAssertions {
+	t.Run("int", func(t *testing.T) {
+		for i, original := range positiveAssertionsInt {
 			for _, v := range []int{-1, 0, 1} {
 				t.Run(fmt.Sprintf("%d_%d", i, v), func(t *testing.T) {
 					replacement := assert.Positive(tm, v)
@@ -53,4 +73,24 @@ func TestPositiveReplacements(t *testing.T) {
 			}
 		}
 	})
+
+	positiveAssertionsUint := []func(v uint8) bool{
+		func(v uint8) bool { return assert.Greater(tm, v, uint8(0)) },
+		func(v uint8) bool { return assert.Less(tm, uint8(0), v) },
+		func(v uint8) bool { return assert.True(tm, v > uint8(0)) },
+		func(v uint8) bool { return assert.True(tm, uint8(0) < v) },
+		func(v uint8) bool { return assert.False(tm, v <= uint8(0)) },
+		func(v uint8) bool { return assert.False(tm, uint8(0) >= v) },
+	}
+
+	t.Run("uint", func(t *testing.T) {
+		for i, original := range positiveAssertionsUint {
+			for _, v := range []uint8{ /* -1, */ 0, 1} { // constant -1 overflows uint8
+				t.Run(fmt.Sprintf("%d_%d", i, v), func(t *testing.T) {
+					replacement := assert.Positive(tm, v)
+					assert.Equal(t, original(v), replacement, "not an equivalent replacement")
+				})
+			}
+		}
+	})
 }

internal/checkers/helpers_basic_type.go

@@ -11,10 +11,51 @@ import (
 
 func isZero(e ast.Expr) bool { return isIntNumber(e, 0) }
 
-func isNotZero(e ast.Expr) bool { return !isZero(e) }
-
 func isOne(e ast.Expr) bool { return isIntNumber(e, 1) }
 
+func isAnyZero(e ast.Expr) bool {
+	return isIntNumber(e, 0) || isTypedSignedIntNumber(e, 0) || isTypedUnsignedIntNumber(e, 0)
+}
+
+func isNotAnyZero(e ast.Expr) bool {
+	return !isAnyZero(e)
+}
+
+func isZeroOrSignedZero(e ast.Expr) bool {
+	return isIntNumber(e, 0) || isTypedSignedIntNumber(e, 0)
+}
+
+func isSignedNotZero(pass *analysis.Pass, e ast.Expr) bool {
+	return !isUnsigned(pass, e) && !isZeroOrSignedZero(e)
+}
+
+func isTypedSignedIntNumber(e ast.Expr, v int) bool {
+	return isTypedIntNumber(e, v, "int", "int8", "int16", "int32", "int64")
+}
+
+func isTypedUnsignedIntNumber(e ast.Expr, v int) bool {
+	return isTypedIntNumber(e, v, "uint", "uint8", "uint16", "uint32", "uint64")
+}
+
+func isTypedIntNumber(e ast.Expr, v int, types ...string) bool {
+	ce, ok := e.(*ast.CallExpr)
+	if !ok || len(ce.Args) != 1 {
+		return false
+	}
+
+	fn, ok := ce.Fun.(*ast.Ident)
+	if !ok {
+		return false
+	}
+
+	for _, t := range types {
+		if fn.Name == t {
+			return isIntNumber(ce.Args[0], v)
+		}
+	}
+	return false
+}
+
 func isIntNumber(e ast.Expr, v int) bool {
 	bl, ok := e.(*ast.BasicLit)
 	return ok && bl.Kind == token.INT && bl.Value == fmt.Sprintf("%d", v)
@@ -30,32 +71,43 @@ func isIntBasicLit(e ast.Expr) bool {
 	return ok && bl.Kind == token.INT
 }
 
-func isUntypedConst(p *analysis.Pass, e ast.Expr) bool {
-	t := p.TypesInfo.TypeOf(e)
-	if t == nil {
-		return false
-	}
-
-	b, ok := t.(*types.Basic)
-	return ok && b.Info()&types.IsUntyped > 0
+func isUntypedConst(pass *analysis.Pass, e ast.Expr) bool {
+	return isUnderlying(pass, e, types.IsUntyped)
 }
 
-func isTypedConst(p *analysis.Pass, e ast.Expr) bool {
-	tt, ok := p.TypesInfo.Types[e]
+func isTypedConst(pass *analysis.Pass, e ast.Expr) bool {
+	tt, ok := pass.TypesInfo.Types[e]
 	return ok && tt.IsValue() && tt.Value != nil
 }
 
-func isFloat(pass *analysis.Pass, expr ast.Expr) bool {
-	t := pass.TypesInfo.TypeOf(expr)
+func isFloat(pass *analysis.Pass, e ast.Expr) bool {
+	return isUnderlying(pass, e, types.IsFloat)
+}
+
+func isUnsigned(pass *analysis.Pass, e ast.Expr) bool {
+	return isUnderlying(pass, e, types.IsUnsigned)
+}
+
+func isUnderlying(pass *analysis.Pass, e ast.Expr, flag types.BasicInfo) bool {
+	t := pass.TypesInfo.TypeOf(e)
 	if t == nil {
 		return false
 	}
 
 	bt, ok := t.Underlying().(*types.Basic)
-	return ok && (bt.Info()&types.IsFloat > 0)
+	return ok && (bt.Info()&flag > 0)
 }
 
-func isPointer(pass *analysis.Pass, expr ast.Expr) bool {
-	_, ok := pass.TypesInfo.TypeOf(expr).(*types.Pointer)
+func isPointer(pass *analysis.Pass, e ast.Expr) bool {
+	_, ok := pass.TypesInfo.TypeOf(e).(*types.Pointer)
 	return ok
 }
+
+// untype returns v from type(v) expression or v itself if there is no type cast.
+func untype(e ast.Expr) ast.Expr {
+	ce, ok := e.(*ast.CallExpr)
+	if !ok || len(ce.Args) != 1 {
+		return e
+	}
+	return ce.Args[0]
+}

internal/checkers/negative_postive.go

@@ -29,6 +29,8 @@ import (
 //
 //	assert.Negative(t, value)
 //	assert.Positive(t, value)
+//
+// Typed zeros (like `int8(0)`, ..., `uint64(0)`) are also supported.
 type NegativePositive struct{}
 
 // NewNegativePositive constructs NegativePositive checker.
@@ -54,15 +56,17 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
 		)
 	}
 
+	// NOTE(a.telyshev): We ignore uint-asserts as being no sense for assert.Negative.
+
 	switch call.Fn.NameFTrimmed {
 	case "Less":
 		if len(call.Args) < 2 {
 			return nil
 		}
 		a, b := call.Args[0], call.Args[1]
 
-		if isNotZero(a) && isZero(b) {
-			return newUseNegativeDiagnostic(a.Pos(), b.End(), a)
+		if isSignedNotZero(pass, a) && isZeroOrSignedZero(b) {
+			return newUseNegativeDiagnostic(a.Pos(), b.End(), untype(a))
 		}
 
 	case "Greater":
@@ -71,8 +75,8 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
 		}
 		a, b := call.Args[0], call.Args[1]
 
-		if isZero(a) && isNotZero(b) {
-			return newUseNegativeDiagnostic(a.Pos(), b.End(), b)
+		if isZeroOrSignedZero(a) && isSignedNotZero(pass, b) {
+			return newUseNegativeDiagnostic(a.Pos(), b.End(), untype(b))
 		}
 
 	case "True":
@@ -81,12 +85,12 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
 		}
 		expr := call.Args[0]
 
-		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZero), token.LSS, p(isZero)) // a < 0
-		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZero), token.GTR, p(isNotZero)) // 0 > a
+		a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.LSS, p(isZeroOrSignedZero)) // a < 0
+		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroOrSignedZero), token.GTR, isSignedNotZero) // 0 > a
 
 		survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
 		if ok {
-			return newUseNegativeDiagnostic(expr.Pos(), expr.End(), survivingArg)
+			return newUseNegativeDiagnostic(expr.Pos(), expr.End(), untype(survivingArg))
 		}
 
 	case "False":
@@ -95,12 +99,12 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
 		}
 		expr := call.Args[0]
 
-		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZero), token.GEQ, p(isZero)) // a >= 0
-		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZero), token.LEQ, p(isNotZero)) // 0 <= a
+		a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.GEQ, p(isZeroOrSignedZero)) // a >= 0
+		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroOrSignedZero), token.LEQ, isSignedNotZero) // 0 <= a
 
 		survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
 		if ok {
-			return newUseNegativeDiagnostic(expr.Pos(), expr.End(), survivingArg)
+			return newUseNegativeDiagnostic(expr.Pos(), expr.End(), untype(survivingArg))
 		}
 	}
 	return nil
@@ -125,8 +129,8 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
 		}
 		a, b := call.Args[0], call.Args[1]
 
-		if isNotZero(a) && isZero(b) {
-			return newUsePositiveDiagnostic(a.Pos(), b.End(), a)
+		if isNotAnyZero(a) && isAnyZero(b) {
+			return newUsePositiveDiagnostic(a.Pos(), b.End(), untype(a))
 		}
 
 	case "Less":
@@ -135,8 +139,8 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
 		}
 		a, b := call.Args[0], call.Args[1]
 
-		if isZero(a) && isNotZero(b) {
-			return newUsePositiveDiagnostic(a.Pos(), b.End(), b)
+		if isAnyZero(a) && isNotAnyZero(b) {
+			return newUsePositiveDiagnostic(a.Pos(), b.End(), untype(b))
 		}
 
 	case "True":
@@ -145,12 +149,12 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
 		}
 		expr := call.Args[0]
 
-		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZero), token.GTR, p(isZero)) // a > 0
-		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZero), token.LSS, p(isNotZero)) // 0 < a
+		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotAnyZero), token.GTR, p(isAnyZero)) // a > 0
+		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isAnyZero), token.LSS, p(isNotAnyZero)) // 0 < a
 
 		survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
 		if ok {
-			return newUsePositiveDiagnostic(expr.Pos(), expr.End(), survivingArg)
+			return newUsePositiveDiagnostic(expr.Pos(), expr.End(), untype(survivingArg))
 		}
 
 	case "False":
@@ -159,12 +163,12 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
 		}
 		expr := call.Args[0]
 
-		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZero), token.LEQ, p(isZero)) // a <= 0
-		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZero), token.GEQ, p(isNotZero)) // 0 >= a
+		a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotAnyZero), token.LEQ, p(isAnyZero)) // a <= 0
+		_, b, ok2 := isStrictComparisonWith(pass, expr, p(isAnyZero), token.GEQ, p(isNotAnyZero)) // 0 >= a
 
 		survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
 		if ok {
-			return newUsePositiveDiagnostic(expr.Pos(), expr.End(), survivingArg)
+			return newUsePositiveDiagnostic(expr.Pos(), expr.End(), untype(survivingArg))
 		}
 	}
 	return nil