Revert "Add unitless duration lint check."
This reverts commit b55059174c9f0cda51e7a12038ba79644d158bf3.
It turned out to have too many false positives.
diff --git a/lint.go b/lint.go
index 11f6ed7..7de99bc 100644
--- a/lint.go
+++ b/lint.go
@@ -21,7 +21,6 @@
"unicode"
"unicode/utf8"
- "golang.org/x/tools/go/exact"
"golang.org/x/tools/go/gcimporter"
"golang.org/x/tools/go/types"
)
@@ -188,7 +187,6 @@
f.lintErrorReturn()
f.lintUnexportedReturn()
f.lintTimeNames()
- f.lintDurationUnits()
}
type link string
@@ -1398,70 +1396,6 @@
})
}
-func (f *file) lintDurationUnits() {
- f.walk(func(node ast.Node) bool {
- e, ok := node.(ast.Expr)
- if !ok {
- return true
- }
-
- tv := f.pkg.typesInfo.Types[e]
- if !f.pkg.isNamedType(tv.Type, "time", "Duration") {
- return true
- }
- if tv.Value == nil {
- return true
- }
- // OK for 0 to be unitless; non-constants are OK.
- if x, ok := exact.Int64Val(tv.Value); !ok || x == 0 {
- return true
- }
-
- // e has a constant value. What matters now is how it was defined.
- // Its definition might be spread across many expressions:
- //
- // const (
- // a = iota
- // b
- // c = b * time.Second
- // e = c * 10
- // )
- //
- // Rather than attempt to reconstruct e's origins,
- // just handle the common cases.
- switch e.(type) {
- default:
- // Assume that any non-literal, non-call expression is ok.
- // In particular, allow 3 * time.Second, which is an *ast.BinExpr.
- // This means that we fail to flag cases like
- // time.Duration(15) + time.Duration(30) and
- // time.Sleep(x) where 'const x = 10'.
- return false
- case *ast.CallExpr:
- // Descend into call expressions in order to catch time.Duration(15).
- return true
- case *ast.BasicLit:
- // A basic literal cannot possibly use time package constants.
- }
-
- f.errorf(e, 0.8, category("time"), `time.Duration expression "%v" should use time package constants`, f.render(e))
- // TODO: Generate a ReplacementLine with an appropriately sized unit.
- // For example, 3e9 should become 3 * time.Second. The unit part is easy.
- // The challenge is figuring out exactly what else is on the line
- // and handling the case in which there are multiple expressions
- // to replace on the same line.
- // The latter opens up API questions. If there are multiple
- // problems on the same line, should they all generate identical
- // replacements, or one replacement per problem?
- // The former would mean that this linter has to maintain state across
- // expressions.
- // Also, the test framework does not yet have a means to represent
- // multiple matches or multiple replacements on a single line.
-
- return true
- })
-}
-
func receiverType(fn *ast.FuncDecl) string {
switch e := fn.Recv.List[0].Type.(type) {
case *ast.Ident:
diff --git a/testdata/time.go b/testdata/time.go
index 44cda9d..9271acb 100644
--- a/testdata/time.go
+++ b/testdata/time.go
@@ -11,32 +11,3 @@
var rpcTimeoutMsec = flag.Duration("rpc_timeout", 100*time.Millisecond, "some flag") // MATCH /Msec.*\*time.Duration/
var timeoutSecs = 5 * time.Second // MATCH /Secs.*time.Duration/
-
-var delay = flag.Duration("delay", 5, "wait impatiently") // MATCH /time.*expression.*"5".*constants/
-
-func f(...time.Duration) {}
-
-var (
- _ = time.Duration(15) // MATCH /time.*expression.*"15".*constants/
- _ = time.Duration(15) + time.Duration(30) // Not caught, but rare in real code; see comments in lintDurationUnits
- _ = []time.Duration{
- 1, // MATCH /time.*expression.*"1".*constants/
- 10, // MATCH /time.*expression.*"10".*constants/
- 1 * time.Nanosecond,
- 10 * 10 * 10 * 10,
- time.Hour,
- 0, // 0 is OK
- }
- abc = f(17) // MATCH /time.*expression.*"17".*constants/
-)
-
-const x = 10
-
-func g() {
- time.Sleep(
- 12, // MATCH /time.*expression.*"12".*constants/
- )
- time.Sleep(x) // Not caught; see comments in lintDurationUnits
- _ = f(1, // MATCH /time.*expression.*"1".*constants/
- 1e12) // MATCH /time.*expression.*"1e12".*constants/
-}
