Skip to content

Commit 2f0f318

Browse files
ccoVeilleccoVeille
committed
add new checker: negative-positive
1 parent 7d5e124 commit 2f0f318

10 files changed

+336
-52
lines changed

‎CONTRIBUTING.md

+11-29
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ package checkers
2121

2222
// Zero detects situations like
2323
//
24-
// assert.Equal(t, 0, count)
24+
// assert.Equal(t, 0, count)
2525
// assert.Equal(t, nil, userObj)
2626
//
2727
// and requires
2828
//
29-
// assert.Zero(t, count)
29+
// assert.Zero(t, count)
3030
// assert.Zero(t, userObj)
3131
type Zero struct{}
3232

@@ -41,8 +41,8 @@ The above code is enough to satisfy the `checkers.Checker` interface.
4141

4242
The earlier the checker is in [the registry](internal/checkers/checkers_registry.go), the more priority it is.
4343

44-
For example, the `zero` checker takes precedence over the `expected-actual` or `empty`,
45-
because its check is more "narrow" and when you fix the warning from `zero`,
44+
For example, the `zero` checker takes precedence over the `expected-actual` or `empty`,
45+
because its check is more "narrow" and when you fix the warning from `zero`,
4646
the rest of the checkers will become irrelevant.
4747

4848
```go
@@ -89,7 +89,7 @@ FAIL
8989

9090
### 7) Implement the checker
9191

92-
`Zero` is an example of [checkers.RegularChecker](./internal/checkers/checker.go) because it works with "general"
92+
`Zero` is an example of [checkers.RegularChecker](./internal/checkers/checker.go) because it works with "general"
9393
assertion call. For more complex checkers, use the [checkers.AdvancedChecker](./internal/checkers/checker.go) interface.
9494

9595
If the checker turns out to be too “fat”, then you can omit some obviously rare combinations,
@@ -130,7 +130,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
130130
- [float-compare](#float-compare)
131131
- [http-const](#http-const)
132132
- [http-sugar](#http-sugar)
133-
- [negative-positive](#negative-positive)
134133
- [no-fmt-mess](#no-fmt-mess)
135134
- [require-len](#require-len)
136135
- [suite-run](#suite-run)
@@ -229,7 +228,7 @@ func (s *ServiceIntegrationSuite) TearDownTest() {
229228
assert.GreaterOrEqual(t, a, 42.42)
230229
assert.Less(t, a, 42.42)
231230
assert.LessOrEqual(t, a, 42.42)
232-
231+
233232
assert.True(t, a != 42.42) // assert.False(t, a == 42.42)
234233
assert.True(t, a > 42.42) // assert.False(t, a <= 42.42)
235234
assert.True(t, a >= 42.42) // assert.False(t, a < 42.42)
@@ -283,7 +282,7 @@ And similar idea for `assert.InEpsilonSlice` / `assert.InDeltaSlice`.
283282
```go
284283
❌ assert.HTTPStatusCode(t,
285284
handler, http.MethodGet, "/index", nil, http.StatusOK)
286-
assert.HTTPStatusCode(t,
285+
assert.HTTPStatusCode(t,
287286
handler, http.MethodGet, "/admin", nil, http.StatusNotFound)
288287
assert.HTTPStatusCode(t,
289288
handler, http.MethodGet, "/oauth", nil, http.StatusFound)
@@ -300,23 +299,6 @@ And similar idea for `assert.InEpsilonSlice` / `assert.InDeltaSlice`.
300299

301300
---
302301

303-
### negative-positive
304-
305-
```go
306-
❌ assert.Less(t, val, 0)
307-
assert.Greater(t, val, 0)
308-
309-
✅ assert.Negative(t, val)
310-
assert.Positive(t, val)
311-
```
312-
313-
**Autofix**: true. <br>
314-
**Enabled by default**: maybe? <br>
315-
**Reason**: More appropriate `testify` API with clearer failure message. <br>
316-
**Related issues**: [#76](https://github.com/Antonboom/testifylint/issues/76)
317-
318-
---
319-
320302
### no-fmt-mess
321303

322304
**Autofix**: true. <br>
@@ -330,7 +312,7 @@ func Equal(t TestingT, expected, actual interface{}, msgAndArgs ...interface{})
330312
func Equalf(t TestingT, expected interface{}, actual interface{}, msg string, args ...interface{}) bool
331313
```
332314

333-
The f-functions [were added a long time ago](https://github.com/stretchr/testify/pull/356) to eliminate
315+
The f-functions [were added a long time ago](https://github.com/stretchr/testify/pull/356) to eliminate
334316
`govet` [complain](https://go.googlesource.com/tools/+/refs/heads/release-branch.go1.12/go/analysis/passes/printf/printf.go?pli=1#506).
335317

336318
This introduces some inconsistency into the test code, and the next strategies are seen for the checker:
@@ -343,7 +325,7 @@ This will make it easier to migrate to [v2](https://github.com/stretchr/testify/
343325
344326
But it doesn't look like a "go way" and the `govet` won't be happy.
345327

346-
2) IMHO, a more appropriate option is to disallow the use of `msgAndArgs` in non-f assertions. Look at
328+
2) IMHO, a more appropriate option is to disallow the use of `msgAndArgs` in non-f assertions. Look at
347329
[the comment](https://github.com/stretchr/testify/issues/1089#issuecomment-1695059265).
348330

349331
But there will be no non-stylistic benefits from the checker in this case (depends on the view of API in `v2`).
@@ -357,7 +339,7 @@ assert.Error(t, err, fmt.Sprintf("Profile %s should not be valid", test.profile)
357339

358340
### require-len
359341

360-
The main idea: if code contains array/slice indexing,
342+
The main idea: if code contains array/slice indexing,
361343
then before that there must be a length constraint through `require`.
362344

363345
```go
@@ -383,7 +365,7 @@ func (s *Suite) TestSomething() {
383365
// ...
384366
assert.Equal(t, "gopher", result)
385367
})
386-
368+
387369
✅
388370
s.Run("subtest1", func() {
389371
// ...

‎README.md

+43-23
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,24 @@ https://golangci-lint.run/usage/linters/#testifylint
6969

7070
| Name | Enabled By Default | Autofix |
7171
|-----------------------------------------------------|--------------------|---------|
72-
| [blank-import](#blank-import) | ✅ | ❌ |
73-
| [bool-compare](#bool-compare) | ✅ | ✅ |
74-
| [compares](#compares) | ✅ | ✅ |
75-
| [empty](#empty) | ✅ | ✅ |
76-
| [error-is-as](#error-is-as) | ✅ | ✅ |
77-
| [error-nil](#error-nil) | ✅ | ✅ |
78-
| [expected-actual](#expected-actual) | ✅ | ✅ |
79-
| [float-compare](#float-compare) | ✅ | ❌ |
80-
| [go-require](#go-require) | ✅ | ❌ |
81-
| [len](#len) | ✅ | ✅ |
82-
| [nil-compare](#nil-compare) | ✅ | ✅ |
83-
| [require-error](#require-error) | ✅ | ❌ |
84-
| [suite-dont-use-pkg](#suite-dont-use-pkg) | ✅ | ✅ |
85-
| [suite-extra-assert-call](#suite-extra-assert-call) | ✅ | ✅ |
86-
| [suite-thelper](#suite-thelper) | ❌ | ✅ |
87-
| [useless-assert](#useless-assert) | ✅ | ❌ |
72+
| [blank-import](#blank-import) | ✅ | ❌ |
73+
| [bool-compare](#bool-compare) | ✅ | ✅ |
74+
| [compares](#compares) | ✅ | ✅ |
75+
| [empty](#empty) | ✅ | ✅ |
76+
| [error-is-as](#error-is-as) | ✅ | ✅ |
77+
| [error-nil](#error-nil) | ✅ | ✅ |
78+
| [error-nil](#error-nil) | ✅ | ✅ |
79+
| [expected-actual](#expected-actual) | ✅ | ✅ |
80+
| [float-compare](#float-compare) | ✅ | ❌ |
81+
| [go-require](#go-require) | ✅ | ❌ |
82+
| [len](#len) | ✅ | ✅ |
83+
| [negative-positive](#negative-positive) | ✅ | ✅ |
84+
| [nil-compare](#nil-compare) | ✅ | ✅ |
85+
| [require-error](#require-error) | ✅ | ❌ |
86+
| [suite-dont-use-pkg](#suite-dont-use-pkg) | ✅ | ✅ |
87+
| [suite-extra-assert-call](#suite-extra-assert-call) | ✅ | ✅ |
88+
| [suite-thelper](#suite-thelper) | ❌ | ✅ |
89+
| [useless-assert](#useless-assert) | ✅ | ❌ |
8890

8991
> ⚠️ Also look at open for contribution [checkers](CONTRIBUTING.md#open-for-contribution)
9092
@@ -124,7 +126,7 @@ import (
124126
assert.EqualValues(t, false, result)
125127
assert.Exactly(t, false, result)
126128
assert.NotEqual(t, true, result)
127-
assert.NotEqualValues(t, true, result)
129+
assert.NotEqualValues(t, true, result)
128130
assert.False(t, !result)
129131
assert.True(t, result == true)
130132
// And other variations...
@@ -229,7 +231,7 @@ To turn off this behavior use the `--bool-compare.ignore-custom-types` flag.
229231
**Reason**: In the first two cases, a common mistake that leads to hiding the incorrect wrapping of sentinel errors.
230232
In the rest cases – more appropriate `testify` API with clearer failure message.
231233

232-
Also `error-is-as` repeats `go vet`'s [errorsas check](https://cs.opensource.google/go/x/tools/+/master:go/analysis/passes/errorsas/errorsas.go)
234+
Also `error-is-as` repeats `go vet`'s [errorsas check](https://cs.opensource.google/go/x/tools/+/master:go/analysis/passes/errorsas/errorsas.go)
233235
logic, but without autofix.
234236

235237
---
@@ -305,7 +307,7 @@ It is planned [to change the order of assertion arguments](https://github.com/st
305307
assert.Exactly(t, 42.42, result)
306308
assert.True(t, result == 42.42)
307309
assert.False(t, result != 42.42)
308-
310+
309311
✅ assert.InEpsilon(t, 42.42, result, 0.0001) // Or assert.InDelta
310312
```
311313

@@ -323,7 +325,7 @@ This checker is similar to the [floatcompare](https://github.com/golangci/golang
323325
go func() {
324326
conn, err = lis.Accept()
325327
require.NoError(t, err) ❌
326-
328+
327329
if assert.Error(err) {
328330
assert.FailNow(t, msg) ❌
329331
}
@@ -337,7 +339,7 @@ go func() {
337339
This checker is a radically improved analogue of `go vet`'s
338340
[testinggoroutine](https://cs.opensource.google/go/x/tools/+/master:go/analysis/passes/testinggoroutine/doc.go) check.
339341

340-
The point of the check is that, according to the [documentation](https://pkg.go.dev/testing#T),
342+
The point of the check is that, according to the [documentation](https://pkg.go.dev/testing#T),
341343
functions leading to `t.FailNow` (essentially to `runtime.GoExit`) must only be used in the goroutine that runs the test.
342344
Otherwise, they will not work as declared, namely, finish the test function.
343345

@@ -376,6 +378,24 @@ P.S. Related `testify`'s [thread](https://github.com/stretchr/testify/issues/772
376378

377379
---
378380

381+
### negative-positive
382+
383+
```go
384+
❌ assert.Less(t, val, 0)
385+
assert.Greater(t, 0, val)
386+
assert.Less(t, val, 0)
387+
assert.Greater(t, 0, val)
388+
389+
✅ assert.Negative(t, val)
390+
assert.Positive(t, val)
391+
```
392+
393+
**Autofix**: true. <br>
394+
**Enabled by default**: true <br>
395+
**Reason**: More appropriate `testify` API with clearer failure message.
396+
397+
---
398+
379399
### nil-compare
380400

381401
```go
@@ -433,7 +453,7 @@ The best option here is to just use `Nil` / `NotNil` (see [details](https://gith
433453
**Enabled by default**: true. <br>
434454
**Reason**: Such "ignoring" of errors leads to further panics, making the test harder to debug.
435455

436-
[testify/require](https://pkg.go.dev/github.com/stretchr/testify@master/require#hdr-Assertions) allows
456+
[testify/require](https://pkg.go.dev/github.com/stretchr/testify@master/require#hdr-Assertions) allows
437457
to stop test execution when a test fails.
438458

439459
By default `require-error` only checks the `*Error*` assertions, presented above. <br>
@@ -484,7 +504,7 @@ But sometimes, on the contrary, people want consistency with `s.Assert()` and `s
484504
```go
485505
func (s *MySuite) TestSomething() {
486506
// ...
487-
507+
488508
❌
489509
s.Require().NoError(err)
490510
s.Equal(42, value)

‎analyzer/checkers_factory_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ func Test_newCheckers(t *testing.T) {
1919
checkers.NewEmpty(),
2020
checkers.NewLen(),
2121
checkers.NewCompares(),
22+
checkers.NewNegativePositive(),
2223
checkers.NewErrorNil(),
2324
checkers.NewNilCompare(),
2425
checkers.NewErrorIsAs(),
@@ -33,6 +34,7 @@ func Test_newCheckers(t *testing.T) {
3334
checkers.NewEmpty(),
3435
checkers.NewLen(),
3536
checkers.NewCompares(),
37+
checkers.NewNegativePositive(),
3638
checkers.NewErrorNil(),
3739
checkers.NewNilCompare(),
3840
checkers.NewErrorIsAs(),

‎analyzer/testdata/src/checkers-default/negative-positive/negative_positive_test.go

+57
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎analyzer/testdata/src/checkers-default/negative-positive/negative_positive_test.go.golden

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Code generated by testifylint/internal/testgen. DO NOT EDIT.
2+
3+
package negativepositive
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestNegativePositiveChecker(t *testing.T) {
12+
var a int
13+
14+
// Invalid.
15+
{
16+
assert.Positive(t, a) // want "negative-positive: use assert\\.Positive"
17+
assert.Positivef(t, a, "msg with args %d %s", 42, "42") // want "negative-positive: use assert\\.Positivef"
18+
assert.Negative(t, a) // want "negative-positive: use assert\\.Negative"
19+
assert.Negativef(t, a, "msg with args %d %s", 42, "42") // want "negative-positive: use assert\\.Negativef"
20+
assert.Negative(t, a) // want "negative-positive: use assert\\.Negative"
21+
assert.Negativef(t, a, "msg with args %d %s", 42, "42") // want "negative-positive: use assert\\.Negativef"
22+
assert.Positive(t, a) // want "negative-positive: use assert\\.Positive"
23+
assert.Positivef(t, a, "msg with args %d %s", 42, "42") // want "negative-positive: use assert\\.Positivef"
24+
}
25+
26+
// Valid.
27+
{
28+
assert.Greater(t, 1, a)
29+
assert.Greaterf(t, 1, a, "msg with args %d %s", 42, "42")
30+
assert.Less(t, 1, a)
31+
assert.Lessf(t, 1, a, "msg with args %d %s", 42, "42")
32+
}
33+
34+
// Ignored.
35+
{
36+
assert.Greater(t, 1, a)
37+
assert.Greaterf(t, 1, a, "msg with args %d %s", 42, "42")
38+
assert.Greater(t, -1, a)
39+
assert.Greaterf(t, -1, a, "msg with args %d %s", 42, "42")
40+
assert.Greater(t, 0, 0)
41+
assert.Greaterf(t, 0, 0, "msg with args %d %s", 42, "42")
42+
assert.GreaterOrEqual(t, 0, a)
43+
assert.GreaterOrEqualf(t, 0, a, "msg with args %d %s", 42, "42")
44+
assert.GreaterOrEqual(t, a, 0)
45+
assert.GreaterOrEqualf(t, a, 0, "msg with args %d %s", 42, "42")
46+
assert.Less(t, 1, a)
47+
assert.Lessf(t, 1, a, "msg with args %d %s", 42, "42")
48+
assert.Less(t, -1, a)
49+
assert.Lessf(t, -1, a, "msg with args %d %s", 42, "42")
50+
assert.Less(t, 0, 0)
51+
assert.Lessf(t, 0, 0, "msg with args %d %s", 42, "42")
52+
assert.LessOrEqual(t, 0, a)
53+
assert.LessOrEqualf(t, 0, a, "msg with args %d %s", 42, "42")
54+
assert.LessOrEqual(t, a, 0)
55+
assert.LessOrEqualf(t, a, 0, "msg with args %d %s", 42, "42")
56+
}
57+
}

‎internal/checkers/checkers_registry.go

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ var registry = checkersRegistry{
1212
{factory: asCheckerFactory(NewEmpty), enabledByDefault: true},
1313
{factory: asCheckerFactory(NewLen), enabledByDefault: true},
1414
{factory: asCheckerFactory(NewCompares), enabledByDefault: true},
15+
{factory: asCheckerFactory(NewNegativePositive), enabledByDefault: true},
1516
{factory: asCheckerFactory(NewErrorNil), enabledByDefault: true},
1617
{factory: asCheckerFactory(NewNilCompare), enabledByDefault: true},
1718
{factory: asCheckerFactory(NewErrorIsAs), enabledByDefault: true},

0 commit comments

Comments
 (0)