diff --git a/staticcheck/sa1004/sa1004.go b/staticcheck/sa1004/sa1004.go index b3dee1621..0df038e82 100644 --- a/staticcheck/sa1004/sa1004.go +++ b/staticcheck/sa1004/sa1004.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/constant" + "go/token" "go/types" "honnef.co/go/tools/analysis/code" @@ -43,14 +44,10 @@ for some amount of nanoseconds.`, var Analyzer = SCAnalyzer.Analyzer -var ( - checkTimeSleepConstantPatternQ = pattern.MustParse(`(CallExpr (Symbol "time.Sleep") lit@(IntegerLiteral value))`) - checkTimeSleepConstantPatternRns = pattern.MustParse(`(BinaryExpr duration "*" (SelectorExpr (Ident "time") (Ident "Nanosecond")))`) - checkTimeSleepConstantPatternRs = pattern.MustParse(`(BinaryExpr duration "*" (SelectorExpr (Ident "time") (Ident "Second")))`) -) +var checkTimeSleepConstantPatternQ = pattern.MustParse(`(CallExpr (Symbol "time.Sleep") lit@(IntegerLiteral value))`) func run(pass *analysis.Pass) (any, error) { - for _, m := range code.Matches(pass, checkTimeSleepConstantPatternQ) { + for callNode, m := range code.Matches(pass, checkTimeSleepConstantPatternQ) { n, ok := constant.Int64Val(m.State["value"].(types.TypeAndValue).Value) if !ok { continue @@ -62,11 +59,20 @@ func run(pass *analysis.Pass) (any, error) { continue } - lit := m.State["lit"].(ast.Node) + lit := m.State["lit"].(ast.Expr) + call := callNode.(*ast.CallExpr) + selector, ok := ast.Unparen(call.Fun).(*ast.SelectorExpr) + if !ok { + report.Report(pass, lit, fmt.Sprintf("sleeping for %d nanoseconds is probably a bug; be explicit if it isn't", n)) + continue + } + pkg := selector.X + oneNS := &ast.BinaryExpr{X: lit, Op: token.MUL, Y: &ast.SelectorExpr{X: pkg, Sel: ast.NewIdent("Nanosecond")}} + oneS := &ast.BinaryExpr{X: lit, Op: token.MUL, Y: &ast.SelectorExpr{X: pkg, Sel: ast.NewIdent("Second")}} report.Report(pass, lit, fmt.Sprintf("sleeping for %d nanoseconds is probably a bug; be explicit if it isn't", n), report.Fixes( - edit.Fix("Explicitly use nanoseconds", edit.ReplaceWithPattern(pass.Fset, lit, checkTimeSleepConstantPatternRns, pattern.State{"duration": lit})), - edit.Fix("Use seconds", edit.ReplaceWithPattern(pass.Fset, lit, checkTimeSleepConstantPatternRs, pattern.State{"duration": lit})))) + edit.Fix("Explicitly use nanoseconds", edit.ReplaceWithNode(pass.Fset, lit, oneNS)), + edit.Fix("Use seconds", edit.ReplaceWithNode(pass.Fset, lit, oneS)))) } return nil, nil } diff --git a/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go new file mode 100644 index 000000000..e4fc76a5b --- /dev/null +++ b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go @@ -0,0 +1,16 @@ +package pkg + +import timelib "time" + +const c1 = 1 +const c2 = 200 + +func fn() { + timelib.Sleep(1) //@ diag(`sleeping for 1`) + timelib.Sleep(42) //@ diag(`sleeping for 42`) + timelib.Sleep(201) + timelib.Sleep(c1) + timelib.Sleep(c2) + timelib.Sleep(2 * timelib.Nanosecond) + timelib.Sleep(timelib.Nanosecond) +} diff --git a/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go.golden b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go.golden new file mode 100644 index 000000000..8941a488a --- /dev/null +++ b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantAlias/CheckTimeSleepConstantAlias.go.golden @@ -0,0 +1,35 @@ +-- Explicitly use nanoseconds -- +package pkg + +import timelib "time" + +const c1 = 1 +const c2 = 200 + +func fn() { + timelib.Sleep(1 * timelib.Nanosecond) //@ diag(`sleeping for 1`) + timelib.Sleep(42 * timelib.Nanosecond) //@ diag(`sleeping for 42`) + timelib.Sleep(201) + timelib.Sleep(c1) + timelib.Sleep(c2) + timelib.Sleep(2 * timelib.Nanosecond) + timelib.Sleep(timelib.Nanosecond) +} + +-- Use seconds -- +package pkg + +import timelib "time" + +const c1 = 1 +const c2 = 200 + +func fn() { + timelib.Sleep(1 * timelib.Second) //@ diag(`sleeping for 1`) + timelib.Sleep(42 * timelib.Second) //@ diag(`sleeping for 42`) + timelib.Sleep(201) + timelib.Sleep(c1) + timelib.Sleep(c2) + timelib.Sleep(2 * timelib.Nanosecond) + timelib.Sleep(timelib.Nanosecond) +} diff --git a/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go new file mode 100644 index 000000000..92c0aeae3 --- /dev/null +++ b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn() { + Sleep(1) //@ diag(`sleeping for 1`) +} diff --git a/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go.golden b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go.golden new file mode 100644 index 000000000..92c0aeae3 --- /dev/null +++ b/staticcheck/sa1004/testdata/go1.0/CheckTimeSleepConstantDotImport/scope.go.golden @@ -0,0 +1,7 @@ +package pkg + +import . "time" + +func fn() { + Sleep(1) //@ diag(`sleeping for 1`) +} diff --git a/staticcheck/sa1008/sa1008.go b/staticcheck/sa1008/sa1008.go index 3e0d1bb54..913c432e4 100644 --- a/staticcheck/sa1008/sa1008.go +++ b/staticcheck/sa1008/sa1008.go @@ -3,6 +3,7 @@ package sa1008 import ( "fmt" "go/ast" + "go/types" "net/http" "strconv" @@ -92,11 +93,13 @@ func run(pass *analysis.Pass) (any, error) { case *ast.BasicLit: fix = edit.Fix("Canonicalize header key", edit.ReplaceWithString(op.Index, strconv.Quote(canonical))) case *ast.Ident: - call := &ast.CallExpr{ - Fun: edit.Selector("http", "CanonicalHeaderKey"), - Args: []ast.Expr{op.Index}, + if name := importAlias(pass, op, "net/http"); name != "" { + call := &ast.CallExpr{ + Fun: edit.Selector(name, "CanonicalHeaderKey"), + Args: []ast.Expr{op.Index}, + } + fix = edit.Fix("Wrap in http.CanonicalHeaderKey", edit.ReplaceWithNode(pass.Fset, op.Index, call)) } - fix = edit.Fix("Wrap in http.CanonicalHeaderKey", edit.ReplaceWithNode(pass.Fset, op.Index, call)) } msg := fmt.Sprintf("keys in http.Header are canonicalized, %q is not canonical; fix the constant or use http.CanonicalHeaderKey", s) if fix.Message != "" { @@ -109,3 +112,30 @@ func run(pass *analysis.Pass) (any, error) { pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.AssignStmt)(nil), (*ast.IndexExpr)(nil)}, fn) return nil, nil } + +func importAlias(pass *analysis.Pass, node ast.Node, importPath string) string { + for _, spec := range code.File(pass, node).Imports { + p, err := strconv.Unquote(spec.Path.Value) + if err != nil || p != importPath { + continue + } + var pkgName *types.PkgName + if spec.Name == nil { + pkgName, _ = pass.TypesInfo.Implicits[spec].(*types.PkgName) + } else { + pkgName, _ = pass.TypesInfo.Defs[spec.Name].(*types.PkgName) + } + if pkgName == nil || pkgName.Name() == "." || pkgName.Name() == "_" { + return "" + } + scope := pass.TypesInfo.Scopes[code.File(pass, node)].Innermost(node.Pos()) + if scope != nil { + _, obj := scope.LookupParent(pkgName.Name(), node.Pos()) + if obj == pkgName { + return pkgName.Name() + } + } + return "" + } + return "" +} diff --git a/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go new file mode 100644 index 000000000..daaf5fdbf --- /dev/null +++ b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go @@ -0,0 +1,16 @@ +package pkg + +import httpalias "net/http" + +func fn() { + const hdr = "foo" + var r httpalias.Request + h := httpalias.Header{} + var m map[string][]string + _ = h["foo"] //@ diag(`keys in http.Header are canonicalized`) + _ = h[hdr] //@ diag(`keys in http.Header are canonicalized`) + h["foo"] = nil + _ = r.Header["foo"] //@ diag(`keys in http.Header are canonicalized`) + r.Header["foo"] = nil + _ = m["foo"] +} diff --git a/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go.golden b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go.golden new file mode 100644 index 000000000..fcedce569 --- /dev/null +++ b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyAlias/CheckCanonicalHeaderKeyAlias.go.golden @@ -0,0 +1,16 @@ +package pkg + +import httpalias "net/http" + +func fn() { + const hdr = "foo" + var r httpalias.Request + h := httpalias.Header{} + var m map[string][]string + _ = h["Foo"] //@ diag(`keys in http.Header are canonicalized`) + _ = h[httpalias.CanonicalHeaderKey(hdr)] //@ diag(`keys in http.Header are canonicalized`) + h["foo"] = nil + _ = r.Header["Foo"] //@ diag(`keys in http.Header are canonicalized`) + r.Header["foo"] = nil + _ = m["foo"] +} diff --git a/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go new file mode 100644 index 000000000..04f537e83 --- /dev/null +++ b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go @@ -0,0 +1,11 @@ +package pkg + +import htp "net/http" + +func fn() { + h := htp.Header{} + htp := 1 + _ = htp + const hdr = "foo" + _ = h[hdr] //@ diag(`keys in http.Header are canonicalized`) +} diff --git a/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go.golden b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go.golden new file mode 100644 index 000000000..04f537e83 --- /dev/null +++ b/staticcheck/sa1008/testdata/go1.0/CheckCanonicalHeaderKeyShadow/scope.go.golden @@ -0,0 +1,11 @@ +package pkg + +import htp "net/http" + +func fn() { + h := htp.Header{} + htp := 1 + _ = htp + const hdr = "foo" + _ = h[hdr] //@ diag(`keys in http.Header are canonicalized`) +} diff --git a/staticcheck/sa1012/sa1012.go b/staticcheck/sa1012/sa1012.go index b04fd8c88..a67e19f64 100644 --- a/staticcheck/sa1012/sa1012.go +++ b/staticcheck/sa1012/sa1012.go @@ -3,6 +3,7 @@ package sa1012 import ( "go/ast" "go/types" + "strconv" "honnef.co/go/tools/analysis/code" "honnef.co/go/tools/analysis/edit" @@ -36,12 +37,6 @@ var Analyzer = SCAnalyzer.Analyzer var checkNilContextQ = pattern.MustParse(`(CallExpr fun@(Symbol _) (Builtin "nil"):_)`) func run(pass *analysis.Pass) (any, error) { - todo := &ast.CallExpr{ - Fun: edit.Selector("context", "TODO"), - } - bg := &ast.CallExpr{ - Fun: edit.Selector("context", "Background"), - } for node, m := range code.Matches(pass, checkNilContextQ) { call := node.(*ast.CallExpr) fun, ok := m.State["fun"].(*types.Func) @@ -59,10 +54,44 @@ func run(pass *analysis.Pass) (any, error) { if !typeutil.IsTypeWithName(sig.Params().At(0).Type(), "context.Context") { continue } - report.Report(pass, call.Args[0], - "do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use", report.Fixes( + options := []report.Option(nil) + if contextAlias := importAlias(pass, call, "context"); contextAlias != "" { + todo := &ast.CallExpr{Fun: edit.Selector(contextAlias, "TODO")} + bg := &ast.CallExpr{Fun: edit.Selector(contextAlias, "Background")} + options = append(options, report.Fixes( edit.Fix("Use context.TODO", edit.ReplaceWithNode(pass.Fset, call.Args[0], todo)), edit.Fix("Use context.Background", edit.ReplaceWithNode(pass.Fset, call.Args[0], bg)))) + } + report.Report(pass, call.Args[0], + "do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use", options...) } return nil, nil } + +func importAlias(pass *analysis.Pass, node ast.Node, path string) string { + file := code.File(pass, node) + for _, spec := range file.Imports { + importPath, err := strconv.Unquote(spec.Path.Value) + if err != nil || importPath != path { + continue + } + var pkgName *types.PkgName + if spec.Name == nil { + pkgName, _ = pass.TypesInfo.Implicits[spec].(*types.PkgName) + } else { + pkgName, _ = pass.TypesInfo.Defs[spec.Name].(*types.PkgName) + } + if pkgName == nil || pkgName.Name() == "." || pkgName.Name() == "_" { + return "" + } + scope := pass.TypesInfo.Scopes[file].Innermost(node.Pos()) + if scope != nil { + _, obj := scope.LookupParent(pkgName.Name(), node.Pos()) + if obj == pkgName { + return pkgName.Name() + } + } + return "" + } + return "" +} diff --git a/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go b/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go new file mode 100644 index 000000000..b477fb919 --- /dev/null +++ b/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go @@ -0,0 +1,9 @@ +package pkg + +import ctx "context" + +func fn(c ctx.Context) {} + +func call() { + fn(nil) //@ diag(`do not pass a nil Context`) +} diff --git a/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go.golden b/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go.golden new file mode 100644 index 000000000..996ed0f8b --- /dev/null +++ b/staticcheck/sa1012/testdata/go1.0/CheckNilContextAlias/nil-context-alias.go.golden @@ -0,0 +1,20 @@ +-- Use context.Background -- +package pkg + +import ctx "context" + +func fn(c ctx.Context) {} + +func call() { + fn(ctx.Background()) //@ diag(`do not pass a nil Context`) +} +-- Use context.TODO -- +package pkg + +import ctx "context" + +func fn(c ctx.Context) {} + +func call() { + fn(ctx.TODO()) //@ diag(`do not pass a nil Context`) +} diff --git a/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go b/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go new file mode 100644 index 000000000..2234a3d41 --- /dev/null +++ b/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go @@ -0,0 +1,11 @@ +package pkg + +import ctx "context" + +func fn(c ctx.Context) {} + +func call() { + ctx := 1 + _ = ctx + fn(nil) //@ diag(`do not pass a nil Context`) +} diff --git a/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go.golden b/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go.golden new file mode 100644 index 000000000..2234a3d41 --- /dev/null +++ b/staticcheck/sa1012/testdata/go1.0/CheckNilContextShadow/scope.go.golden @@ -0,0 +1,11 @@ +package pkg + +import ctx "context" + +func fn(c ctx.Context) {} + +func call() { + ctx := 1 + _ = ctx + fn(nil) //@ diag(`do not pass a nil Context`) +} diff --git a/staticcheck/sa1016/sa1016.go b/staticcheck/sa1016/sa1016.go index bf89f6cc0..6d8a6ef8b 100644 --- a/staticcheck/sa1016/sa1016.go +++ b/staticcheck/sa1016/sa1016.go @@ -3,6 +3,8 @@ package sa1016 import ( "fmt" "go/ast" + "go/types" + "strconv" "honnef.co/go/tools/analysis/code" "honnef.co/go/tools/analysis/edit" @@ -73,17 +75,19 @@ func run(pass *analysis.Pass) (any, error) { if isSignal(pass, arg, "os.Kill") || isSignal(pass, arg, "syscall.SIGKILL") { var fixes []analysis.SuggestedFix if !hasSigterm { - nargs := make([]ast.Expr, len(call.Args)) - for j, a := range call.Args { - if i == j { - nargs[j] = edit.Selector("syscall", "SIGTERM") - } else { - nargs[j] = a + if syscallAlias := importAlias(pass, call, "syscall"); syscallAlias != "" { + nargs := make([]ast.Expr, len(call.Args)) + for j, a := range call.Args { + if i == j { + nargs[j] = edit.Selector(syscallAlias, "SIGTERM") + } else { + nargs[j] = a + } } + ncall := *call + ncall.Args = nargs + fixes = append(fixes, edit.Fix(fmt.Sprintf("Use syscall.SIGTERM instead of %s", report.Render(pass, arg)), edit.ReplaceWithNode(pass.Fset, call, &ncall))) } - ncall := *call - ncall.Args = nargs - fixes = append(fixes, edit.Fix(fmt.Sprintf("Use syscall.SIGTERM instead of %s", report.Render(pass, arg)), edit.ReplaceWithNode(pass.Fset, call, &ncall))) } nargs := make([]ast.Expr, 0, len(call.Args)) for j, a := range call.Args { @@ -113,3 +117,30 @@ func run(pass *analysis.Pass) (any, error) { } return nil, nil } + +func importAlias(pass *analysis.Pass, node ast.Node, importPath string) string { + for _, spec := range code.File(pass, node).Imports { + p, err := strconv.Unquote(spec.Path.Value) + if err != nil || p != importPath { + continue + } + var pkgName *types.PkgName + if spec.Name == nil { + pkgName, _ = pass.TypesInfo.Implicits[spec].(*types.PkgName) + } else { + pkgName, _ = pass.TypesInfo.Defs[spec.Name].(*types.PkgName) + } + if pkgName == nil || pkgName.Name() == "." || pkgName.Name() == "_" { + return "" + } + scope := pass.TypesInfo.Scopes[code.File(pass, node)].Innermost(node.Pos()) + if scope != nil { + _, obj := scope.LookupParent(pkgName.Name(), node.Pos()) + if obj == pkgName { + return pkgName.Name() + } + } + return "" + } + return "" +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go new file mode 100644 index 000000000..73db099a1 --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go @@ -0,0 +1,11 @@ +package pkg + +import ( + "os" + "os/signal" + system "syscall" +) + +func fn(c chan os.Signal) { + signal.Notify(c, system.SIGKILL) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go.golden b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go.golden new file mode 100644 index 000000000..09a662721 --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/CheckUntrappableSignalAlias.go.golden @@ -0,0 +1,25 @@ +-- Remove system.SIGKILL from list of arguments -- +package pkg + +import ( + "os" + "os/signal" + system "syscall" +) + +func fn(c chan os.Signal) { + signal.Notify(c) //@ diag(`cannot be trapped`) +} + +-- Use syscall.SIGTERM instead of system.SIGKILL -- +package pkg + +import ( + "os" + "os/signal" + system "syscall" +) + +func fn(c chan os.Signal) { + signal.Notify(c, system.SIGTERM) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go new file mode 100644 index 000000000..9f8b0ac2b --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go @@ -0,0 +1,10 @@ +package pkg + +import ( + "os" + "os/signal" +) + +func fn2(c chan os.Signal) { + signal.Notify(c, os.Kill) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go.golden b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go.golden new file mode 100644 index 000000000..24e51ca96 --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalAlias/os-kill-without-syscall.go.golden @@ -0,0 +1,11 @@ +-- Remove os.Kill from list of arguments -- +package pkg + +import ( + "os" + "os/signal" +) + +func fn2(c chan os.Signal) { + signal.Notify(c) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go new file mode 100644 index 000000000..78d288945 --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go @@ -0,0 +1,15 @@ +package pkg + +import ( + "os" + "os/signal" + sys "syscall" +) + +var _ = sys.SIGTERM + +func fn(c chan os.Signal) { + sys := 1 + _ = sys + signal.Notify(c, os.Kill) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go.golden b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go.golden new file mode 100644 index 000000000..5358d8721 --- /dev/null +++ b/staticcheck/sa1016/testdata/go1.0/CheckUntrappableSignalShadow/scope.go.golden @@ -0,0 +1,15 @@ +package pkg + +import ( + "os" + "os/signal" + sys "syscall" +) + +var _ = sys.SIGTERM + +func fn(c chan os.Signal) { + sys := 1 + _ = sys + signal.Notify(c) //@ diag(`cannot be trapped`) +} diff --git a/staticcheck/sa6005/sa6005.go b/staticcheck/sa6005/sa6005.go index 05cfc1cd9..d6309d7ec 100644 --- a/staticcheck/sa6005/sa6005.go +++ b/staticcheck/sa6005/sa6005.go @@ -55,12 +55,29 @@ var ( (CallExpr fun@(Symbol (Or "strings.ToLower" "strings.ToUpper")) [a]) tok@(Or "==" "!=") (CallExpr fun [b]))`) - checkToLowerToUpperComparisonR = pattern.MustParse(`(CallExpr (SelectorExpr (Ident "strings") (Ident "EqualFold")) [a b])`) ) func run(pass *analysis.Pass) (any, error) { for node, m := range code.Matches(pass, checkToLowerToUpperComparisonQ) { - rn := pattern.NodeToAST(checkToLowerToUpperComparisonR.Root, m.State).(ast.Expr) + comparison := node.(*ast.BinaryExpr) + leftCall := comparison.X.(*ast.CallExpr) + selector, ok := ast.Unparen(leftCall.Fun).(*ast.SelectorExpr) + if !ok { + method := "strings.EqualFold" + if m.State["tok"].(token.Token) == token.NEQ { + method = "!" + method + } + report.Report(pass, node, fmt.Sprintf("should use %s instead", method)) + continue + } + stringsPackage := selector.X + var rn ast.Expr = &ast.CallExpr{ + Fun: &ast.SelectorExpr{X: stringsPackage, Sel: ast.NewIdent("EqualFold")}, + Args: []ast.Expr{ + m.State["a"].(ast.Expr), + m.State["b"].(ast.Expr), + }, + } method := "strings.EqualFold" if m.State["tok"].(token.Token) == token.NEQ { rn = &ast.UnaryExpr{ diff --git a/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go new file mode 100644 index 000000000..9f7fa5a5d --- /dev/null +++ b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go @@ -0,0 +1,43 @@ +package pkg + +import stringsalias "strings" + +func fn() { + const ( + s1 = "foo" + s2 = "bar" + ) + + if stringsalias.ToLower(s1) == stringsalias.ToLower(s2) { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if stringsalias.ToUpper(s1) == stringsalias.ToUpper(s2) { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if stringsalias.ToLower(s1) != stringsalias.ToLower(s2) { //@ diag(`should use !strings.EqualFold instead`) + panic("") + } + + switch stringsalias.ToLower(s1) == stringsalias.ToLower(s2) { //@ diag(`should use strings.EqualFold instead`) + case true, false: + panic("") + } + + if stringsalias.ToLower(s1) == stringsalias.ToLower(s2) || s1+s2 == s2+s1 { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if stringsalias.ToLower(s1) > stringsalias.ToLower(s2) { + panic("") + } + + if stringsalias.ToLower(s1) < stringsalias.ToLower(s2) { + panic("") + } + + if stringsalias.ToLower(s1) == stringsalias.ToUpper(s2) { + panic("") + } +} diff --git a/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go.golden b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go.golden new file mode 100644 index 000000000..a3f3afe54 --- /dev/null +++ b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonAlias/CheckToLowerToUpperComparisonAlias.go.golden @@ -0,0 +1,43 @@ +package pkg + +import stringsalias "strings" + +func fn() { + const ( + s1 = "foo" + s2 = "bar" + ) + + if stringsalias.EqualFold(s1, s2) { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if stringsalias.EqualFold(s1, s2) { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if !stringsalias.EqualFold(s1, s2) { //@ diag(`should use !strings.EqualFold instead`) + panic("") + } + + switch stringsalias.EqualFold(s1, s2) { //@ diag(`should use strings.EqualFold instead`) + case true, false: + panic("") + } + + if stringsalias.EqualFold(s1, s2) || s1+s2 == s2+s1 { //@ diag(`should use strings.EqualFold instead`) + panic("") + } + + if stringsalias.ToLower(s1) > stringsalias.ToLower(s2) { + panic("") + } + + if stringsalias.ToLower(s1) < stringsalias.ToLower(s2) { + panic("") + } + + if stringsalias.ToLower(s1) == stringsalias.ToUpper(s2) { + panic("") + } +} diff --git a/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go new file mode 100644 index 000000000..977c08957 --- /dev/null +++ b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go @@ -0,0 +1,8 @@ +package pkg + +import . "strings" + +func fn(a, b string) { + _ = ToLower(a) == ToLower(b) //@ diag(`should use strings.EqualFold`) + _ = ToUpper(a) != ToUpper(b) //@ diag(`should use !strings.EqualFold`) +} diff --git a/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go.golden b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go.golden new file mode 100644 index 000000000..977c08957 --- /dev/null +++ b/staticcheck/sa6005/testdata/go1.0/CheckToLowerToUpperComparisonDotImport/scope.go.golden @@ -0,0 +1,8 @@ +package pkg + +import . "strings" + +func fn(a, b string) { + _ = ToLower(a) == ToLower(b) //@ diag(`should use strings.EqualFold`) + _ = ToUpper(a) != ToUpper(b) //@ diag(`should use !strings.EqualFold`) +}