Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SA1019: Fix missing deprecated fields in struct initialization #1382

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 66 additions & 23 deletions staticcheck/sa1019/sa1019.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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.
Expand Down Expand Up @@ -110,32 +110,20 @@ func run(pass *analysis.Pass) (interface{}, error) {
report.Render(pass, node), formatGoVersion(std.DeprecatedSince), formatGoVersion(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()
}
Expand Down Expand Up @@ -166,7 +154,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
}
Expand Down Expand Up @@ -203,7 +244,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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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",
}
}


Expand Down