diff --git a/simple/s1012/s1012.go b/simple/s1012/s1012.go index 33edd55b8..560fdb638 100644 --- a/simple/s1012/s1012.go +++ b/simple/s1012/s1012.go @@ -1,6 +1,8 @@ package s1012 import ( + "go/ast" + "honnef.co/go/tools/analysis/code" "honnef.co/go/tools/analysis/edit" "honnef.co/go/tools/analysis/facts/generated" @@ -36,11 +38,23 @@ var ( ) func run(pass *analysis.Pass) (any, error) { - for node, m := range code.Matches(pass, checkTimeSinceQ) { - edits := code.EditMatch(pass, node, m, checkTimeSinceR) + for node := range code.Matches(pass, checkTimeSinceQ) { + call := node.(*ast.CallExpr) + sub := call.Fun.(*ast.SelectorExpr) + now := sub.X.(*ast.CallExpr) + selector, ok := ast.Unparen(now.Fun).(*ast.SelectorExpr) + if !ok { + report.Report(pass, node, "should use time.Since instead of time.Now().Sub", report.FilterGenerated()) + continue + } + time := selector.X + replacement := &ast.CallExpr{ + Fun: &ast.SelectorExpr{X: time, Sel: ast.NewIdent("Since")}, + Args: call.Args, + } report.Report(pass, node, "should use time.Since instead of time.Now().Sub", report.FilterGenerated(), - report.Fixes(edit.Fix("Replace with call to time.Since", edits...))) + report.Fixes(edit.Fix("Replace with call to time.Since", edit.ReplaceWithNode(pass.Fset, node, replacement)))) } return nil, nil } diff --git a/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go b/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go new file mode 100644 index 000000000..5a9d93a10 --- /dev/null +++ b/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go @@ -0,0 +1,8 @@ +package pkg + +import timelib "time" + +func fn() { + t1 := timelib.Now() + _ = timelib.Now().Sub(t1) //@ diag(`time.Since`) +} diff --git a/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go.golden b/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go.golden new file mode 100644 index 000000000..7ab52a39e --- /dev/null +++ b/simple/s1012/testdata/go1.0/CheckTimeSinceAlias/time-since-alias.go.golden @@ -0,0 +1,8 @@ +package pkg + +import timelib "time" + +func fn() { + t1 := timelib.Now() + _ = timelib.Since(t1) //@ diag(`time.Since`) +} diff --git a/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go b/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go new file mode 100644 index 000000000..81610b3f4 --- /dev/null +++ b/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn(t Time) { + _ = Now().Sub(t) //@ diag(`time.Since`) +} diff --git a/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go.golden b/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go.golden new file mode 100644 index 000000000..81610b3f4 --- /dev/null +++ b/simple/s1012/testdata/go1.0/CheckTimeSinceDotImport/dot.go.golden @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn(t Time) { + _ = Now().Sub(t) //@ diag(`time.Since`) +} diff --git a/simple/s1024/s1024.go b/simple/s1024/s1024.go index 82d3730d6..5cacf80d2 100644 --- a/simple/s1024/s1024.go +++ b/simple/s1024/s1024.go @@ -34,22 +34,39 @@ var Analyzer = SCAnalyzer.Analyzer var ( checkTimeUntilQ = pattern.MustParse(`(CallExpr (Symbol "(time.Time).Sub") [(CallExpr (Symbol "time.Now") [])])`) - checkTimeUntilR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "time") (Ident "Until")) [arg])`) ) func run(pass *analysis.Pass) (any, error) { for node := range code.Matches(pass, checkTimeUntilQ) { - if sel, ok := node.(*ast.CallExpr).Fun.(*ast.SelectorExpr); ok { - r := pattern.NodeToAST(checkTimeUntilR.Root, map[string]any{"arg": sel.X}).(ast.Node) + call, ok := node.(*ast.CallExpr) + if !ok { report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())", - report.FilterGenerated(), report.MinimumStdlibVersion("go1.8"), - report.Fixes(edit.Fix("Replace with call to time.Until", edit.ReplaceWithNode(pass.Fset, node, r)))) - } else { + report.FilterGenerated()) + continue + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())", report.MinimumStdlibVersion("go1.8"), report.FilterGenerated()) + continue + } + timeNow := call.Args[0].(*ast.CallExpr) + nowSelector, ok := ast.Unparen(timeNow.Fun).(*ast.SelectorExpr) + if !ok { + report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())", report.MinimumStdlibVersion("go1.8"), report.FilterGenerated()) + continue + } + timePkg := nowSelector.X + replacement := &ast.CallExpr{ + Fun: &ast.SelectorExpr{X: timePkg, Sel: ast.NewIdent("Until")}, + Args: []ast.Expr{selector.X}, } + report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())", + report.FilterGenerated(), + report.MinimumStdlibVersion("go1.8"), + report.Fixes(edit.Fix("Replace with call to time.Until", edit.ReplaceWithNode(pass.Fset, node, replacement)))) } return nil, nil } diff --git a/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go b/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go new file mode 100644 index 000000000..2f066f6e5 --- /dev/null +++ b/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go @@ -0,0 +1,7 @@ +package pkg + +import timelib "time" + +func fn(t timelib.Time) { + t.Sub(timelib.Now()) //@ diag(`time.Until`) +} diff --git a/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go.golden b/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go.golden new file mode 100644 index 000000000..ede404cf6 --- /dev/null +++ b/simple/s1024/testdata/go1.8/CheckTimeUntilAlias/time-until-alias.go.golden @@ -0,0 +1,7 @@ +package pkg + +import timelib "time" + +func fn(t timelib.Time) { + timelib.Until(t) //@ diag(`time.Until`) +} diff --git a/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go b/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go new file mode 100644 index 000000000..e8eef82ca --- /dev/null +++ b/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn(t Time) { + _ = t.Sub(Now()) //@ diag(`time.Until`) +} diff --git a/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go.golden b/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go.golden new file mode 100644 index 000000000..e8eef82ca --- /dev/null +++ b/simple/s1024/testdata/go1.8/CheckTimeUntilDotImport/dot.go.golden @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn(t Time) { + _ = t.Sub(Now()) //@ diag(`time.Until`) +} diff --git a/simple/s1037/s1037.go b/simple/s1037/s1037.go index a067c1ab7..ce907f5dc 100644 --- a/simple/s1037/s1037.go +++ b/simple/s1037/s1037.go @@ -32,17 +32,27 @@ can much simpler be expressed with a simple call to time.Sleep.`, var Analyzer = SCAnalyzer.Analyzer var ( - checkElaborateSleepQ = pattern.MustParse(`(SelectStmt (CommClause (UnaryExpr "<-" (CallExpr (Symbol "time.After") [arg])) body))`) + checkElaborateSleepQ = pattern.MustParse(`(SelectStmt (CommClause (UnaryExpr "<-" afterCall@(CallExpr (Symbol "time.After") [arg])) body))`) checkElaborateSleepR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "time") (Ident "Sleep")) [arg])`) ) func run(pass *analysis.Pass) (any, error) { for node, m := range code.Matches(pass, checkElaborateSleepQ) { if body, ok := m.State["body"].([]ast.Stmt); ok && len(body) == 0 { + selector, ok := ast.Unparen(m.State["afterCall"].(*ast.CallExpr).Fun).(*ast.SelectorExpr) + if !ok { + report.Report(pass, node, "should use time.Sleep instead of elaborate way of sleeping", report.ShortRange(), report.FilterGenerated()) + continue + } + time := selector.X + replacement := &ast.CallExpr{ + Fun: &ast.SelectorExpr{X: time, Sel: ast.NewIdent("Sleep")}, + Args: []ast.Expr{m.State["arg"].(ast.Expr)}, + } report.Report(pass, node, "should use time.Sleep instead of elaborate way of sleeping", report.ShortRange(), report.FilterGenerated(), - report.Fixes(edit.Fix("Use time.Sleep", edit.ReplaceWithPattern(pass.Fset, node, checkElaborateSleepR, m.State)))) + report.Fixes(edit.Fix("Use time.Sleep", edit.ReplaceWithNode(pass.Fset, node, replacement)))) } else { // TODO(dh): we could make a suggested fix if the body // doesn't declare or shadow any identifiers diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go b/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go new file mode 100644 index 000000000..f1ff670a7 --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go @@ -0,0 +1,9 @@ +package pkg + +import timelib "time" + +func fn() { + select { //@ diag(`should use time.Sleep`) + case <-timelib.After(0): + } +} diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go.golden b/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go.golden new file mode 100644 index 000000000..03f3c6fa1 --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepAlias/elaborate-sleep-alias.go.golden @@ -0,0 +1,7 @@ +package pkg + +import timelib "time" + +func fn() { + timelib.Sleep(0) +} diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go b/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go new file mode 100644 index 000000000..ac6b8fccc --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go @@ -0,0 +1,9 @@ +package pkg + +import . "time" + +func fn() { + select { //@ diag(`should use time.Sleep`) + case <-After(0): + } +} diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go.golden b/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go.golden new file mode 100644 index 000000000..ac6b8fccc --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepDotImport/dot.go.golden @@ -0,0 +1,9 @@ +package pkg + +import . "time" + +func fn() { + select { //@ diag(`should use time.Sleep`) + case <-After(0): + } +} diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go b/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go new file mode 100644 index 000000000..7b77e9ce6 --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go @@ -0,0 +1,9 @@ +package pkg + +import t "time" + +func fn(x struct{ After func() t.Duration }) { + select { //@ diag(`should use time.Sleep`) + case <-t.After(x.After()): + } +} diff --git a/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go.golden b/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go.golden new file mode 100644 index 000000000..2af30d693 --- /dev/null +++ b/simple/s1037/testdata/go1.0/CheckElaborateSleepNestedAfter/nested.go.golden @@ -0,0 +1,7 @@ +package pkg + +import t "time" + +func fn(x struct{ After func() t.Duration }) { + t.Sleep(x.After()) +}