From 86a446890c556ac7da9ac9dfec1723b66ff63bdc Mon Sep 17 00:00:00 2001 From: Hein Oldewage Date: Sat, 11 Mar 2023 18:53:20 +0000 Subject: [PATCH] SA1019: Fix missing deprecated fields in struct initialization --- staticcheck/sa1019/sa1019.go | 89 ++++++++++++++----- .../CheckDeprecatedassist_external.go | 11 +++ .../CheckDeprecated/CheckDeprecated.go | 15 ++++ 3 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 staticcheck/sa1019/testdata/src/example.com/CheckDeprecated.assist_external/CheckDeprecatedassist_external.go diff --git a/staticcheck/sa1019/sa1019.go b/staticcheck/sa1019/sa1019.go index 0cab04ca7..8384dec6c 100644 --- a/staticcheck/sa1019/sa1019.go +++ b/staticcheck/sa1019/sa1019.go @@ -50,7 +50,7 @@ func run(pass *analysis.Pass) (interface{}, error) { return !strings.Contains(path, ".") } - handleDeprecation := func(depr *deprecated.IsDeprecated, node ast.Node, deprecatedObjName string, pkgPath string, tfn types.Object) { + handleDeprecation := func(depr *deprecated.IsDeprecated, node ast.Node, deprecatedObjName string, pkgPath string, tfn types.Object, nodeRender func() string) { std, ok := knowledge.StdlibDeprecations[deprecatedObjName] if !ok && isStdlibPath(pkgPath) { // Deprecated object in the standard library, but we don't know the details of the deprecation. @@ -105,32 +105,20 @@ func run(pass *analysis.Pass) (interface{}, error) { report.Render(pass, node), std.DeprecatedSince, std.AlternativeAvailableSince, depr.Msg)) } } else { - report.Report(pass, node, fmt.Sprintf("%s is deprecated: %s", report.Render(pass, node), depr.Msg)) + report.Report(pass, node, fmt.Sprintf("%s is deprecated: %s", nodeRender(), depr.Msg)) } } var tfn types.Object stack := 0 - fn := func(node ast.Node, push bool) bool { - if !push { - stack-- - return false - } - stack++ - if stack == 1 { - tfn = nil - } - if fn, ok := node.(*ast.FuncDecl); ok { - tfn = pass.TypesInfo.ObjectOf(fn.Name) - } - // FIXME(dh): this misses dot-imported objects - sel, ok := node.(*ast.SelectorExpr) - if !ok { - return true - } - - obj := pass.TypesInfo.ObjectOf(sel.Sel) + checkIdentObj := func( + node ast.Node, + id *ast.Ident, + obj types.Object, + nameFunc func() string, + nodeRender func() string, + ) bool { if obj_, ok := obj.(*types.Func); ok { obj = obj_.Origin() } @@ -161,7 +149,60 @@ func run(pass *analysis.Pass) (interface{}, error) { } if depr, ok := deprs.Objects[obj]; ok { - handleDeprecation(depr, sel, code.SelectorName(pass, sel), obj.Pkg().Path(), tfn) + handleDeprecation(depr, node, nameFunc(), obj.Pkg().Path(), tfn, nodeRender) + } + return true + } + + fn := func(node ast.Node, push bool) bool { + if !push { + stack-- + return false + } + stack++ + if stack == 1 { + tfn = nil + } + if fn, ok := node.(*ast.FuncDecl); ok { + tfn = pass.TypesInfo.ObjectOf(fn.Name) + } + + switch v := node.(type) { + // FIXME(dh): this misses dot-imported objects + case *ast.SelectorExpr: + sel := v + obj := pass.TypesInfo.ObjectOf(sel.Sel) + return checkIdentObj(sel, sel.Sel, obj, func() string { + return code.SelectorName(pass, sel) + }, func() string { + return report.Render(pass, sel) + }) + + case *ast.CompositeLit: + for _, elt := range v.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + return true + } + key, ok := kv.Key.(*ast.Ident) + // ast.KeyValueExpr also represents key value pairs in maps, where the `Key` can be a *ast.BasicLit + if !ok { + return true + } + obj := pass.TypesInfo.ObjectOf(key) + checkIdentObj(key, key, obj, func() string { + return key.Name + }, func() string { + if se, ok := v.Type.(*ast.SelectorExpr); ok { + if xI, ok := se.X.(*ast.Ident); ok { + return fmt.Sprintf("%s.%s.%s", xI.Name, se.Sel.Name, key.Name) + } else { + return fmt.Sprintf("%s.%s", se.Sel.Name, key.Name) + } + } + return key.Name + }) + } } return true } @@ -198,7 +239,9 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - handleDeprecation(depr, spec.Path, path, path, nil) + handleDeprecation(depr, spec.Path, path, path, nil, func() string { + return report.Render(pass, node) + }) } } pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes(nil, fn) diff --git a/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated.assist_external/CheckDeprecatedassist_external.go b/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated.assist_external/CheckDeprecatedassist_external.go new file mode 100644 index 000000000..cc33e9ad2 --- /dev/null +++ b/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated.assist_external/CheckDeprecatedassist_external.go @@ -0,0 +1,11 @@ +package pkg + +type SD struct { + // Deprecated: external don't use me + D string +} + +type SN struct { + // Not deprecated, but named the same + D string +} diff --git a/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go b/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go index a28673353..bd8edd73c 100644 --- a/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go +++ b/staticcheck/sa1019/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go @@ -5,6 +5,7 @@ import _ "example.com/CheckDeprecated.assist" //@ diag(`Alas, it is dep import _ "example.com/AnotherCheckDeprecated.assist" //@ diag(`Alas, it is deprecated.`) import foo "example.com/AnotherCheckDeprecated.assist" //@ diag(`Alas, it is deprecated.`) import "example.com/AnotherCheckDeprecated.assist" //@ diag(`Alas, it is deprecated.`) +import ae "example.com/CheckDeprecated.assist_external" func init() { foo.Fn() @@ -13,6 +14,20 @@ func init() { // Field is deprecated, but we're using it from the same package, which is fine. var s S _ = s.Field + + s2 := ae.SD{ + D: "used", //@ diag(`external don't use me`) + } + _ = s2 + // Struct with the same name should not be flagged + s3 := ae.SN{ + D:"also", + } + _ = s3 + // Other Key-Value expressions should be safely ignored + _ = map[string]string { + "left":"right", + } }