From 297fbfae7180b6a57b765d243a53cfa7567e44f7 Mon Sep 17 00:00:00 2001 From: Anton Telyshev Date: Tue, 23 Jan 2024 22:11:19 +0200 Subject: [PATCH 1/3] new checker 'useless-assert' --- CONTRIBUTING.md | 48 +++-- README.md | 20 +++ analyzer/checkers_factory_test.go | 2 + .../expected-actual/expected_actual_test.go | 4 + .../expected_actual_test.go.golden | 4 + .../useless-assert/useless_assert_test.go | 170 ++++++++++++++++++ internal/checkers/checkers_registry.go | 1 + internal/checkers/checkers_registry_test.go | 2 + internal/checkers/expected_actual.go | 4 +- internal/checkers/useless_assert.go | 70 ++++++++ internal/testgen/gen_expected_actual.go | 9 + internal/testgen/gen_useless_assert.go | 137 ++++++++++++++ internal/testgen/main.go | 1 + 13 files changed, 445 insertions(+), 27 deletions(-) create mode 100644 analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go create mode 100644 internal/checkers/useless_assert.go create mode 100644 internal/testgen/gen_useless_assert.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a179966..482fec27 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -130,12 +130,12 @@ Describe a new checker in [checkers section](./README.md#checkers). - [float-compare](#float-compare) - [http-const](#http-const) - [http-sugar](#http-sugar) -- [inefficient-assert](#inefficient-assert) - [negative-positive](#negative-positive) - [no-fmt-mess](#no-fmt-mess) - [require-len](#require-len) - [suite-run](#suite-run) - [suite-test-name](#suite-test-name) +- [useless-assert](#useless-assert) - [zero](#zero) --- @@ -299,31 +299,6 @@ And similar idea for `assert.InEpsilonSlice` / `assert.InDeltaSlice`. --- -### inefficient-assert - -Simple: -```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)) -``` - -Complex: -```go -require.NoError(t, err) -assert.ErrorContains(t, err, "user") ❌ -``` - -**Autofix**: false.
-**Enabled by default**: Probably false, depends on implementation performance.
-**Reason**: Code simplification, elimination of possible bugs. - ---- - ### negative-positive ```go @@ -459,6 +434,27 @@ 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") ❌ +``` + +--- + ### zero ```go diff --git a/README.md b/README.md index 5e33b818..47a52cd1 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,7 @@ https://golangci-lint.run/usage/linters/#testifylint | [suite-dont-use-pkg](#suite-dont-use-pkg) | ✅ | ✅ | | [suite-extra-assert-call](#suite-extra-assert-call) | ✅ | ✅ | | [suite-thelper](#suite-thelper) | ❌ | ✅ | +| [useless-assert](#useless-assert) | ✅ | ❌ | > ⚠️ Also look at open for contribution [checkers](CONTRIBUTING.md#open-for-contribution) @@ -253,6 +254,7 @@ logic, but without autofix. assert.InDeltaMapValues(t, result, map[string]float64{"score": 0.99}, 1.0) assert.InDeltaSlice(t, result, []float64{0.98, 0.99}, 1.0) assert.InEpsilon(t, result, 42.42, 0.0001) + assert.InEpsilonSlice(t, result, []float64{0.9801, 0.9902}, 0.0001) assert.IsType(t, result, (*User)(nil)) assert.NotEqual(t, result, "expected") assert.NotEqualValues(t, result, "expected") @@ -511,6 +513,24 @@ The checker rather acts as an example of a [checkers.AdvancedChecker](https://gi --- +### useless-assert + +Currently the checker guards against assertion of the same variable: + +```go +❌ assert.Equal(t, tt.value, tt.value) + assert.ElementsMatch(t, users, users) + // And so on... +``` + +More complex cases are [open for contribution](CONTRIBUTING.md#useless-assert). + +**Autofix**: false.
+**Enabled by default**: true.
+**Reason**: Protection from bugs and possible dead code. + +--- + ## Chain of warnings Linter does not automatically handle the "evolution" of changes. diff --git a/analyzer/checkers_factory_test.go b/analyzer/checkers_factory_test.go index cea1bc25..660b89d0 100644 --- a/analyzer/checkers_factory_test.go +++ b/analyzer/checkers_factory_test.go @@ -25,6 +25,7 @@ func Test_newCheckers(t *testing.T) { checkers.NewExpectedActual(), checkers.NewSuiteExtraAssertCall(), checkers.NewSuiteDontUsePkg(), + checkers.NewUselessAssert(), } allRegularCheckers := []checkers.RegularChecker{ checkers.NewFloatCompare(), @@ -38,6 +39,7 @@ func Test_newCheckers(t *testing.T) { checkers.NewExpectedActual(), checkers.NewSuiteExtraAssertCall(), checkers.NewSuiteDontUsePkg(), + checkers.NewUselessAssert(), } enabledByDefaultAdvancedCheckers := []checkers.AdvancedChecker{ diff --git a/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go b/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go index 4c3b9dca..206b9809 100644 --- a/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go +++ b/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go @@ -221,6 +221,10 @@ func TestExpectedActualChecker_Other(t *testing.T) { assert.InEpsilonf(t, result, expected, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.InEpsilon(t, result, 42.42, 0.0001) // want "expected-actual: need to reverse actual and expected values" assert.InEpsilonf(t, result, 42.42, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlice(t, result, expected, 0.0001) // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlicef(t, result, expected, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlice(t, result, []float64{0.9801, 0.9902}, 0.0001) // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlicef(t, result, []float64{0.9801, 0.9902}, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.IsType(t, result, expected) // want "expected-actual: need to reverse actual and expected values" assert.IsTypef(t, result, expected, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.IsType(t, result, user{}) // want "expected-actual: need to reverse actual and expected values" diff --git a/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go.golden b/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go.golden index 1aa28c95..3bb78d9b 100644 --- a/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go.golden +++ b/analyzer/testdata/src/checkers-default/expected-actual/expected_actual_test.go.golden @@ -221,6 +221,10 @@ func TestExpectedActualChecker_Other(t *testing.T) { assert.InEpsilonf(t, expected, result, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.InEpsilon(t, 42.42, result, 0.0001) // want "expected-actual: need to reverse actual and expected values" assert.InEpsilonf(t, 42.42, result, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlice(t, expected, result, 0.0001) // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlicef(t, expected, result, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlice(t, []float64{0.9801, 0.9902}, result, 0.0001) // want "expected-actual: need to reverse actual and expected values" + assert.InEpsilonSlicef(t, []float64{0.9801, 0.9902}, result, 0.0001, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.IsType(t, expected, result) // want "expected-actual: need to reverse actual and expected values" assert.IsTypef(t, expected, result, "msg with args %d %s", 42, "42") // want "expected-actual: need to reverse actual and expected values" assert.IsType(t, user{}, result) // want "expected-actual: need to reverse actual and expected values" diff --git a/analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go b/analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go new file mode 100644 index 00000000..81d2473f --- /dev/null +++ b/analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go @@ -0,0 +1,170 @@ +// Code generated by testifylint/internal/testgen. DO NOT EDIT. +package uselessassert + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestUselessAssertChecker(t *testing.T) { + var value any + var err error + var elapsed time.Time + var str string + var tc testCase + + // Invalid. + { + assert.Equal(t, 42, 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, 42, 42, "msg") // want "useless-assert: asserting of the same variable" + assert.Equal(t, 42, 42, "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, 42, 42, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, 42, 42, "msg") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, 42, 42, "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, 42, 42, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equal(t, "value", "value") // want "useless-assert: asserting of the same variable" + assert.Equal(t, "value", "value", "msg") // want "useless-assert: asserting of the same variable" + assert.Equal(t, "value", "value", "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, "value", "value", "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, "value", "value", "msg") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, "value", "value", "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, "value", "value", "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equal(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Equal(t, value, value, "msg") // want "useless-assert: asserting of the same variable" + assert.Equal(t, value, value, "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, value, value, "msg") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, value, value, "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equal(t, tc.A(), tc.A()) // want "useless-assert: asserting of the same variable" + assert.Equal(t, tc.A(), tc.A(), "msg") // want "useless-assert: asserting of the same variable" + assert.Equal(t, tc.A(), tc.A(), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, tc.A(), tc.A(), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, tc.A(), tc.A(), "msg") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, tc.A(), tc.A(), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, tc.A(), tc.A(), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equal(t, testCase{}.A().B().C(), testCase{}.A().B().C()) // want "useless-assert: asserting of the same variable" + assert.Equal(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg") // want "useless-assert: asserting of the same variable" + assert.Equal(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equal(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg") // want "useless-assert: asserting of the same variable" + assert.Equalf(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, testCase{}.A().B().C(), testCase{}.A().B().C(), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.IsType(t, (*testCase)(nil), (*testCase)(nil)) // want "useless-assert: asserting of the same variable" + assert.IsType(t, (*testCase)(nil), (*testCase)(nil), "msg") // want "useless-assert: asserting of the same variable" + assert.IsType(t, (*testCase)(nil), (*testCase)(nil), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.IsType(t, (*testCase)(nil), (*testCase)(nil), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.IsTypef(t, (*testCase)(nil), (*testCase)(nil), "msg") // want "useless-assert: asserting of the same variable" + assert.IsTypef(t, (*testCase)(nil), (*testCase)(nil), "msg with arg %d", 42) // want "useless-assert: asserting of the same variable" + assert.IsTypef(t, (*testCase)(nil), (*testCase)(nil), "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Contains(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Containsf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Equal(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Equalf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Less(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Lessf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Implements(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Implementsf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.InDeltaSlice(t, value, value, 0.01) // want "useless-assert: asserting of the same variable" + assert.InDeltaSlicef(t, value, value, 0.01, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.JSONEq(t, str, str) // want "useless-assert: asserting of the same variable" + assert.JSONEqf(t, str, str, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotErrorIs(t, err, err) // want "useless-assert: asserting of the same variable" + assert.NotErrorIsf(t, err, err, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.EqualExportedValues(t, value, value) // want "useless-assert: asserting of the same variable" + assert.EqualExportedValuesf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.ErrorIs(t, err, err) // want "useless-assert: asserting of the same variable" + assert.ErrorIsf(t, err, err, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.ErrorAs(t, err, err) // want "useless-assert: asserting of the same variable" + assert.ErrorAsf(t, err, err, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Greater(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Greaterf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotEqualValues(t, value, value) // want "useless-assert: asserting of the same variable" + assert.NotEqualValuesf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Subset(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Subsetf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.YAMLEq(t, str, str) // want "useless-assert: asserting of the same variable" + assert.YAMLEqf(t, str, str, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.IsType(t, value, value) // want "useless-assert: asserting of the same variable" + assert.IsTypef(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.LessOrEqual(t, value, value) // want "useless-assert: asserting of the same variable" + assert.LessOrEqualf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotSame(t, value, value) // want "useless-assert: asserting of the same variable" + assert.NotSamef(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Same(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Samef(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.WithinDuration(t, elapsed, elapsed, time.Second) // want "useless-assert: asserting of the same variable" + assert.WithinDurationf(t, elapsed, elapsed, time.Second, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.GreaterOrEqual(t, value, value) // want "useless-assert: asserting of the same variable" + assert.GreaterOrEqualf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.InEpsilonSlice(t, value, value, 0.0001) // want "useless-assert: asserting of the same variable" + assert.InEpsilonSlicef(t, value, value, 0.0001, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotRegexp(t, value, value) // want "useless-assert: asserting of the same variable" + assert.NotRegexpf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.ElementsMatch(t, value, value) // want "useless-assert: asserting of the same variable" + assert.ElementsMatchf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.InEpsilon(t, value, value, 0.0001) // want "useless-assert: asserting of the same variable" + assert.InEpsilonf(t, value, value, 0.0001, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotSubset(t, value, value) // want "useless-assert: asserting of the same variable" + assert.NotSubsetf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Regexp(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Regexpf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.EqualValues(t, value, value) // want "useless-assert: asserting of the same variable" + assert.EqualValuesf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.Exactly(t, value, value) // want "useless-assert: asserting of the same variable" + assert.Exactlyf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.InDelta(t, value, value, 0.01) // want "useless-assert: asserting of the same variable" + assert.InDeltaf(t, value, value, 0.01, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.InDeltaMapValues(t, value, value, 0.01) // want "useless-assert: asserting of the same variable" + assert.InDeltaMapValuesf(t, value, value, 0.01, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + assert.NotEqual(t, value, value) // want "useless-assert: asserting of the same variable" + assert.NotEqualf(t, value, value, "msg with args %d %s", 42, "42") // want "useless-assert: asserting of the same variable" + } + + // Valid. + { + assert.Equal(t, value, 42) + assert.Equal(t, value, 42, "msg") + assert.Equal(t, value, 42, "msg with arg %d", 42) + assert.Equal(t, value, 42, "msg with args %d %s", 42, "42") + assert.Equalf(t, value, 42, "msg") + assert.Equalf(t, value, 42, "msg with arg %d", 42) + assert.Equalf(t, value, 42, "msg with args %d %s", 42, "42") + assert.Equal(t, value, "value") + assert.Equal(t, value, "value", "msg") + assert.Equal(t, value, "value", "msg with arg %d", 42) + assert.Equal(t, value, "value", "msg with args %d %s", 42, "42") + assert.Equalf(t, value, "value", "msg") + assert.Equalf(t, value, "value", "msg with arg %d", 42) + assert.Equalf(t, value, "value", "msg with args %d %s", 42, "42") + assert.Equal(t, tc.A(), "tc.A()") + assert.Equal(t, tc.A(), "tc.A()", "msg") + assert.Equal(t, tc.A(), "tc.A()", "msg with arg %d", 42) + assert.Equal(t, tc.A(), "tc.A()", "msg with args %d %s", 42, "42") + assert.Equalf(t, tc.A(), "tc.A()", "msg") + assert.Equalf(t, tc.A(), "tc.A()", "msg with arg %d", 42) + assert.Equalf(t, tc.A(), "tc.A()", "msg with args %d %s", 42, "42") + assert.Equal(t, testCase{}.A().B().C(), tc.A().B().C()) + assert.Equal(t, testCase{}.A().B().C(), tc.A().B().C(), "msg") + assert.Equal(t, testCase{}.A().B().C(), tc.A().B().C(), "msg with arg %d", 42) + assert.Equal(t, testCase{}.A().B().C(), tc.A().B().C(), "msg with args %d %s", 42, "42") + assert.Equalf(t, testCase{}.A().B().C(), tc.A().B().C(), "msg") + assert.Equalf(t, testCase{}.A().B().C(), tc.A().B().C(), "msg with arg %d", 42) + assert.Equalf(t, testCase{}.A().B().C(), tc.A().B().C(), "msg with args %d %s", 42, "42") + assert.IsType(t, tc, testCase{}) + assert.IsType(t, tc, testCase{}, "msg") + assert.IsType(t, tc, testCase{}, "msg with arg %d", 42) + assert.IsType(t, tc, testCase{}, "msg with args %d %s", 42, "42") + assert.IsTypef(t, tc, testCase{}, "msg") + assert.IsTypef(t, tc, testCase{}, "msg with arg %d", 42) + assert.IsTypef(t, tc, testCase{}, "msg with args %d %s", 42, "42") + } +} + +type testCase struct{} + +func (testCase) A() testCase { return testCase{} } +func (testCase) B() testCase { return testCase{} } +func (testCase) C() testCase { return testCase{} } diff --git a/internal/checkers/checkers_registry.go b/internal/checkers/checkers_registry.go index e629495a..e34a21bf 100644 --- a/internal/checkers/checkers_registry.go +++ b/internal/checkers/checkers_registry.go @@ -18,6 +18,7 @@ var registry = checkersRegistry{ {factory: asCheckerFactory(NewExpectedActual), enabledByDefault: true}, {factory: asCheckerFactory(NewSuiteExtraAssertCall), enabledByDefault: true}, {factory: asCheckerFactory(NewSuiteDontUsePkg), enabledByDefault: true}, + {factory: asCheckerFactory(NewUselessAssert), enabledByDefault: true}, // Advanced checkers. {factory: asCheckerFactory(NewBlankImport), enabledByDefault: true}, {factory: asCheckerFactory(NewGoRequire), enabledByDefault: true}, diff --git a/internal/checkers/checkers_registry_test.go b/internal/checkers/checkers_registry_test.go index b88b1637..251c6950 100644 --- a/internal/checkers/checkers_registry_test.go +++ b/internal/checkers/checkers_registry_test.go @@ -46,6 +46,7 @@ func TestAll(t *testing.T) { "expected-actual", "suite-extra-assert-call", "suite-dont-use-pkg", + "useless-assert", "blank-import", "go-require", "require-error", @@ -75,6 +76,7 @@ func TestEnabledByDefault(t *testing.T) { "expected-actual", "suite-extra-assert-call", "suite-dont-use-pkg", + "useless-assert", "blank-import", "go-require", "require-error", diff --git a/internal/checkers/expected_actual.go b/internal/checkers/expected_actual.go index 8b3a14bc..0cd01ded 100644 --- a/internal/checkers/expected_actual.go +++ b/internal/checkers/expected_actual.go @@ -26,6 +26,7 @@ var DefaultExpectedVarPattern = regexp.MustCompile( // assert.InDeltaMapValues(t, result, map[string]float64{"score": 0.99}, 1.0) // assert.InDeltaSlice(t, result, []float64{0.98, 0.99}, 1.0) // assert.InEpsilon(t, result, 42.42, 0.0001) +// assert.InEpsilonSlice(t, result, []float64{0.9801, 0.9902}, 0.0001) // assert.IsType(t, result, (*User)(nil)) // assert.NotEqual(t, result, "expected") // assert.NotEqualValues(t, result, "expected") @@ -64,12 +65,13 @@ func (checker ExpectedActual) Check(pass *analysis.Pass, call *CallMeta) *analys "EqualExportedValues", "EqualValues", "Exactly", - "JSONEq", "InDelta", "InDeltaMapValues", "InDeltaSlice", "InEpsilon", + "InEpsilonSlice", "IsType", + "JSONEq", "NotEqual", "NotEqualValues", "NotSame", diff --git a/internal/checkers/useless_assert.go b/internal/checkers/useless_assert.go new file mode 100644 index 00000000..91826963 --- /dev/null +++ b/internal/checkers/useless_assert.go @@ -0,0 +1,70 @@ +package checkers + +import ( + "golang.org/x/tools/go/analysis" + + "github.com/Antonboom/testifylint/internal/analysisutil" +) + +// UselessAssert detects useless asserts like +// +// 1) Asserting of the same variable +// +// assert.Equal(t, tt.value, tt.value) +// assert.ElementsMatch(t, users, users) +// +// 2) Open for contribution... +type UselessAssert struct{} + +// NewUselessAssert constructs UselessAssert checker. +func NewUselessAssert() UselessAssert { return UselessAssert{} } +func (UselessAssert) Name() string { return "useless-assert" } + +func (checker UselessAssert) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic { + switch call.Fn.NameFTrimmed { + case + "Contains", + "ElementsMatch", + "Equal", + "EqualExportedValues", + "EqualValues", + "ErrorAs", + "ErrorIs", + "Exactly", + "Greater", + "GreaterOrEqual", + "Implements", + "InDelta", + "InDeltaMapValues", + "InDeltaSlice", + "InEpsilon", + "InEpsilonSlice", + "IsType", + "JSONEq", + "Less", + "LessOrEqual", + "NotEqual", + "NotEqualValues", + "NotErrorIs", + "NotRegexp", + "NotSame", + "NotSubset", + "Regexp", + "Same", + "Subset", + "WithinDuration", + "YAMLEq": + default: + return nil + } + + if len(call.Args) < 2 { + return nil + } + first, second := call.Args[0], call.Args[1] + + if analysisutil.NodeString(pass.Fset, first) == analysisutil.NodeString(pass.Fset, second) { + return newDiagnostic(checker.Name(), call, "asserting of the same variable", nil) + } + return nil +} diff --git a/internal/testgen/gen_expected_actual.go b/internal/testgen/gen_expected_actual.go index 25fcc05a..3d578337 100644 --- a/internal/testgen/gen_expected_actual.go +++ b/internal/testgen/gen_expected_actual.go @@ -155,6 +155,15 @@ func (g ExpectedActualTestsGenerator) TemplateData() any { ReportMsgf: report, ProposedArgsf: "42.42, result, 0.0001", }, + { + Fn: "InEpsilonSlice", Argsf: "result, expected, 0.0001", + ReportMsgf: report, ProposedArgsf: "expected, result, 0.0001", + }, + { + Fn: "InEpsilonSlice", Argsf: `result, []float64{0.9801, 0.9902}, 0.0001`, + ReportMsgf: report, ProposedArgsf: `[]float64{0.9801, 0.9902}, result, 0.0001`, + }, + {Fn: "IsType", Argsf: "result, expected", ReportMsgf: report, ProposedArgsf: "expected, result"}, {Fn: "IsType", Argsf: "result, user{}", ReportMsgf: report, ProposedArgsf: "user{}, result"}, {Fn: "IsType", Argsf: "result, (*user)(nil)", ReportMsgf: report, ProposedArgsf: "(*user)(nil), result"}, diff --git a/internal/testgen/gen_useless_assert.go b/internal/testgen/gen_useless_assert.go new file mode 100644 index 00000000..68080353 --- /dev/null +++ b/internal/testgen/gen_useless_assert.go @@ -0,0 +1,137 @@ +package main + +import ( + "text/template" + + "github.com/Antonboom/testifylint/internal/checkers" +) + +type UselessAssertTestsGenerator struct{} + +func (UselessAssertTestsGenerator) Checker() checkers.Checker { + return checkers.NewUselessAssert() +} + +func (g UselessAssertTestsGenerator) TemplateData() any { + var ( + checker = g.Checker().Name() + sameVarReport = checker + ": asserting of the same variable" + ) + + var twoSideAssertions []Assertion //nolint:prealloc + for fn, args := range map[string]string{ + "Contains": "value, value", + "ElementsMatch": "value, value", + "Equal": "value, value", + "EqualExportedValues": "value, value", + "EqualValues": "value, value", + "ErrorAs": "err, err", + "ErrorIs": "err, err", + "Exactly": "value, value", + "Greater": "value, value", + "GreaterOrEqual": "value, value", + "Implements": "value, value", + "InDelta": "value, value, 0.01", + "InDeltaMapValues": "value, value, 0.01", + "InDeltaSlice": "value, value, 0.01", + "InEpsilon": "value, value, 0.0001", + "InEpsilonSlice": "value, value, 0.0001", + "IsType": "value, value", + "JSONEq": "str, str", + "Less": "value, value", + "LessOrEqual": "value, value", + "NotEqual": "value, value", + "NotEqualValues": "value, value", + "NotErrorIs": "err, err", + "NotRegexp": "value, value", + "NotSame": "value, value", + "NotSubset": "value, value", + "Regexp": "value, value", + "Same": "value, value", + "Subset": "value, value", + "WithinDuration": "elapsed, elapsed, time.Second", + "YAMLEq": "str, str", + } { + twoSideAssertions = append(twoSideAssertions, + Assertion{Fn: fn, Argsf: args, ReportMsgf: sameVarReport}) + } + + return struct { + CheckerName CheckerName + InvalidAssertionsSmoke []Assertion + InvalidAssertions []Assertion + ValidAssertions []Assertion + }{ + CheckerName: CheckerName(checker), + InvalidAssertionsSmoke: []Assertion{ + {Fn: "Equal", Argsf: "42, 42", ReportMsgf: sameVarReport}, + {Fn: "Equal", Argsf: `"value", "value"`, ReportMsgf: sameVarReport}, + {Fn: "Equal", Argsf: "value, value", ReportMsgf: sameVarReport}, + {Fn: "Equal", Argsf: "tc.A(), tc.A()", ReportMsgf: sameVarReport}, + {Fn: "Equal", Argsf: "testCase{}.A().B().C(), testCase{}.A().B().C()", ReportMsgf: sameVarReport}, + {Fn: "IsType", Argsf: "(*testCase)(nil), (*testCase)(nil)", ReportMsgf: sameVarReport}, + }, + InvalidAssertions: twoSideAssertions, + ValidAssertions: []Assertion{ + {Fn: "Equal", Argsf: "value, 42"}, + {Fn: "Equal", Argsf: `value, "value"`}, + {Fn: "Equal", Argsf: `tc.A(), "tc.A()"`}, + {Fn: "Equal", Argsf: "testCase{}.A().B().C(), tc.A().B().C()"}, + {Fn: "IsType", Argsf: "tc, testCase{}"}, + }, + } +} + +func (UselessAssertTestsGenerator) ErroredTemplate() Executor { + return template.Must(template.New("UselessAssertTestsGenerator.ErroredTemplate"). + Funcs(fm). + Parse(uselessAssertTestTmpl)) +} + +func (UselessAssertTestsGenerator) GoldenTemplate() Executor { + // NOTE(a.telyshev): Only the developer understands the correct picture. + return nil +} + +const uselessAssertTestTmpl = header + ` +package {{ .CheckerName.AsPkgName }} + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func {{ .CheckerName.AsTestName }}(t *testing.T) { + var value any + var err error + var elapsed time.Time + var str string + var tc testCase + + // Invalid. + { + {{- range $ai, $assrn := $.InvalidAssertionsSmoke }} + {{ NewAssertionExpander.FullMode.Expand $assrn "assert" "t" nil }} + {{- end }} + + {{- range $ai, $assrn := $.InvalidAssertions }} + {{ NewAssertionExpander.Expand $assrn "assert" "t" nil }} + {{- end }} + } + + // Valid. + { + {{- range $ai, $assrn := $.ValidAssertions }} + {{ NewAssertionExpander.FullMode.Expand $assrn "assert" "t" nil }} + {{- end }} + } +} + +type testCase struct{} + +func (testCase) A() testCase { return testCase{} } +func (testCase) B() testCase { return testCase{} } +func (testCase) C() testCase { return testCase{} } +` diff --git a/internal/testgen/main.go b/internal/testgen/main.go index aa6d7d0d..ac5ca4d8 100644 --- a/internal/testgen/main.go +++ b/internal/testgen/main.go @@ -38,6 +38,7 @@ var checkerTestsGenerators = []CheckerTestsGenerator{ SuiteDontUsePkg{}, SuiteExtraAssertCallTestsGenerator{}, SuiteTHelperTestsGenerator{}, + UselessAssertTestsGenerator{}, } func init() { From db685e17e40f3e79c81646d1b6634d546a0422de Mon Sep 17 00:00:00 2001 From: Anton Telyshev Date: Wed, 24 Jan 2024 08:15:24 +0200 Subject: [PATCH 2/3] fix dependabot github-actions version policy --- .github/dependabot.yml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 66a7d390..8dbcb718 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,13 +1,17 @@ version: 2 updates: - - package-ecosystem: github-actions - directory: / - update-types: semver-major + - package-ecosystem: "github-actions" + directory: "/" schedule: - interval: monthly + interval: "monthly" + ignore: + - dependency-name: "*" + update-types: + - "version-update:semver-minor" + - "version-update:semver-patch" - - package-ecosystem: gomod - directory: / + - package-ecosystem: "gomod" + directory: "/" schedule: - interval: monthly + interval: "monthly" From 2bd66b83b9cf29503ffc33f2e4d93f1d86889986 Mon Sep 17 00:00:00 2001 From: Anton Telyshev Date: Wed, 24 Jan 2024 08:23:47 +0200 Subject: [PATCH 3/3] useless-assert: self-review changes --- internal/checkers/useless_assert.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/checkers/useless_assert.go b/internal/checkers/useless_assert.go index 91826963..669f9d18 100644 --- a/internal/checkers/useless_assert.go +++ b/internal/checkers/useless_assert.go @@ -12,6 +12,7 @@ import ( // // assert.Equal(t, tt.value, tt.value) // assert.ElementsMatch(t, users, users) +// ... // // 2) Open for contribution... type UselessAssert struct{}