diff --git a/go/analysis/cmd/vet-lite/main.go b/go/analysis/cmd/vet-lite/main.go index ae66a7df..70d22fbb 100644 --- a/go/analysis/cmd/vet-lite/main.go +++ b/go/analysis/cmd/vet-lite/main.go @@ -33,6 +33,7 @@ import ( "golang.org/x/tools/go/analysis/passes/stdmethods" "golang.org/x/tools/go/analysis/passes/structtag" "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/analysis/passes/unmarshal" "golang.org/x/tools/go/analysis/passes/unreachable" "golang.org/x/tools/go/analysis/passes/unsafeptr" "golang.org/x/tools/go/analysis/passes/unusedresult" @@ -59,6 +60,7 @@ var analyzers = []*analysis.Analyzer{ stdmethods.Analyzer, structtag.Analyzer, tests.Analyzer, + unmarshal.Analyzer, unreachable.Analyzer, unsafeptr.Analyzer, unusedresult.Analyzer, diff --git a/go/analysis/cmd/vet/vet.go b/go/analysis/cmd/vet/vet.go index ed075c43..3830f013 100644 --- a/go/analysis/cmd/vet/vet.go +++ b/go/analysis/cmd/vet/vet.go @@ -37,6 +37,7 @@ import ( "golang.org/x/tools/go/analysis/passes/stdmethods" "golang.org/x/tools/go/analysis/passes/structtag" "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/analysis/passes/unmarshal" "golang.org/x/tools/go/analysis/passes/unreachable" "golang.org/x/tools/go/analysis/passes/unsafeptr" "golang.org/x/tools/go/analysis/passes/unusedresult" @@ -64,6 +65,7 @@ func main() { stdmethods.Analyzer, structtag.Analyzer, tests.Analyzer, + unmarshal.Analyzer, unreachable.Analyzer, unsafeptr.Analyzer, unusedresult.Analyzer, diff --git a/go/analysis/passes/unmarshal/cmd/unmarshal/main.go b/go/analysis/passes/unmarshal/cmd/unmarshal/main.go new file mode 100644 index 00000000..993bf745 --- /dev/null +++ b/go/analysis/passes/unmarshal/cmd/unmarshal/main.go @@ -0,0 +1,9 @@ +// The unmarshal command runs the unmarshal analyzer. +package main + +import ( + "golang.org/x/tools/go/analysis/passes/unmarshal" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { singlechecker.Main(unmarshal.Analyzer) } diff --git a/go/analysis/passes/unmarshal/testdata/src/a/a.go b/go/analysis/passes/unmarshal/testdata/src/a/a.go new file mode 100644 index 00000000..5c47fe7c --- /dev/null +++ b/go/analysis/passes/unmarshal/testdata/src/a/a.go @@ -0,0 +1,58 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the unmarshal checker. + +package testdata + +import ( + "encoding/gob" + "encoding/json" + "encoding/xml" + "io" +) + +func _() { + type t struct { + a int + } + var v t + var r io.Reader + + json.Unmarshal([]byte{}, v) // want "call of Unmarshal passes non-pointer as second argument" + json.Unmarshal([]byte{}, &v) + json.NewDecoder(r).Decode(v) // want "call of Decode passes non-pointer" + json.NewDecoder(r).Decode(&v) + gob.NewDecoder(r).Decode(v) // want "call of Decode passes non-pointer" + gob.NewDecoder(r).Decode(&v) + xml.Unmarshal([]byte{}, v) // want "call of Unmarshal passes non-pointer as second argument" + xml.Unmarshal([]byte{}, &v) + xml.NewDecoder(r).Decode(v) // want "call of Decode passes non-pointer" + xml.NewDecoder(r).Decode(&v) + + var p *t + json.Unmarshal([]byte{}, p) + json.Unmarshal([]byte{}, *p) // want "call of Unmarshal passes non-pointer as second argument" + json.NewDecoder(r).Decode(p) + json.NewDecoder(r).Decode(*p) // want "call of Decode passes non-pointer" + gob.NewDecoder(r).Decode(p) + gob.NewDecoder(r).Decode(*p) // want "call of Decode passes non-pointer" + xml.Unmarshal([]byte{}, p) + xml.Unmarshal([]byte{}, *p) // want "call of Unmarshal passes non-pointer as second argument" + xml.NewDecoder(r).Decode(p) + xml.NewDecoder(r).Decode(*p) // want "call of Decode passes non-pointer" + + var i interface{} + json.Unmarshal([]byte{}, i) + json.NewDecoder(r).Decode(i) + + json.Unmarshal([]byte{}, nil) // want "call of Unmarshal passes non-pointer as second argument" + json.Unmarshal([]byte{}, []t{}) // want "call of Unmarshal passes non-pointer as second argument" + json.Unmarshal([]byte{}, map[string]int{}) // want "call of Unmarshal passes non-pointer as second argument" + json.NewDecoder(r).Decode(nil) // want "call of Decode passes non-pointer" + json.NewDecoder(r).Decode([]t{}) // want "call of Decode passes non-pointer" + json.NewDecoder(r).Decode(map[string]int{}) // want "call of Decode passes non-pointer" + + json.Unmarshal(func() ([]byte, interface{}) { return []byte{}, v }()) +} diff --git a/go/analysis/passes/unmarshal/unmarshal.go b/go/analysis/passes/unmarshal/unmarshal.go new file mode 100644 index 00000000..6cf4358a --- /dev/null +++ b/go/analysis/passes/unmarshal/unmarshal.go @@ -0,0 +1,92 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// The unmarshal package defines an Analyzer that checks for passing +// non-pointer or non-interface types to unmarshal and decode functions. +package unmarshal + +import ( + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" +) + +const doc = `report passing non-pointer or non-interface values to unmarshal + +The unmarshal analysis reports calls to functions such as json.Unmarshal +in which the argument type is not a pointer or an interface.` + +var Analyzer = &analysis.Analyzer{ + Name: "unmarshal", + Doc: doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + call := n.(*ast.CallExpr) + fn := typeutil.StaticCallee(pass.TypesInfo, call) + if fn == nil { + return // not a static call + } + + // Classify the callee (without allocating memory). + argidx := -1 + recv := fn.Type().(*types.Signature).Recv() + if fn.Name() == "Unmarshal" && recv == nil { + // "encoding/json".Unmarshal + // "encoding/xml".Unmarshal + switch fn.Pkg().Path() { + case "encoding/json", "encoding/xml": + argidx = 1 // func([]byte, interface{}) + } + } else if fn.Name() == "Decode" && recv != nil { + // (*"encoding/json".Decoder).Decode + // (* "encoding/gob".Decoder).Decode + // (* "encoding/xml".Decoder).Decode + t := recv.Type() + if ptr, ok := t.(*types.Pointer); ok { + t = ptr.Elem() + } + tname := t.(*types.Named).Obj() + if tname.Name() == "Decoder" { + switch tname.Pkg().Path() { + case "encoding/json", "encoding/xml", "encoding/gob": + argidx = 0 // func(interface{}) + } + } + } + if argidx < 0 { + return // not a function we are interested in + } + + if len(call.Args) < argidx+1 { + return // not enough arguments, e.g. called with return values of another function + } + + t := pass.TypesInfo.Types[call.Args[argidx]].Type + switch t.Underlying().(type) { + case *types.Pointer, *types.Interface: + return + } + + switch argidx { + case 0: + pass.Reportf(call.Lparen, "call of %s passes non-pointer", fn.Name()) + case 1: + pass.Reportf(call.Lparen, "call of %s passes non-pointer as second argument", fn.Name()) + } + }) + return nil, nil +} diff --git a/go/analysis/passes/unmarshal/unmarshal_test.go b/go/analysis/passes/unmarshal/unmarshal_test.go new file mode 100644 index 00000000..ae19e5dd --- /dev/null +++ b/go/analysis/passes/unmarshal/unmarshal_test.go @@ -0,0 +1,17 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package unmarshal_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/unmarshal" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, unmarshal.Analyzer, "a") +}