Skip to content

Commit 018a843

Browse files
semihbkgrAntonboom
andauthored
fix: false-positive useless-assert on Positive function (#236)
* fix: false-positive useless-assert on Positive function when integer type is unsigned * useless-assert: test and doc --------- Co-authored-by: Anton Telyshev <[email protected]>
1 parent c055b2b commit 018a843

File tree

6 files changed

+46
-23
lines changed

6 files changed

+46
-23
lines changed

README.md

-2
Original file line numberDiff line numberDiff line change
@@ -1123,14 +1123,12 @@ assert.Zero(t, false) // Any bool literal.
11231123
assert.Negative(len(x))
11241124
assert.Less(len(x), 0)
11251125
assert.Greater(0, len(x))
1126-
assert.Positive(len(x))
11271126
assert.GreaterOrEqual(len(x), 0)
11281127
assert.LessOrEqual(0, len(x))
11291128

11301129
assert.Negative(uintVal)
11311130
assert.Less(uintVal, 0)
11321131
assert.Greater(0, uintVal)
1133-
assert.Positive(uintVal)
11341132
assert.GreaterOrEqual(uintVal, 0)
11351133
assert.LessOrEqual(0, uintVal)
11361134
```

analyzer/testdata/src/checkers-default/useless-assert/useless_assert_test.go

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

analyzer/testdata/src/debug/suite_method_signature_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ func (s *BrokenSuite) SetupTest() {
1818
var _ suite.TearDownSubTest
1919
}
2020

21-
func (s *BrokenSuite) SetT() { // *BrokenSuite does not implement suite.TestingSuite (wrong type for method SetT)
22-
}
21+
//func (s *BrokenSuite) SetT() {} // *BrokenSuite does not implement suite.TestingSuite (wrong type for method SetT)
2322

2423
func (s *BrokenSuite) TestTypo(_ *testing.T) { // value.go:424: test panicked: reflect: Call with too few input arguments
2524
s.True(true)

analyzer/testdata/src/debug/useless_assert_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,7 @@ func TestUselessAsserts(t *testing.T) {
2323
assert.Zero(t, "")
2424
assert.Zero(t, nil)
2525
}
26+
27+
func TestPositiveZero(t *testing.T) {
28+
assert.Positive(t, uint64(0))
29+
}

internal/checkers/useless_assert.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,12 @@ import (
5454
// assert.Negative(len(x))
5555
// assert.Less(len(x), 0)
5656
// assert.Greater(0, len(x))
57-
// assert.Positive(len(x))
5857
// assert.GreaterOrEqual(len(x), 0)
5958
// assert.LessOrEqual(0, len(x))
6059
//
6160
// assert.Negative(uintVal)
6261
// assert.Less(uintVal, 0)
6362
// assert.Greater(0, uintVal)
64-
// assert.Positive(uintVal)
6563
// assert.GreaterOrEqual(uintVal, 0)
6664
// assert.LessOrEqual(0, uintVal)
6765
type UselessAssert struct{}
@@ -94,7 +92,13 @@ func (checker UselessAssert) Check(pass *analysis.Pass, call *CallMeta) *analysi
9492
case "LessOrEqual", "Greater":
9593
isMeaningless = (len(call.Args) >= 2) && isAnyZero(call.Args[0]) && canNotBeNegative(pass, call.Args[1])
9694

97-
case "Negative", "Positive":
95+
case "Positive":
96+
if len(call.Args) < 1 {
97+
return nil
98+
}
99+
_, isMeaningless = isIntBasicLit(call.Args[0])
100+
101+
case "Negative":
98102
if len(call.Args) < 1 {
99103
return nil
100104
}

internal/testgen/gen_useless_assert.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,12 @@ func (g UselessAssertTestsGenerator) TemplateData() any {
134134
{Fn: "Negative", Argsf: "len(x)", ReportMsgf: defaultReport},
135135
{Fn: "Less", Argsf: "len(x), 0", ReportMsgf: defaultReport},
136136
{Fn: "Greater", Argsf: "0, len(x)", ReportMsgf: defaultReport},
137-
{Fn: "Positive", Argsf: "len(x)", ReportMsgf: defaultReport},
138137
{Fn: "GreaterOrEqual", Argsf: "len(x), 0", ReportMsgf: defaultReport},
139138
{Fn: "LessOrEqual", Argsf: "0, len(x)", ReportMsgf: defaultReport},
140139

141140
{Fn: "Negative", Argsf: "uint(42)", ReportMsgf: defaultReport},
142141
{Fn: "Less", Argsf: "uint(42), 0", ReportMsgf: defaultReport},
143142
{Fn: "Greater", Argsf: "0, uint(42)", ReportMsgf: defaultReport},
144-
{Fn: "Positive", Argsf: "uint(42)", ReportMsgf: defaultReport},
145143
{Fn: "GreaterOrEqual", Argsf: "uint(42), 0", ReportMsgf: defaultReport},
146144
{Fn: "LessOrEqual", Argsf: "0, uint(42)", ReportMsgf: defaultReport},
147145
},
@@ -173,6 +171,12 @@ func (g UselessAssertTestsGenerator) TemplateData() any {
173171
{Fn: "Zero", Argsf: "str"},
174172
{Fn: "Zero", Argsf: "new(testCase)"},
175173
{Fn: "Zero", Argsf: "b"},
174+
175+
// NOTE(a.telyshev): An unsigned value can be 0.
176+
{Fn: "Positive", Argsf: "len(x)"},
177+
{Fn: "Positive", Argsf: "uint(42)"},
178+
{Fn: "Greater", Argsf: "len(x), 0"},
179+
{Fn: "Greater", Argsf: "uint(42), 0"},
176180
},
177181
}
178182
}

0 commit comments

Comments
 (0)