Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

negative-positive: support typed 0 #126

Merged
merged 8 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
negative-positive: support signed zeros only
  • Loading branch information
Antonboom authored and ccoVeille committed Jun 19, 2024
commit e03ca9420b1aaa8aa150d66247f9157958954897
8 changes: 7 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ assert.Positive(t, a)
**Enabled by default**: true <br>
**Reason**: More appropriate `testify` API with clearer failure message.

Typed zeros (like `int(0)`, `int(8)`, … `uint(64)`) are also supported.
Typed signed zeros (like `int(0)`, `int8(0)`, ..., `int64(0)`) are also supported.

---

Expand Down
5 changes: 5 additions & 0 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ tasks:
- echo "Generate analyzer tests..."
- go run ./internal/testgen

# task test:checker -- negative-positive
test:checker:
cmds:
- go test -count 1 -run TestTestifyLint_CheckersDefault/{{.CLI_ARGS}} ./analyzer

profile:
cmds:
- echo "Do profiling..."
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ func TestNegativeReplacements(t *testing.T) {
func(v int) bool { return assert.True(tm, 0 > v) },
func(v int) bool { return assert.False(tm, v >= 0) },
func(v int) bool { return assert.False(tm, 0 <= v) },

func(v int) bool { return assert.Less(tm, int64(v), int64(0)) },
func(v int) bool { return assert.Greater(tm, uint8(0), uint8(v)) },
}

t.Run("assert.Negative", func(t *testing.T) {
Expand All @@ -41,6 +44,9 @@ func TestPositiveReplacements(t *testing.T) {
func(v int) bool { return assert.True(tm, 0 < v) },
func(v int) bool { return assert.False(tm, v <= 0) },
func(v int) bool { return assert.False(tm, 0 >= v) },

func(v int) bool { return assert.Greater(tm, int8(v), int8(0)) },
func(v int) bool { return assert.Less(tm, uint64(0), uint64(v)) },
}

t.Run("assert.Positive", func(t *testing.T) {
Expand Down
44 changes: 23 additions & 21 deletions internal/checkers/helpers_basic_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ func isZero(e ast.Expr) bool { return isIntNumber(e, 0) }

func isOne(e ast.Expr) bool { return isIntNumber(e, 1) }

func isZeroValue(e ast.Expr) bool {
return isZero(e) || isTypedIntNumber(e, 0)
func isSignedZero(e ast.Expr) bool {
return isIntNumber(e, 0) || isTypedSignedIntNumber(e, 0)
}

func isNotZeroValue(e ast.Expr) bool {
return !isZeroValue(e)
func isSignedNotZero(pass *analysis.Pass, e ast.Expr) bool {
return !isUnsigned(pass, e) && !isSignedZero(e)
}

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)
}

func isTypedIntNumber(e ast.Expr, v int) bool {
func isTypedSignedIntNumber(e ast.Expr, v int) bool {
ce, ok := e.(*ast.CallExpr)
if !ok || len(ce.Args) != 1 {
return false
Expand All @@ -38,7 +38,7 @@ func isTypedIntNumber(e ast.Expr, v int) bool {
}

switch fn.Name {
case "int", "int8", "int16", "int32", "int64", "uint", "uint8", "uint16", "uint32", "uint64":
case "int", "int8", "int16", "int32", "int64":
return isIntNumber(ce.Args[0], v)
}

Expand All @@ -55,32 +55,34 @@ 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
}
28 changes: 14 additions & 14 deletions internal/checkers/negative_postive.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
// assert.False(t, a <= 0)
// assert.False(t, 0 >= a)
//
// Typed zeros (like `int(0)`, `int(8)`, … `uint(64)`) are also supported.
//
// and requires
//
// assert.Negative(t, value)
// assert.Positive(t, value)
//
// Typed signed zeros (like `int(0)`, `int8(0)`, ..., `int64(0)`) are also supported.
type NegativePositive struct{}

// NewNegativePositive constructs NegativePositive checker.
Expand Down Expand Up @@ -63,7 +63,7 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
}
a, b := call.Args[0], call.Args[1]

if !isZeroValue(a) && isZeroValue(b) {
if isSignedNotZero(pass, a) && isSignedZero(b) {
return newUseNegativeDiagnostic(a.Pos(), b.End(), a)
}

Expand All @@ -73,7 +73,7 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
}
a, b := call.Args[0], call.Args[1]

if isZeroValue(a) && !isZeroValue(b) {
if isSignedZero(a) && isSignedNotZero(pass, b) {
return newUseNegativeDiagnostic(a.Pos(), b.End(), b)
}

Expand All @@ -83,8 +83,8 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
}
expr := call.Args[0]

a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZeroValue), token.LSS, p(isZeroValue)) // a < 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroValue), token.GTR, p(isNotZeroValue)) // 0 > a
a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.LSS, p(isSignedZero)) // a < 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isSignedZero), token.GTR, isSignedNotZero) // 0 > a

survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
if ok {
Expand All @@ -97,8 +97,8 @@ func (checker NegativePositive) checkNegative(pass *analysis.Pass, call *CallMet
}
expr := call.Args[0]

a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZeroValue), token.GEQ, p(isZeroValue)) // a >= 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroValue), token.LEQ, p(isNotZeroValue)) // 0 <= a
a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.GEQ, p(isSignedZero)) // a >= 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isSignedZero), token.LEQ, isSignedNotZero) // 0 <= a

survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
if ok {
Expand Down Expand Up @@ -127,7 +127,7 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
}
a, b := call.Args[0], call.Args[1]

if !isZeroValue(a) && isZeroValue(b) {
if isSignedNotZero(pass, a) && isSignedZero(b) {
return newUsePositiveDiagnostic(a.Pos(), b.End(), a)
}

Expand All @@ -137,7 +137,7 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
}
a, b := call.Args[0], call.Args[1]

if isZeroValue(a) && !isZeroValue(b) {
if isSignedZero(a) && isSignedNotZero(pass, b) {
return newUsePositiveDiagnostic(a.Pos(), b.End(), b)
}

Expand All @@ -147,8 +147,8 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
}
expr := call.Args[0]

a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZeroValue), token.GTR, p(isZeroValue)) // a > 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroValue), token.LSS, p(isNotZeroValue)) // 0 < a
a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.GTR, p(isSignedZero)) // a > 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isSignedZero), token.LSS, isSignedNotZero) // 0 < a

survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
if ok {
Expand All @@ -161,8 +161,8 @@ func (checker NegativePositive) checkPositive(pass *analysis.Pass, call *CallMet
}
expr := call.Args[0]

a, _, ok1 := isStrictComparisonWith(pass, expr, p(isNotZeroValue), token.LEQ, p(isZeroValue)) // a <= 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isZeroValue), token.GEQ, p(isNotZeroValue)) // 0 >= a
a, _, ok1 := isStrictComparisonWith(pass, expr, isSignedNotZero, token.LEQ, p(isSignedZero)) // a <= 0
_, b, ok2 := isStrictComparisonWith(pass, expr, p(isSignedZero), token.GEQ, isSignedNotZero) // 0 >= a

survivingArg, ok := anyVal([]bool{ok1, ok2}, a, b)
if ok {
Expand Down
Loading