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

go/analysis/passes/atomic: split out of vet 

Also: don't abort loading just because there were parse/type errors.
It's the driver's job to decide whether to fail due to errors.

Change-Id: I055033fb89319d957b328c4fa4a30144afc7457c
Reviewed-on: https://go-review.googlesource.com/c/140738
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Alan Donovan 2018-10-08 20:33:22 -04:00
parent 65a9b9c4ab
commit 3f6d6d9ddd
4 changed files with 116 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
go/analysis/passes/atomic/atomic.go Normal file
View File

@ -0,0 +1,92 @@
// Copyright 2013 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 atomic
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/analysis/passes/internal/analysisutil"
"golang.org/x/tools/go/ast/inspector"
)
var Analyzer = &analysis.Analyzer{
Name: "atomic",
Doc: `check for common mistakes using the sync/atomic package
The atomic checker looks for assignment statements of the form:
x = atomic.AddUint64(&x, 1)
which are not atomic.`,
Requires: []*analysis.Analyzer{inspect.Analyzer},
RunDespiteErrors: true,
Run: run,
}
func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{
(*ast.AssignStmt)(nil),
}
inspect.Preorder(nodeFilter, func(node ast.Node) {
n := node.(*ast.AssignStmt)
if len(n.Lhs) != len(n.Rhs) {
return
}
if len(n.Lhs) == 1 && n.Tok == token.DEFINE {
return
}
for i, right := range n.Rhs {
call, ok := right.(*ast.CallExpr)
if !ok {
continue
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
continue
}
pkgIdent, _ := sel.X.(*ast.Ident)
pkgName, ok := pass.TypesInfo.Uses[pkgIdent].(*types.PkgName)
if !ok || pkgName.Imported().Path() != "sync/atomic" {
continue
}
switch sel.Sel.Name {
case "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr":
checkAtomicAddAssignment(pass, n.Lhs[i], call)
}
}
})
return nil, nil
}
// checkAtomicAddAssignment walks the atomic.Add* method calls checking
// for assigning the return value to the same variable being used in the
// operation
func checkAtomicAddAssignment(pass *analysis.Pass, left ast.Expr, call *ast.CallExpr) {
if len(call.Args) != 2 {
return
}
arg := call.Args[0]
broken := false
gofmt := func(e ast.Expr) string { return analysisutil.Format(pass.Fset, e) }
if uarg, ok := arg.(*ast.UnaryExpr); ok && uarg.Op == token.AND {
broken = gofmt(left) == gofmt(uarg.X)
} else if star, ok := left.(*ast.StarExpr); ok {
broken = gofmt(star.X) == gofmt(arg)
}
if broken {
pass.Reportf(left.Pos(), "direct assignment to atomic value")
}
}

13
go/analysis/passes/atomic/atomic_test.go Normal file
View File

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

22
go/analysis/passes/vet/testdata/atomic.go → go/analysis/passes/atomic/testdata/src/a/a.go vendored
View File

@ -4,7 +4,7 @@
// This file contains tests for the atomic checker.
package testdata
package a
import (
"sync/atomic"
@ -14,39 +14,39 @@ type Counter uint64
func AtomicTests() {
x := uint64(1)
x = atomic.AddUint64(&x, 1) // ERROR "direct assignment to atomic value"
_, x = 10, atomic.AddUint64(&x, 1) // ERROR "direct assignment to atomic value"
x, _ = atomic.AddUint64(&x, 1), 10 // ERROR "direct assignment to atomic value"
x = atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
_, x = 10, atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
x, _ = atomic.AddUint64(&x, 1), 10 // want "direct assignment to atomic value"
y := &x
*y = atomic.AddUint64(y, 1) // ERROR "direct assignment to atomic value"
*y = atomic.AddUint64(y, 1) // want "direct assignment to atomic value"
var su struct{ Counter uint64 }
su.Counter = atomic.AddUint64(&su.Counter, 1) // ERROR "direct assignment to atomic value"
su.Counter = atomic.AddUint64(&su.Counter, 1) // want "direct assignment to atomic value"
z1 := atomic.AddUint64(&su.Counter, 1)
_ = z1 // Avoid err "z declared and not used"
var sp struct{ Counter *uint64 }
*sp.Counter = atomic.AddUint64(sp.Counter, 1) // ERROR "direct assignment to atomic value"
*sp.Counter = atomic.AddUint64(sp.Counter, 1) // want "direct assignment to atomic value"
z2 := atomic.AddUint64(sp.Counter, 1)
_ = z2 // Avoid err "z declared and not used"
au := []uint64{10, 20}
au[0] = atomic.AddUint64(&au[0], 1) // ERROR "direct assignment to atomic value"
au[0] = atomic.AddUint64(&au[0], 1) // want "direct assignment to atomic value"
au[1] = atomic.AddUint64(&au[0], 1)
ap := []*uint64{&au[0], &au[1]}
*ap[0] = atomic.AddUint64(ap[0], 1) // ERROR "direct assignment to atomic value"
*ap[0] = atomic.AddUint64(ap[0], 1) // want "direct assignment to atomic value"
*ap[1] = atomic.AddUint64(ap[0], 1)
x = atomic.AddUint64() // Used to make vet crash; now silently ignored.
{
// A variable declaration creates a new variable in the current scope.
x := atomic.AddUint64(&x, 1) // ERROR "declaration of .x. shadows declaration at atomic.go:16"
x := atomic.AddUint64(&x, 1)
// Re-declaration assigns a new value.
x, w := atomic.AddUint64(&x, 1), 10 // ERROR "direct assignment to atomic value"
x, w := atomic.AddUint64(&x, 1), 10 // want "direct assignment to atomic value"
_ = w
}
}

73
go/analysis/passes/vet/atomic.go
View File

@ -1,73 +0,0 @@
// +build ignore
// Copyright 2013 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 main
import (
"go/ast"
"go/token"
"go/types"
)
func init() {
register("atomic",
"check for common mistaken usages of the sync/atomic package",
checkAtomicAssignment,
assignStmt)
}
// checkAtomicAssignment walks the assignment statement checking for common
// mistaken usage of atomic package, such as: x = atomic.AddUint64(&x, 1)
func checkAtomicAssignment(f *File, node ast.Node) {
n := node.(*ast.AssignStmt)
if len(n.Lhs) != len(n.Rhs) {
return
}
if len(n.Lhs) == 1 && n.Tok == token.DEFINE {
return
}
for i, right := range n.Rhs {
call, ok := right.(*ast.CallExpr)
if !ok {
continue
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
continue
}
pkgIdent, _ := sel.X.(*ast.Ident)
pkgName, ok := f.pkg.uses[pkgIdent].(*types.PkgName)
if !ok || pkgName.Imported().Path() != "sync/atomic" {
continue
}
switch sel.Sel.Name {
case "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr":
f.checkAtomicAddAssignment(n.Lhs[i], call)
}
}
}
// checkAtomicAddAssignment walks the atomic.Add* method calls checking for assigning the return value
// to the same variable being used in the operation
func (f *File) checkAtomicAddAssignment(left ast.Expr, call *ast.CallExpr) {
if len(call.Args) != 2 {
return
}
arg := call.Args[0]
broken := false
if uarg, ok := arg.(*ast.UnaryExpr); ok && uarg.Op == token.AND {
broken = f.gofmt(left) == f.gofmt(uarg.X)
} else if star, ok := left.(*ast.StarExpr); ok {
broken = f.gofmt(star.X) == f.gofmt(arg)
}
if broken {
f.Bad(left.Pos(), "direct assignment to atomic value")
}
}