close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

go/analysis/passes/shadow: adapt for analysis API 

Change-Id: I9b7c98ab9647e17c286e656860038498e32f99f2
Reviewed-on: https://go-review.googlesource.com/c/142358
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Alan Donovan 2018-10-15 17:45:59 -04:00
parent 4805105a3a
commit 45f20876fc
3 changed files with 118 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

155
go/analysis/passes/shadow/shadow.go
View File

@ -1,11 +1,24 @@
// +build ignore
// Copyright 2013 The Go Authors. All rights reserved. // Copyright 2013 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style // Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file. // license that can be found in the LICENSE file.
/* package shadow
This file contains the code to check for shadowed variables.
import (
"go/ast"
"go/token"
"go/types"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)
// NOTE: Experimental. Not part of the vet suite.
const Doc = `check for possible unintended shadowing of variables
This analyzer check for shadowed variables.
A shadowed variable is a variable declared in an inner scope A shadowed variable is a variable declared in an inner scope
with the same name and type as a variable in an outer scope, with the same name and type as a variable in an outer scope,
and where the outer variable is mentioned after the inner one and where the outer variable is mentioned after the inner one
@ -27,41 +40,70 @@ For example:
} }
return err return err
} }
`
*/ var Analyzer = &analysis.Analyzer{
Name: "shadow",
Doc: Doc,
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
}
package main // flags
var strict = false
import (
"flag"
"go/ast"
"go/token"
"go/types"
)
var strictShadowing = flag.Bool("shadowstrict", false, "whether to be strict about shadowing; can be noisy")
func init() { func init() {
register("shadow", Analyzer.Flags.BoolVar(&strict, "strict", strict, "whether to be strict about shadowing; can be noisy")
"check for shadowed variables (experimental; must be set explicitly)",
checkShadow,
assignStmt, genDecl)
experimental["shadow"] = true
} }
func checkShadow(f *File, node ast.Node) { func run(pass *analysis.Pass) (interface{}, error) {
switch n := node.(type) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
case *ast.AssignStmt:
checkShadowAssignment(f, n) spans := make(map[types.Object]span)
case *ast.GenDecl: for id, obj := range pass.TypesInfo.Defs {
checkShadowDecl(f, n) // Ignore identifiers that don't denote objects
// (package names, symbolic variables such as t
// in t := x.(type) of type switch headers).
if obj != nil {
growSpan(spans, obj, id.Pos(), id.End())
}
} }
for id, obj := range pass.TypesInfo.Uses {
growSpan(spans, obj, id.Pos(), id.End())
}
for node, obj := range pass.TypesInfo.Implicits {
// A type switch with a short variable declaration
// such as t := x.(type) doesn't declare the symbolic
// variable (t in the example) at the switch header;
// instead a new variable t (with specific type) is
// declared implicitly for each case. Such variables
// are found in the types.Info.Implicits (not Defs)
// map. Add them here, assuming they are declared at
// the type cases' colon ":".
if cc, ok := node.(*ast.CaseClause); ok {
growSpan(spans, obj, cc.Colon, cc.Colon)
}
}
nodeFilter := []ast.Node{
(*ast.AssignStmt)(nil),
(*ast.GenDecl)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
switch n := n.(type) {
case *ast.AssignStmt:
checkShadowAssignment(pass, spans, n)
case *ast.GenDecl:
checkShadowDecl(pass, spans, n)
}
})
return nil, nil
} }
// Span stores the minimum range of byte positions in the file in which a // A span stores the minimum range of byte positions in the file in which a
// given variable (types.Object) is mentioned. It is lexically defined: it spans // given variable (types.Object) is mentioned. It is lexically defined: it spans
// from the beginning of its first mention to the end of its last mention. // from the beginning of its first mention to the end of its last mention.
// A variable is considered shadowed (if *strictShadowing is off) only if the // A variable is considered shadowed (if strict is off) only if the
// shadowing variable is declared within the span of the shadowed variable. // shadowing variable is declared within the span of the shadowed variable.
// In other words, if a variable is shadowed but not used after the shadowed // In other words, if a variable is shadowed but not used after the shadowed
// variable is declared, it is inconsequential and not worth complaining about. // variable is declared, it is inconsequential and not worth complaining about.
@ -78,56 +120,56 @@ func checkShadow(f *File, node ast.Node) {
// - A variable declared inside a function literal can falsely be identified // - A variable declared inside a function literal can falsely be identified
// as shadowing a variable in the outer function. // as shadowing a variable in the outer function.
// //
type Span struct { type span struct {
min token.Pos min token.Pos
max token.Pos max token.Pos
} }
// contains reports whether the position is inside the span. // contains reports whether the position is inside the span.
func (s Span) contains(pos token.Pos) bool { func (s span) contains(pos token.Pos) bool {
return s.min <= pos && pos < s.max return s.min <= pos && pos < s.max
} }
// growSpan expands the span for the object to contain the source range [pos, end). // growSpan expands the span for the object to contain the source range [pos, end).
func (pkg *Package) growSpan(obj types.Object, pos, end token.Pos) { func growSpan(spans map[types.Object]span, obj types.Object, pos, end token.Pos) {
if *strictShadowing { if strict {
return // No need return // No need
} }
span, ok := pkg.spans[obj] s, ok := spans[obj]
if ok { if ok {
if span.min > pos { if s.min > pos {
span.min = pos s.min = pos
} }
if span.max < end { if s.max < end {
span.max = end s.max = end
} }
} else { } else {
span = Span{pos, end} s = span{pos, end}
} }
pkg.spans[obj] = span spans[obj] = s
} }
// checkShadowAssignment checks for shadowing in a short variable declaration. // checkShadowAssignment checks for shadowing in a short variable declaration.
func checkShadowAssignment(f *File, a *ast.AssignStmt) { func checkShadowAssignment(pass *analysis.Pass, spans map[types.Object]span, a *ast.AssignStmt) {
if a.Tok != token.DEFINE { if a.Tok != token.DEFINE {
return return
} }
if f.idiomaticShortRedecl(a) { if idiomaticShortRedecl(pass, a) {
return return
} }
for _, expr := range a.Lhs { for _, expr := range a.Lhs {
ident, ok := expr.(*ast.Ident) ident, ok := expr.(*ast.Ident)
if !ok { if !ok {
f.Badf(expr.Pos(), "invalid AST: short variable declaration of non-identifier") pass.Reportf(expr.Pos(), "invalid AST: short variable declaration of non-identifier")
return return
} }
checkShadowing(f, ident) checkShadowing(pass, spans, ident)
} }
} }
// idiomaticShortRedecl reports whether this short declaration can be ignored for // idiomaticShortRedecl reports whether this short declaration can be ignored for
// the purposes of shadowing, that is, that any redeclarations it contains are deliberate. // the purposes of shadowing, that is, that any redeclarations it contains are deliberate.
func (f *File) idiomaticShortRedecl(a *ast.AssignStmt) bool { func idiomaticShortRedecl(pass *analysis.Pass, a *ast.AssignStmt) bool {
// Don't complain about deliberate redeclarations of the form // Don't complain about deliberate redeclarations of the form
// i := i // i := i
// Such constructs are idiomatic in range loops to create a new variable // Such constructs are idiomatic in range loops to create a new variable
@ -140,7 +182,7 @@ func (f *File) idiomaticShortRedecl(a *ast.AssignStmt) bool {
for i, expr := range a.Lhs { for i, expr := range a.Lhs {
lhs, ok := expr.(*ast.Ident) lhs, ok := expr.(*ast.Ident)
if !ok { if !ok {
f.Badf(expr.Pos(), "invalid AST: short variable declaration of non-identifier") pass.Reportf(expr.Pos(), "invalid AST: short variable declaration of non-identifier")
return true // Don't do any more processing. return true // Don't do any more processing.
} }
switch rhs := a.Rhs[i].(type) { switch rhs := a.Rhs[i].(type) {
@ -163,7 +205,7 @@ func (f *File) idiomaticShortRedecl(a *ast.AssignStmt) bool {
// idiomaticRedecl reports whether this declaration spec can be ignored for // idiomaticRedecl reports whether this declaration spec can be ignored for
// the purposes of shadowing, that is, that any redeclarations it contains are deliberate. // the purposes of shadowing, that is, that any redeclarations it contains are deliberate.
func (f *File) idiomaticRedecl(d *ast.ValueSpec) bool { func idiomaticRedecl(d *ast.ValueSpec) bool {
// Don't complain about deliberate redeclarations of the form // Don't complain about deliberate redeclarations of the form
// var i, j = i, j // var i, j = i, j
if len(d.Names) != len(d.Values) { if len(d.Names) != len(d.Values) {
@ -180,34 +222,34 @@ func (f *File) idiomaticRedecl(d *ast.ValueSpec) bool {
} }
// checkShadowDecl checks for shadowing in a general variable declaration. // checkShadowDecl checks for shadowing in a general variable declaration.
func checkShadowDecl(f *File, d *ast.GenDecl) { func checkShadowDecl(pass *analysis.Pass, spans map[types.Object]span, d *ast.GenDecl) {
if d.Tok != token.VAR { if d.Tok != token.VAR {
return return
} }
for _, spec := range d.Specs { for _, spec := range d.Specs {
valueSpec, ok := spec.(*ast.ValueSpec) valueSpec, ok := spec.(*ast.ValueSpec)
if !ok { if !ok {
f.Badf(spec.Pos(), "invalid AST: var GenDecl not ValueSpec") pass.Reportf(spec.Pos(), "invalid AST: var GenDecl not ValueSpec")
return return
} }
// Don't complain about deliberate redeclarations of the form // Don't complain about deliberate redeclarations of the form
// var i = i // var i = i
if f.idiomaticRedecl(valueSpec) { if idiomaticRedecl(valueSpec) {
return return
} }
for _, ident := range valueSpec.Names { for _, ident := range valueSpec.Names {
checkShadowing(f, ident) checkShadowing(pass, spans, ident)
} }
} }
} }
// checkShadowing checks whether the identifier shadows an identifier in an outer scope. // checkShadowing checks whether the identifier shadows an identifier in an outer scope.
func checkShadowing(f *File, ident *ast.Ident) { func checkShadowing(pass *analysis.Pass, spans map[types.Object]span, ident *ast.Ident) {
if ident.Name == "_" { if ident.Name == "_" {
// Can't shadow the blank identifier. // Can't shadow the blank identifier.
return return
} }
obj := f.pkg.defs[ident] obj := pass.TypesInfo.Defs[ident]
if obj == nil { if obj == nil {
return return
} }
@ -221,7 +263,7 @@ func checkShadowing(f *File, ident *ast.Ident) {
if shadowed.Parent() == types.Universe { if shadowed.Parent() == types.Universe {
return return
} }
if *strictShadowing { if strict {
// The shadowed identifier must appear before this one to be an instance of shadowing. // The shadowed identifier must appear before this one to be an instance of shadowing.
if shadowed.Pos() > ident.Pos() { if shadowed.Pos() > ident.Pos() {
return return
@ -229,9 +271,9 @@ func checkShadowing(f *File, ident *ast.Ident) {
} else { } else {
// Don't complain if the span of validity of the shadowed identifier doesn't include // Don't complain if the span of validity of the shadowed identifier doesn't include
// the shadowing identifier. // the shadowing identifier.
span, ok := f.pkg.spans[shadowed] span, ok := spans[shadowed]
if !ok { if !ok {
f.Badf(shadowed.Pos(), "internal error: no range for %q", shadowed.Name()) pass.Reportf(ident.Pos(), "internal error: no range for %q", ident.Name)
return return
} }
if !span.contains(ident.Pos()) { if !span.contains(ident.Pos()) {
@ -240,6 +282,7 @@ func checkShadowing(f *File, ident *ast.Ident) {
} }
// Don't complain if the types differ: that implies the programmer really wants two different things. // Don't complain if the types differ: that implies the programmer really wants two different things.
if types.Identical(obj.Type(), shadowed.Type()) { if types.Identical(obj.Type(), shadowed.Type()) {
f.Badf(ident.Pos(), "declaration of %q shadows declaration at %s", obj.Name(), f.loc(shadowed.Pos())) line := pass.Fset.Position(shadowed.Pos()).Line
pass.Reportf(ident.Pos(), "declaration of %q shadows declaration at line %d", obj.Name(), line)
} }
} }

13
go/analysis/passes/shadow/shadow_test.go Normal file
View File

@ -0,0 +1,13 @@
package shadow_test
import (
"testing"
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/shadow"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, shadow.Analyzer, "a")
}

12
go/analysis/passes/shadow/testdata/src/a/a.go vendored
View File

@ -6,7 +6,7 @@
// Some of these errors are caught by the compiler (shadowed return parameters for example) // Some of these errors are caught by the compiler (shadowed return parameters for example)
// but are nonetheless useful tests. // but are nonetheless useful tests.
package testdata package a
import "os" import "os"
@ -17,7 +17,7 @@ func ShadowRead(f *os.File, buf []byte) (err error) {
_ = err _ = err
} }
if f != nil { if f != nil {
_, err := f.Read(buf) // ERROR "declaration of .err. shadows declaration at shadow.go:13" _, err := f.Read(buf) // want "declaration of .err. shadows declaration at line 13"
if err != nil { if err != nil {
return err return err
} }
@ -25,8 +25,8 @@ func ShadowRead(f *os.File, buf []byte) (err error) {
_ = i _ = i
} }
if f != nil { if f != nil {
x := one() // ERROR "declaration of .x. shadows declaration at shadow.go:14" x := one() // want "declaration of .x. shadows declaration at line 14"
var _, err = f.Read(buf) // ERROR "declaration of .err. shadows declaration at shadow.go:13" var _, err = f.Read(buf) // want "declaration of .err. shadows declaration at line 13"
if x == 1 && err != nil { if x == 1 && err != nil {
return err return err
} }
@ -46,7 +46,7 @@ func ShadowRead(f *os.File, buf []byte) (err error) {
if shadowTemp := shadowTemp; true { // OK: obviously intentional idiomatic redeclaration if shadowTemp := shadowTemp; true { // OK: obviously intentional idiomatic redeclaration
var f *os.File // OK because f is not mentioned later in the function. var f *os.File // OK because f is not mentioned later in the function.
// The declaration of x is a shadow because x is mentioned below. // The declaration of x is a shadow because x is mentioned below.
var x int // ERROR "declaration of .x. shadows declaration at shadow.go:14" var x int // want "declaration of .x. shadows declaration at line 14"
_, _, _ = x, f, shadowTemp _, _, _ = x, f, shadowTemp
} }
// Use a couple of variables to trigger shadowing errors. // Use a couple of variables to trigger shadowing errors.
@ -78,7 +78,7 @@ func shadowTypeSwitch(a interface{}) {
switch t := a.(type) { switch t := a.(type) {
case int: case int:
{ {
t := 0 // ERROR "declaration of .t. shadows declaration at shadow.go:78" t := 0 // want "declaration of .t. shadows declaration at line 78"
_ = t _ = t
} }
_ = t _ = t