Skip to content

Commit

Permalink
Merge branch 'empty-error' of github.com:mmorel-35/testifylint into e…
Browse files Browse the repository at this point in the history
…mpty-error
  • Loading branch information
Antonboom committed Oct 3, 2024
2 parents 4ccb328 + 6cd0162 commit a7e1c09
Show file tree
Hide file tree
Showing 50 changed files with 1,891 additions and 266 deletions.
22 changes: 0 additions & 22 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
- [http-sugar](#http-sugar)
- [require-len](#require-len)
- [suite-test-name](#suite-test-name)
- [useless-assert](#useless-assert)

---

Expand Down Expand Up @@ -367,26 +366,5 @@ func (s *HandlersSuite) Test_UsecaseSuccess()

---

### useless-assert

Support more complex cases, e.g.

```go
body, err := io.ReadAll(rr.Body)
require.NoError(t, err)
require.NoError(t, err) ❌

expectedJSON, err := json.Marshal(expected)
require.NoError(t, err)
require.JSONEq(t, string(expectedJSON), string(body))
```

```go
require.NoError(t, err)
assert.ErrorContains(t, err, "user") ❌
```

---

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).
77 changes: 70 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ https://golangci-lint.run/usage/linters/#testifylint
| [compares](#compares) |||
| [contains](#contains) |||
| [empty](#empty) |||
| [encoded-compare](#encoded-compare) |||
| [error-is-as](#error-is-as) || 🤏 |
| [error-nil](#error-nil) |||
| [expected-actual](#expected-actual) |||
Expand Down Expand Up @@ -274,6 +275,39 @@ assert.NotEmpty(t, err)

---

### encoded-compare

```go
assert.Equal(t, `{"foo": "bar"}`, body)
assert.EqualValues(t, `{"foo": "bar"}`, body)
assert.Exactly(t, `{"foo": "bar"}`, body)
assert.Equal(t, expectedJSON, resultJSON)
assert.Equal(t, expBodyConst, w.Body.String())
assert.Equal(t, fmt.Sprintf(`{"value":"%s"}`, hexString), result)
assert.Equal(t, "{}", json.RawMessage(resp))
assert.Equal(t, expJSON, strings.Trim(string(resultJSONBytes), "\n")) // + Replace, ReplaceAll, TrimSpace

assert.Equal(t, expectedYML, conf)

assert.JSONEq(t, `{"foo": "bar"}`, body)
assert.YAMLEq(t, expectedYML, conf)
```

**Autofix**: true. <br>
**Enabled by default**: true. <br>
**Reason**: Protection from bugs and more appropriate `testify` API with clearer failure message.

`encoded-compare` detects JSON-style string constants (usable in `fmt.Sprintf` also) and JSON-style/YAML-style named
variables. If variable is converted to `json.RawMessage`, then it is considered JSON unconditionally.

When fixing, `encoded-compare` removes unnecessary conversions to `[]byte`, `string`, `json.RawMessage` and calls of
`strings.Replace`, `strings.ReplaceAll`, `strings.Trim`, `strings.TrimSpace`, and adds a conversion to `string` when
needed.

---

### error-is-as

```go
Expand Down Expand Up @@ -654,7 +688,7 @@ assert.Equal(t, (chan Event)(nil), eventsChan)
assert.NotEqual(t, (chan Event)(nil), eventsChan)
```

But in the case of `Equal`, `NotEqual` and `Exactly` type casting approach still doesn't work for the function type.
But in the case of `Equal`, `NotEqual` and `Exactly` type conversion approach still doesn't work for the function type.

The best option here is to just use `Nil` / `NotNil` (see [details](https://github.com/stretchr/testify/issues/1524)).

Expand Down Expand Up @@ -884,22 +918,51 @@ a [checkers.AdvancedChecker](https://github.com/Antonboom/testifylint/blob/67632

### useless-assert

Currently the checker guards against assertion of the same variable:
The checker guards against assertion of the same variable:

```go
assert.Contains(t, tt.value, tt.value)
assert.ElementsMatch(t, tt.value, tt.value)
assert.Equal(t, tt.value, tt.value)
assert.ElementsMatch(t, users, users)
// And so on...
assert.EqualExportedValues(t, tt.value, tt.value)
// And other assert functions...

assert.True(t, num > num)
assert.True(t, num < num)
assert.True(t, num >= num)
assert.True(t, num <= num)
assert.True(t, num == num)
assert.True(t, num != num)

assert.False(t, num > num)
assert.False(t, num < num)
assert.False(t, num >= num)
assert.False(t, num <= num)
assert.False(t, num == num)
assert.False(t, num != num)
```

More complex cases are [open for contribution](CONTRIBUTING.md#useless-assert).
And against these meaningless assertions:
```go
assert.Empty(t, "")
assert.False(t, false)
assert.Implements(t, (*any)(nil), new(Conn))
assert.Negative(t, -42)
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.Zero(t, nil)
```

**Autofix**: false. <br>
**Enabled by default**: true. <br>
**Reason**: Protection from bugs and possible dead code.
**Reason**: Protection from bugs and dead code.

---

Expand Down
60 changes: 37 additions & 23 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,29 @@ import (
func TestTestifyLint(t *testing.T) {
t.Parallel()

for _, checker := range checkers.All() {
checker := checker

t.Run(checker, func(t *testing.T) {
t.Parallel()

anlzr := analyzer.New()
if err := anlzr.Flags.Set("disable-all", "true"); err != nil {
t.Fatal(err)
}
if err := anlzr.Flags.Set("enable", checker); err != nil {
t.Fatal(err)
}

pkg := filepath.Join("checkers-default", checker)
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), anlzr, pkg)
})
}
}

func TestTestifyLint_NotDefaultCases(t *testing.T) {
t.Parallel()

cases := []struct {
dir string
flags map[string]string
Expand Down Expand Up @@ -53,6 +76,20 @@ func TestTestifyLint(t *testing.T) {
"expected-actual.pattern": "goldenValue",
},
},
{
dir: "formatter-issue170",
flags: map[string]string{
"disable-all": "true",
"enable": checkers.NewFormatter().Name(),
},
},
{
dir: "formatter-issue170-suite",
flags: map[string]string{
"disable-all": "true",
"enable": checkers.NewFormatter().Name(),
},
},
{
dir: "formatter-not-defaults",
flags: map[string]string{
Expand Down Expand Up @@ -129,26 +166,3 @@ func TestTestifyLint(t *testing.T) {
})
}
}

func TestTestifyLint_CheckersDefault(t *testing.T) {
t.Parallel()

for _, checker := range checkers.All() {
checker := checker

t.Run(checker, func(t *testing.T) {
t.Parallel()

anlzr := analyzer.New()
if err := anlzr.Flags.Set("disable-all", "true"); err != nil {
t.Fatal(err)
}
if err := anlzr.Flags.Set("enable", checker); err != nil {
t.Fatal(err)
}

pkg := filepath.Join("checkers-default", checker)
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), anlzr, pkg)
})
}
}
2 changes: 2 additions & 0 deletions analyzer/checkers_factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func Test_newCheckers(t *testing.T) {
checkers.NewErrorNil(),
checkers.NewNilCompare(),
checkers.NewErrorIsAs(),
checkers.NewEncodedCompare(),
checkers.NewExpectedActual(),
checkers.NewRegexp(),
checkers.NewSuiteExtraAssertCall(),
Expand All @@ -43,6 +44,7 @@ func Test_newCheckers(t *testing.T) {
checkers.NewErrorNil(),
checkers.NewNilCompare(),
checkers.NewErrorIsAs(),
checkers.NewEncodedCompare(),
checkers.NewExpectedActual(),
checkers.NewRegexp(),
checkers.NewSuiteExtraAssertCall(),
Expand Down
16 changes: 12 additions & 4 deletions analyzer/testdata/src/checkers-default/empty/empty_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a7e1c09

Please sign in to comment.