From 6c2f5ec3ce92167c051883829fba19895e2e8f30 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 21 Feb 2021 17:21:34 +0100 Subject: [PATCH 1/2] go mod tidy --- go.sum | 3 --- 1 file changed, 3 deletions(-) diff --git a/go.sum b/go.sum index 2cf2a9f..21d696a 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,6 @@ -github.com/yuin/goldmark v1.2.1 h1:ruQGxdhGHe7FWOJPT0mKs5+pD2Xs1Bm/kdGlHO04FmM= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= @@ -16,7 +14,6 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= From ddf617f7e282cd31f7292e8bc21edad5d49e4b87 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 21 Feb 2021 17:22:07 +0100 Subject: [PATCH 2/2] code review. --- wastedassign.go | 91 +++++++++++++++++++++++++------------------------ 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/wastedassign.go b/wastedassign.go index da3c044..6e72d9b 100644 --- a/wastedassign.go +++ b/wastedassign.go @@ -1,7 +1,7 @@ package wastedassign import ( - "fmt" + "errors" "go/ast" "go/token" "go/types" @@ -14,7 +14,7 @@ import ( const doc = "wastedassign finds wasted assignment statements." -// Analyzer is ... +// Analyzer is the wastedassign analyzer. var Analyzer = &analysis.Analyzer{ Name: "wastedassign", Doc: doc, @@ -31,8 +31,7 @@ type wastedAssignStruct struct { func run(pass *analysis.Pass) (interface{}, error) { // Plundered from buildssa.Run. - mode := ssa.NaiveForm - prog := ssa.NewProgram(pass.Fset, mode) + prog := ssa.NewProgram(pass.Fset, ssa.NaiveForm) // Create SSA packages for all imports. // Order is not significant. @@ -73,12 +72,12 @@ func run(pass *analysis.Pass) (interface{}, error) { fn := pass.TypesInfo.Defs[fdecl.Name].(*types.Func) if fn == nil { - return nil, fmt.Errorf("failed to get func's typesinfo") + return nil, errors.New("failed to get func's typesinfo") } f := ssapkg.Prog.FuncValue(fn) if f == nil { - return nil, fmt.Errorf("failed to get func's SSA-form intermediate representation") + return nil, errors.New("failed to get func's SSA-form intermediate representation") } var addAnons func(f *ssa.Function) @@ -96,41 +95,50 @@ func run(pass *analysis.Pass) (interface{}, error) { typeSwitchPos := map[int]bool{} inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) inspect.Preorder([]ast.Node{new(ast.TypeSwitchStmt)}, func(n ast.Node) { - switch n := n.(type) { - case *ast.TypeSwitchStmt: + if _, ok := n.(*ast.TypeSwitchStmt); ok { typeSwitchPos[pass.Fset.Position(n.Pos()).Line] = true } }) - wastedAssignMap := []wastedAssignStruct{} + var wastedAssignMap []wastedAssignStruct for _, sf := range srcFuncs { for _, bl := range sf.Blocks { blCopy := *bl for _, ist := range bl.Instrs { blCopy.Instrs = rmInstrFromInstrs(blCopy.Instrs, ist) - switch ist.(type) { - case *ssa.Store: - var buf [10]*ssa.Value - for _, op := range ist.Operands(buf[:0]) { - if (*op) != nil && opInLocals(sf.Locals, op) { - if reason := isNextOperationToOpIsStore([]*ssa.BasicBlock{&blCopy}, op, nil); reason != notWasted { - if ist.Pos() != 0 && !typeSwitchPos[pass.Fset.Position(ist.Pos()).Line] { - wastedAssignMap = append(wastedAssignMap, wastedAssignStruct{ - pos: ist.Pos(), - reason: reason.String(), - }) - } - } - } + if _, ok := ist.(*ssa.Store); !ok { + continue + } + + var buf [10]*ssa.Value + for _, op := range ist.Operands(buf[:0]) { + if (*op) == nil || !opInLocals(sf.Locals, op) { + continue } + + reason := isNextOperationToOpIsStore([]*ssa.BasicBlock{&blCopy}, op, nil) + if reason == notWasted { + continue + } + + if ist.Pos() == 0 || typeSwitchPos[pass.Fset.Position(ist.Pos()).Line] { + continue + } + + wastedAssignMap = append(wastedAssignMap, wastedAssignStruct{ + pos: ist.Pos(), + reason: reason.String(), + }) } } } } + for _, was := range wastedAssignMap { pass.Reportf(was.pos, was.reason) } + return nil, nil } @@ -150,28 +158,31 @@ func (wr wastedReason) String() string { return "wasted assignment" case notWasted: return "" + default: + return "" } - return "" } -func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, haveCheckedMap *map[int]bool) wastedReason { - wastedReasons := []wastedReason{} - wastedReasonsCurrentBls := []wastedReason{} +func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, haveCheckedMap map[int]bool) wastedReason { + var wastedReasons []wastedReason + var wastedReasonsCurrentBls []wastedReason if haveCheckedMap == nil { - haveCheckedMap = &map[int]bool{} + haveCheckedMap = map[int]bool{} } for _, bl := range bls { - if (*haveCheckedMap)[bl.Index] == true { + if haveCheckedMap[bl.Index] { continue } - (*haveCheckedMap)[bl.Index] = true + + haveCheckedMap[bl.Index] = true breakFlag := false for _, ist := range bl.Instrs { if breakFlag { break } + switch w := ist.(type) { case *ssa.Store: var buf [10]*ssa.Value @@ -196,6 +207,7 @@ func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, hav } } } + if len(bl.Succs) != 0 && !breakFlag { wastedReason := isNextOperationToOpIsStore(rmSameBlock(bl.Succs, bl), currentOp, haveCheckedMap) if wastedReason == notWasted { @@ -207,17 +219,15 @@ func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, hav wastedReasons = append(wastedReasons, wastedReasonsCurrentBls...) - if len(wastedReasons) != 0 { - if containReassignedSoon(wastedReasons) { - return reassignedSoon - } - return noUseUntilReturn + if len(wastedReasons) != 0 && containReassignedSoon(wastedReasons) { + return reassignedSoon } + return noUseUntilReturn } func rmSameBlock(bls []*ssa.BasicBlock, currentBl *ssa.BasicBlock) []*ssa.BasicBlock { - rto := []*ssa.BasicBlock{} + var rto []*ssa.BasicBlock for _, bl := range bls { if bl != currentBl { @@ -227,15 +237,6 @@ func rmSameBlock(bls []*ssa.BasicBlock, currentBl *ssa.BasicBlock) []*ssa.BasicB return rto } -func containNotWasted(ws []wastedReason) bool { - for _, w := range ws { - if w == notWasted { - return true - } - } - return false -} - func containReassignedSoon(ws []wastedReason) bool { for _, w := range ws { if w == reassignedSoon {