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

contribution guideline: remove zero checker #103

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Changes from all commits
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
62 changes: 13 additions & 49 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,23 @@ Install dev tools...
### 2) Create a checker skeleton in `internal/checkers/{checker_name}.go`

For example, we want to create a new checker
`Zero` in `internal/checkers/zero.go`:
`TimeCompare` in `internal/checkers/time_compare.go`:

```go
package checkers

// Zero detects situations like
// TimeCompare detects situations like
//
// assert.Equal(t, 0, count)
// assert.Equal(t, nil, userObj)
// assert.Equal(t, expTs, actualTs)
//
// and requires
//
// assert.Zero(t, count)
// assert.Zero(t, userObj)
type Zero struct{}
// assert.True(t, actualTs.Equal(expTs))
type TimeCompare struct{}

// NewZero constructs Zero checker.
func NewZero() Zero { return Zero{} }
func (Zero) Name() string { return "zero" }
// NewTimeCompare constructs TimeCompare checker.
func NewTimeCompare() TimeCompare { return TimeCompare{} }
func (TimeCompare) Name() string { return "TimeCompare" }
```

The above code is enough to satisfy the `checkers.Checker` interface.
Expand All @@ -41,14 +39,14 @@ The above code is enough to satisfy the `checkers.Checker` interface.

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

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

```go
var registry = checkersRegistry{
// ...
{factory: asCheckerFactory(NewZero), enabledByDefault: false},
{factory: asCheckerFactory(NewTimeCompare), enabledByDefault: true},
// ...
{factory: asCheckerFactory(NewEmpty), enabledByDefault: true},
// ...
Expand All @@ -61,7 +59,7 @@ By default, we disable the checker if we doubt its 100% usefulness.

### 4) Create new tests generator in `internal/testgen/gen_{checker_name}.go`

Create new `ZeroTestsGenerator` in `internal/testgen/gen_zero.go`.
Create new `TimeCompareTestsGenerator` in `internal/testgen/gen_time_compare.go`.

See examples in adjacent files.

Expand Down Expand Up @@ -89,7 +87,7 @@ FAIL

### 7) Implement the checker

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

If the checker turns out to be too “fat”, then you can omit some obviously rare combinations,
Expand Down Expand Up @@ -140,7 +138,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
- [suite-run](#suite-run)
- [suite-test-name](#suite-test-name)
- [useless-assert](#useless-assert)
- [zero](#zero)

---

Expand Down Expand Up @@ -409,38 +406,5 @@ assert.ErrorContains(t, err, "user") ❌

---

### zero

```go
❌ assert.Equal(t, 0, count)
assert.Equal(t, nil, userObj)
assert.Equal(t, "", name)
// etc.

✅ assert.Zero(t, count)
assert.Zero(t, userObj)
assert.Zero(t, name)
```

**Autofix**: true. <br>
**Enabled by default**: false. <br>
**Reason**: Just for your reflection and suggestion. <br>
**Related issues**: [#75](https://github.com/Antonboom/testifylint/issues/75)

I'm not sure if anyone uses `assert.Zero` – it looks strange and conflicts with `assert.Empty`:

```go
❌ assert.Equal(t, "", result)
assert.Nil(t, errCh)

✅ assert.Empty(t, result)
assert.Empty(t, errCh)
```

Maybe it's better to make configurable support for other types in the `empty` checker and
vice versa to prohibit the `Zero`?

---

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).
Loading