diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2914942..1e993d8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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) --- @@ -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 🙏
List of `testify` functions [here](https://pkg.go.dev/github.com/stretchr/testify@master/assert#pkg-functions). diff --git a/README.md b/README.md index 7fe6314..114050d 100644 --- a/README.md +++ b/README.md @@ -880,22 +880,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.
**Enabled by default**: true.
-**Reason**: Protection from bugs and possible dead code. +**Reason**: Protection from bugs and dead code. --- 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 index 8781eeb..14e4af1 100644 --- a/analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go +++ b/analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go @@ -15,6 +15,7 @@ func TestUselessAssertChecker(t *testing.T) { var elapsed time.Time var str string var num int + var b bool var tc testCase // Invalid. @@ -61,6 +62,104 @@ func TestUselessAssertChecker(t *testing.T) { 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.Empty(t, "") // want "useless-assert: meaningless assertion" + assert.Empty(t, "", "msg") // want "useless-assert: meaningless assertion" + assert.Empty(t, "", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Empty(t, "", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Emptyf(t, "", "msg") // want "useless-assert: meaningless assertion" + assert.Emptyf(t, "", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Emptyf(t, "", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.False(t, false) // want "useless-assert: meaningless assertion" + assert.False(t, false, "msg") // want "useless-assert: meaningless assertion" + assert.False(t, false, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.False(t, false, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Falsef(t, false, "msg") // want "useless-assert: meaningless assertion" + assert.Falsef(t, false, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Falsef(t, false, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Implements(t, (*any)(nil), new(testing.T)) // want "useless-assert: meaningless assertion" + assert.Implements(t, (*any)(nil), new(testing.T), "msg") // want "useless-assert: meaningless assertion" + assert.Implements(t, (*any)(nil), new(testing.T), "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Implements(t, (*any)(nil), new(testing.T), "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Implementsf(t, (*any)(nil), new(testing.T), "msg") // want "useless-assert: meaningless assertion" + assert.Implementsf(t, (*any)(nil), new(testing.T), "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Implementsf(t, (*any)(nil), new(testing.T), "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Negative(t, -42) // want "useless-assert: meaningless assertion" + assert.Negative(t, -42, "msg") // want "useless-assert: meaningless assertion" + assert.Negative(t, -42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Negative(t, -42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Negativef(t, -42, "msg") // want "useless-assert: meaningless assertion" + assert.Negativef(t, -42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Negativef(t, -42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Nil(t, nil) // want "useless-assert: meaningless assertion" + assert.Nil(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.Nil(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Nil(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Nilf(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.Nilf(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Nilf(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NoError(t, nil) // want "useless-assert: meaningless assertion" + assert.NoError(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.NoError(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NoError(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NoErrorf(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.NoErrorf(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NoErrorf(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotEmpty(t, "value") // want "useless-assert: meaningless assertion" + assert.NotEmpty(t, "value", "msg") // want "useless-assert: meaningless assertion" + assert.NotEmpty(t, "value", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotEmpty(t, "value", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotEmptyf(t, "value", "msg") // want "useless-assert: meaningless assertion" + assert.NotEmptyf(t, "value", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotEmptyf(t, "value", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotZero(t, 42) // want "useless-assert: meaningless assertion" + assert.NotZero(t, 42, "msg") // want "useless-assert: meaningless assertion" + assert.NotZero(t, 42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotZero(t, 42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotZerof(t, 42, "msg") // want "useless-assert: meaningless assertion" + assert.NotZerof(t, 42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotZerof(t, 42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotZero(t, "value") // want "useless-assert: meaningless assertion" + assert.NotZero(t, "value", "msg") // want "useless-assert: meaningless assertion" + assert.NotZero(t, "value", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotZero(t, "value", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.NotZerof(t, "value", "msg") // want "useless-assert: meaningless assertion" + assert.NotZerof(t, "value", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.NotZerof(t, "value", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Positive(t, 42) // want "useless-assert: meaningless assertion" + assert.Positive(t, 42, "msg") // want "useless-assert: meaningless assertion" + assert.Positive(t, 42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Positive(t, 42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Positivef(t, 42, "msg") // want "useless-assert: meaningless assertion" + assert.Positivef(t, 42, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Positivef(t, 42, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.True(t, true) // want "useless-assert: meaningless assertion" + assert.True(t, true, "msg") // want "useless-assert: meaningless assertion" + assert.True(t, true, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.True(t, true, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Truef(t, true, "msg") // want "useless-assert: meaningless assertion" + assert.Truef(t, true, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Truef(t, true, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zero(t, 0) // want "useless-assert: meaningless assertion" + assert.Zero(t, 0, "msg") // want "useless-assert: meaningless assertion" + assert.Zero(t, 0, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zero(t, 0, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zerof(t, 0, "msg") // want "useless-assert: meaningless assertion" + assert.Zerof(t, 0, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zerof(t, 0, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zero(t, "") // want "useless-assert: meaningless assertion" + assert.Zero(t, "", "msg") // want "useless-assert: meaningless assertion" + assert.Zero(t, "", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zero(t, "", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zerof(t, "", "msg") // want "useless-assert: meaningless assertion" + assert.Zerof(t, "", "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zerof(t, "", "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zero(t, nil) // want "useless-assert: meaningless assertion" + assert.Zero(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.Zero(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zero(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" + assert.Zerof(t, nil, "msg") // want "useless-assert: meaningless assertion" + assert.Zerof(t, nil, "msg with arg %d", 42) // want "useless-assert: meaningless assertion" + assert.Zerof(t, nil, "msg with args %d %s", 42, "42") // want "useless-assert: meaningless assertion" 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.ElementsMatch(t, value, value) // want "useless-assert: asserting of the same variable" @@ -186,6 +285,90 @@ func TestUselessAssertChecker(t *testing.T) { 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") + assert.Empty(t, str) + assert.Empty(t, str, "msg") + assert.Empty(t, str, "msg with arg %d", 42) + assert.Empty(t, str, "msg with args %d %s", 42, "42") + assert.Emptyf(t, str, "msg") + assert.Emptyf(t, str, "msg with arg %d", 42) + assert.Emptyf(t, str, "msg with args %d %s", 42, "42") + assert.False(t, b) + assert.False(t, b, "msg") + assert.False(t, b, "msg with arg %d", 42) + assert.False(t, b, "msg with args %d %s", 42, "42") + assert.Falsef(t, b, "msg") + assert.Falsef(t, b, "msg with arg %d", 42) + assert.Falsef(t, b, "msg with args %d %s", 42, "42") + assert.Implements(t, (*testing.TB)(nil), new(testing.T)) + assert.Implements(t, (*testing.TB)(nil), new(testing.T), "msg") + assert.Implements(t, (*testing.TB)(nil), new(testing.T), "msg with arg %d", 42) + assert.Implements(t, (*testing.TB)(nil), new(testing.T), "msg with args %d %s", 42, "42") + assert.Implementsf(t, (*testing.TB)(nil), new(testing.T), "msg") + assert.Implementsf(t, (*testing.TB)(nil), new(testing.T), "msg with arg %d", 42) + assert.Implementsf(t, (*testing.TB)(nil), new(testing.T), "msg with args %d %s", 42, "42") + assert.Negative(t, num) + assert.Negative(t, num, "msg") + assert.Negative(t, num, "msg with arg %d", 42) + assert.Negative(t, num, "msg with args %d %s", 42, "42") + assert.Negativef(t, num, "msg") + assert.Negativef(t, num, "msg with arg %d", 42) + assert.Negativef(t, num, "msg with args %d %s", 42, "42") + assert.Nil(t, new(testCase)) + assert.Nil(t, new(testCase), "msg") + assert.Nil(t, new(testCase), "msg with arg %d", 42) + assert.Nil(t, new(testCase), "msg with args %d %s", 42, "42") + assert.Nilf(t, new(testCase), "msg") + assert.Nilf(t, new(testCase), "msg with arg %d", 42) + assert.Nilf(t, new(testCase), "msg with args %d %s", 42, "42") + assert.NoError(t, err) + assert.NoError(t, err, "msg") + assert.NoError(t, err, "msg with arg %d", 42) + assert.NoError(t, err, "msg with args %d %s", 42, "42") + assert.NoErrorf(t, err, "msg") + assert.NoErrorf(t, err, "msg with arg %d", 42) + assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") + assert.NotEmpty(t, str) + assert.NotEmpty(t, str, "msg") + assert.NotEmpty(t, str, "msg with arg %d", 42) + assert.NotEmpty(t, str, "msg with args %d %s", 42, "42") + assert.NotEmptyf(t, str, "msg") + assert.NotEmptyf(t, str, "msg with arg %d", 42) + assert.NotEmptyf(t, str, "msg with args %d %s", 42, "42") + assert.Positive(t, num) + assert.Positive(t, num, "msg") + assert.Positive(t, num, "msg with arg %d", 42) + assert.Positive(t, num, "msg with args %d %s", 42, "42") + assert.Positivef(t, num, "msg") + assert.Positivef(t, num, "msg with arg %d", 42) + assert.Positivef(t, num, "msg with args %d %s", 42, "42") + assert.True(t, b) + assert.True(t, b, "msg") + assert.True(t, b, "msg with arg %d", 42) + assert.True(t, b, "msg with args %d %s", 42, "42") + assert.Truef(t, b, "msg") + assert.Truef(t, b, "msg with arg %d", 42) + assert.Truef(t, b, "msg with args %d %s", 42, "42") + assert.Zero(t, num) + assert.Zero(t, num, "msg") + assert.Zero(t, num, "msg with arg %d", 42) + assert.Zero(t, num, "msg with args %d %s", 42, "42") + assert.Zerof(t, num, "msg") + assert.Zerof(t, num, "msg with arg %d", 42) + assert.Zerof(t, num, "msg with args %d %s", 42, "42") + assert.Zero(t, str) + assert.Zero(t, str, "msg") + assert.Zero(t, str, "msg with arg %d", 42) + assert.Zero(t, str, "msg with args %d %s", 42, "42") + assert.Zerof(t, str, "msg") + assert.Zerof(t, str, "msg with arg %d", 42) + assert.Zerof(t, str, "msg with args %d %s", 42, "42") + assert.Zero(t, new(testCase)) + assert.Zero(t, new(testCase), "msg") + assert.Zero(t, new(testCase), "msg with arg %d", 42) + assert.Zero(t, new(testCase), "msg with args %d %s", 42, "42") + assert.Zerof(t, new(testCase), "msg") + assert.Zerof(t, new(testCase), "msg with arg %d", 42) + assert.Zerof(t, new(testCase), "msg with args %d %s", 42, "42") } } diff --git a/analyzer/testdata/src/debug/useless_assert_test.go b/analyzer/testdata/src/debug/useless_assert_test.go new file mode 100644 index 0000000..f54e1b6 --- /dev/null +++ b/analyzer/testdata/src/debug/useless_assert_test.go @@ -0,0 +1,25 @@ +package debug + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUselessAsserts(t *testing.T) { + assert.Empty(t, "") + assert.False(t, false) + assert.Implements(t, (*any)(nil), new(testing.T)) + 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.Zero(t, nil) + assert.Positive(t, 42) + assert.True(t, true) + assert.Zero(t, 0) + assert.Zero(t, "") + assert.Zero(t, nil) +} diff --git a/internal/checkers/compares.go b/internal/checkers/compares.go index bdde03d..461286e 100644 --- a/internal/checkers/compares.go +++ b/internal/checkers/compares.go @@ -61,7 +61,9 @@ func (checker Compares) Check(pass *analysis.Pass, call *CallMeta) *analysis.Dia return nil } - if isPointer(pass, be.X) && isPointer(pass, be.Y) { + _, xp := isPointer(pass, be.X) + _, yp := isPointer(pass, be.Y) + if xp && yp { switch proposedFn { case "Equal": proposedFn = "Same" diff --git a/internal/checkers/helpers_basic_type.go b/internal/checkers/helpers_basic_type.go index 432a303..28c0d68 100644 --- a/internal/checkers/helpers_basic_type.go +++ b/internal/checkers/helpers_basic_type.go @@ -1,10 +1,10 @@ package checkers import ( - "fmt" "go/ast" "go/token" "go/types" + "strconv" "golang.org/x/tools/go/analysis" ) @@ -56,9 +56,29 @@ func isTypedIntNumber(e ast.Expr, v int, types ...string) bool { return false } -func isIntNumber(e ast.Expr, v int) bool { +func isIntNumber(e ast.Expr, rhs int) bool { + lhs, ok := isIntBasicLit(e) + return ok && (lhs == rhs) +} + +func isNegativeIntNumber(e ast.Expr) bool { + v, ok := isIntBasicLit(e) + return ok && v < 0 +} + +func isPositiveIntNumber(e ast.Expr) bool { + v, ok := isIntBasicLit(e) + return ok && v > 0 +} + +func isEmptyStringLit(e ast.Expr) bool { bl, ok := e.(*ast.BasicLit) - return ok && bl.Kind == token.INT && bl.Value == fmt.Sprintf("%d", v) + return ok && bl.Kind == token.STRING && bl.Value == `""` +} + +func isNotEmptyStringLit(e ast.Expr) bool { + bl, ok := e.(*ast.BasicLit) + return ok && bl.Kind == token.STRING && bl.Value != `""` } func isBasicLit(e ast.Expr) bool { @@ -66,9 +86,27 @@ func isBasicLit(e ast.Expr) bool { return ok } -func isIntBasicLit(e ast.Expr) bool { +func isIntBasicLit(e ast.Expr) (int, bool) { + if un, ok := e.(*ast.UnaryExpr); ok { + if un.Op == token.SUB { + v, ok := isIntBasicLit(un.X) + return -1 * v, ok + } + } + bl, ok := e.(*ast.BasicLit) - return ok && bl.Kind == token.INT + if !ok { + return 0, false + } + if bl.Kind != token.INT { + return 0, false + } + + v, err := strconv.Atoi(bl.Value) + if err != nil { + return 0, false + } + return v, true } func isUntypedConst(pass *analysis.Pass, e ast.Expr) bool { @@ -98,9 +136,12 @@ func isUnderlying(pass *analysis.Pass, e ast.Expr, flag types.BasicInfo) bool { return ok && (bt.Info()&flag > 0) } -func isPointer(pass *analysis.Pass, e ast.Expr) bool { - _, ok := pass.TypesInfo.TypeOf(e).(*types.Pointer) - return ok +func isPointer(pass *analysis.Pass, e ast.Expr) (types.Type, bool) { + ptr, ok := pass.TypesInfo.TypeOf(e).(*types.Pointer) + if !ok { + return nil, false + } + return ptr.Elem(), true } // untype returns v from type(v) expression or v itself if there is no type cast. diff --git a/internal/checkers/helpers_interface.go b/internal/checkers/helpers_interface.go index b0c0d13..ad39c72 100644 --- a/internal/checkers/helpers_interface.go +++ b/internal/checkers/helpers_interface.go @@ -15,8 +15,11 @@ func isEmptyInterface(pass *analysis.Pass, expr ast.Expr) bool { if !ok { return false } + return isEmptyInterfaceType(t.Type) +} - iface, ok := t.Type.Underlying().(*types.Interface) +func isEmptyInterfaceType(t types.Type) bool { + iface, ok := t.Underlying().(*types.Interface) return ok && iface.NumMethods() == 0 } diff --git a/internal/checkers/len.go b/internal/checkers/len.go index 4733056..70767e5 100644 --- a/internal/checkers/len.go +++ b/internal/checkers/len.go @@ -31,7 +31,7 @@ func (checker Len) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnost a, b := call.Args[0], call.Args[1] if lenArg, expectedLen, ok := xorLenCall(pass, a, b); ok { - if expectedLen == b && !isIntBasicLit(expectedLen) { + if _, ok := isIntBasicLit(expectedLen); (expectedLen == b) && !ok { // https://github.com/Antonboom/testifylint/issues/9 return nil } @@ -50,7 +50,10 @@ func (checker Len) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnost } expr := call.Args[0] - if lenArg, expectedLen, ok := isLenEquality(pass, expr); ok && isIntBasicLit(expectedLen) { + if lenArg, expectedLen, ok := isLenEquality(pass, expr); ok { + if _, ok := isIntBasicLit(expectedLen); !ok { + return nil + } return newUseFunctionDiagnostic(checker.Name(), call, proposedFn, newSuggestedFuncReplacement(call, proposedFn, analysis.TextEdit{ Pos: expr.Pos(), diff --git a/internal/checkers/useless_assert.go b/internal/checkers/useless_assert.go index 6f206d0..0bb455d 100644 --- a/internal/checkers/useless_assert.go +++ b/internal/checkers/useless_assert.go @@ -10,15 +10,40 @@ import ( // UselessAssert detects useless asserts like // -// 1) Asserting of the same variable -// +// 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) +// assert.EqualExportedValues(t, tt.value, tt.value) // ... +// // 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) // -// 2) Open for contribution... +// 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) type UselessAssert struct{} // NewUselessAssert constructs UselessAssert checker. @@ -26,6 +51,58 @@ func NewUselessAssert() UselessAssert { return UselessAssert{} } func (UselessAssert) Name() string { return "useless-assert" } func (checker UselessAssert) Check(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic { + if d := checker.checkSameVars(pass, call); d != nil { + return d + } + + var isMeaningless bool + switch call.Fn.NameFTrimmed { + case "Empty": + isMeaningless = (len(call.Args) >= 1) && isEmptyStringLit(call.Args[0]) + + case "False": + isMeaningless = (len(call.Args) >= 1) && isUntypedFalse(pass, call.Args[0]) + + case "Implements": + if len(call.Args) < 2 { + return nil + } + + elem, ok := isPointer(pass, call.Args[0]) + isMeaningless = ok && isEmptyInterfaceType(elem) + + case "Negative": + isMeaningless = (len(call.Args) >= 1) && isNegativeIntNumber(call.Args[0]) + + case "Nil", "NoError": + isMeaningless = (len(call.Args) >= 1) && isNil(call.Args[0]) + + case "NotEmpty": + isMeaningless = (len(call.Args) >= 1) && isNotEmptyStringLit(call.Args[0]) + + case "NotZero": + isMeaningless = (len(call.Args) >= 1) && + (isNotEmptyStringLit(call.Args[0]) || + isNegativeIntNumber(call.Args[0]) || isPositiveIntNumber(call.Args[0])) + + case "Positive": + isMeaningless = (len(call.Args) >= 1) && isPositiveIntNumber(call.Args[0]) + + case "True": + isMeaningless = (len(call.Args) >= 1) && isUntypedTrue(pass, call.Args[0]) + + case "Zero": + isMeaningless = (len(call.Args) >= 1) && + (isZero(call.Args[0]) || isEmptyStringLit(call.Args[0]) || isNil(call.Args[0])) + } + + if isMeaningless { + return newDiagnostic(checker.Name(), call, "meaningless assertion", nil) + } + return nil +} + +func (checker UselessAssert) checkSameVars(pass *analysis.Pass, call *CallMeta) *analysis.Diagnostic { var first, second ast.Node switch call.Fn.NameFTrimmed { diff --git a/internal/testgen/gen_useless_assert.go b/internal/testgen/gen_useless_assert.go index 5394c63..fc27f93 100644 --- a/internal/testgen/gen_useless_assert.go +++ b/internal/testgen/gen_useless_assert.go @@ -16,6 +16,7 @@ func (g UselessAssertTestsGenerator) TemplateData() any { var ( checker = g.Checker().Name() sameVarReport = checker + ": asserting of the same variable" + defaultReport = checker + ": meaningless assertion" ) var twoSideAssertions []Assertion @@ -86,6 +87,21 @@ func (g UselessAssertTestsGenerator) TemplateData() any { {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}, + + {Fn: "Empty", Argsf: `""`, ReportMsgf: defaultReport}, + {Fn: "False", Argsf: "false", ReportMsgf: defaultReport}, + {Fn: "Implements", Argsf: "(*any)(nil), new(testing.T)", ReportMsgf: defaultReport}, + {Fn: "Negative", Argsf: "-42", ReportMsgf: defaultReport}, + {Fn: "Nil", Argsf: "nil", ReportMsgf: defaultReport}, + {Fn: "NoError", Argsf: "nil", ReportMsgf: defaultReport}, + {Fn: "NotEmpty", Argsf: `"value"`, ReportMsgf: defaultReport}, + {Fn: "NotZero", Argsf: `42`, ReportMsgf: defaultReport}, + {Fn: "NotZero", Argsf: `"value"`, ReportMsgf: defaultReport}, + {Fn: "Positive", Argsf: "42", ReportMsgf: defaultReport}, + {Fn: "True", Argsf: "true", ReportMsgf: defaultReport}, + {Fn: "Zero", Argsf: "0", ReportMsgf: defaultReport}, + {Fn: "Zero", Argsf: `""`, ReportMsgf: defaultReport}, + {Fn: "Zero", Argsf: "nil", ReportMsgf: defaultReport}, }, InvalidAssertions: twoSideAssertions, ValidAssertions: []Assertion{ @@ -94,6 +110,19 @@ func (g UselessAssertTestsGenerator) TemplateData() any { {Fn: "Equal", Argsf: `tc.A(), "tc.A()"`}, {Fn: "Equal", Argsf: "testCase{}.A().B().C(), tc.A().B().C()"}, {Fn: "IsType", Argsf: "tc, testCase{}"}, + + {Fn: "Empty", Argsf: "str"}, + {Fn: "False", Argsf: "b"}, + {Fn: "Implements", Argsf: "(*testing.TB)(nil), new(testing.T)"}, + {Fn: "Negative", Argsf: "num"}, + {Fn: "Nil", Argsf: "new(testCase)"}, + {Fn: "NoError", Argsf: "err"}, + {Fn: "NotEmpty", Argsf: "str"}, + {Fn: "Positive", Argsf: "num"}, + {Fn: "True", Argsf: "b"}, + {Fn: "Zero", Argsf: "num"}, + {Fn: "Zero", Argsf: "str"}, + {Fn: "Zero", Argsf: "new(testCase)"}, }, } } @@ -126,6 +155,7 @@ func {{ .CheckerName.AsTestName }}(t *testing.T) { var elapsed time.Time var str string var num int + var b bool var tc testCase // Invalid.