sanposhiho / wastedassign Goto Github PK

View Code? Open in Web Editor NEW

35.0 2.0 4.0 57 KB

Go Linter: finds wasted assignment statements 🗑

License: MIT License

Go 100.00%

golang staticanalysis analyzer go

wastedassign's Introduction

wastedassign

wastedassign finds wasted assignment statements

found the value ...

reassigned, but never used afterward
reassigned, but reassigned without using the value

Example

package main

import "fmt"

func f() int {
	a := 0 
        b := 0
        fmt.Print(a)
        fmt.Print(b)
        a = 1  // This reassignment is wasted, because never used afterwards. Wastedassign find this 

        b = 1  // This reassignment is wasted, because reassigned without use this value. Wastedassign find this 
        b = 2
        fmt.Print(b)
        
	return 1 + 2
}

$ go vet -vettool=`which wastedassign` sample.go            
# command-line-arguments
./sample.go:10:2: assigned to a, but never used afterwards
./sample.go:12:2: assigned to b, but reassigned without using the value

Installation

Go version < 1.16

go get -u github.com/sanposhiho/wastedassign/v2/cmd/wastedassign

Go version 1.16+

go install github.com/sanposhiho/wastedassign/v2/cmd/wastedassign@latest

Usage

# in your project

go vet -vettool=`which wastedassign` ./...

And, you can use wastedassign in golangci-lint.

Contribution

I am waiting for your contribution :D

Feel free to create an issue or a PR!

Run test

go test

wastedassign's People

Contributors

Stargazers

Watchers

Forkers

ldez kd-collective alingse ramezaan7

wastedassign's Issues

Report with variable name

want to display the variables that were the subject of the report.

false-positive: variable dependencies in for loop

If this is not false-positive, please close it.

package main

func main() {
	var x int = 0
	var y int = 1
	for i := 1; i < 3; i++ {
		x += y
		y *= 2 * i
	}
	println(x)
}

# command-line-arguments
testdata/a.go:8:3: reassigned, but never used afterwards

returnに該当の変数が用いられている時は使用しているとみなす

ref #1 (comment)

他の変数のStoreに用いられているときは使用されているとみなす

ref #1 (comment)

support auto fix

let's delete wasted assignment statements automatically

We can remove all wasted "re"assignments.
If it is not "re"assignment, but the value is never used afterwards, we can remove it.
If it is not "re"assignment and the value is used after assignment, we cannot remove it. So, we have to change it to Variable declarations with VarDecl. (i.e. change a := 2 to var a int)

無限に再帰してpanicするケースがある

社内のコードで試したので再現可能なコードを提出できない 🙇 のですが無限に再帰するパターンがあるっぽいです…

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc0b7fac408 stack=[0xc0b7fac000, 0xc0d7fac000]
fatal error: stack overflow

runtime stack:
runtime.throw(0xe2ecf68, 0xe)
	/usr/local/opt/go/libexec/src/runtime/panic.go:1116 +0x72
runtime.newstack()
	/usr/local/opt/go/libexec/src/runtime/stack.go:1060 +0x790
runtime.morestack()
	/usr/local/opt/go/libexec/src/runtime/asm_amd64.s:449 +0x8f

goroutine 8096 [running]:
runtime.heapBitsSetType(0xc01d912370, 0x50, 0x50, 0x49f2700)
	/usr/local/opt/go/libexec/src/runtime/mbitmap.go:911 +0xb2c fp=0xc0b7fac418 sp=0xc0b7fac410 pc=0xe09630c
runtime.mallocgc(0x50, 0x49f2700, 0x1, 0x10)
	/usr/local/opt/go/libexec/src/runtime/malloc.go:1090 +0x605 fp=0xc0b7fac4b8 sp=0xc0b7fac418 pc=0xe08ce45
runtime.newobject(0x49f2700, 0xc000101b00)
	/usr/local/opt/go/libexec/src/runtime/malloc.go:1195 +0x38 fp=0xc0b7fac4e8 sp=0xc0b7fac4b8 pc=0xe08d5b8
github.com/sanposhiho/wastedassign.isNextOperationToOpIsStore(0xc08597adf0, 0x2, 0x2, 0xc00dd952f8, 0xc08597adf0, 0x2)
	/Users/vvakame/work/gopath/pkg/mod/github.com/sanposhiho/[email protected]/wastedassign.go:92 +0x5d5 fp=0xc0b7fac608 sp=0xc0b7fac4e8 pc=0xe27d7f5
github.com/sanposhiho/wastedassign.isNextOperationToOpIsStore(0xc08597ade0, 0x2, 0x2, 0xc00dd952f8, 0xc08597ade0, 0x2)
	/Users/vvakame/work/gopath/pkg/mod/github.com/sanposhiho/[email protected]/wastedassign.go:102 +0x415 fp=0xc0b7fac728 sp=0xc0b7fac608 pc=0xe27d635
github.com/sanposhiho/wastedassign.isNextOperationToOpIsStore(0xc05c5db4b8, 0x1, 0x1, 0xc00dd952f8, 0xc05c5db4b8, 0x1)
	/Users/vvakame/work/gopath/pkg/mod/github.com/sanposhiho/[email protected]/wastedassign.go:102 +0x415 fp=0xc0b7fac848 sp=0xc0b7fac728 pc=0xe27d635
github.com/sanposhiho/wastedassign.isNextOperationToOpIsStore(0xc08597add0, 0x2, 0x2, 0xc00dd952f8, 0xc08597add0, 0x2)
	/Users/vvakame/work/gopath/pkg/mod/github.com/sanposhiho/[email protected]/wastedassign.go:102 +0x415 fp=0xc0b7fac968 sp=0xc0b7fac848 pc=0xe27d635
github.com/sanposhiho/wastedassign.isNextOperationToOpIsStore(0xc08597adc0, 0x2, 0x2, 0xc00dd952f8, 0xc08597adc0, 0x2)

Type-Switchしている箇所の動作が怪しい気がする

wastedassign.goに対してwastedassignをかけると再現する。

./wastedassign.go:87:11: reassigned, but never used afterwards

該当の箇所は
https://github.com/sanposhiho/wastedassign/blob/master/wastedassign.go#L87
ここ

そもそもreassignedですらないため内部でreassignされ直してるとかなのでは

Suspected false positive: operator assignment inside loop

The following code generates an error for the s *= 2 statement:

func (FermatStrategy) K(n *big.Int) []*big.Int {
	ks := []*big.Int{}
	k := new(big.Int).Rsh(n, 1)
	one := bigint.One()
	s := uint(1)
	for k.Cmp(one) > 0 {
		ks = append(ks, bigint.Clone(k))
		k.Rsh(k, s)
		s *= 2
	}
	return ks
}

https://github.com/mmcloughlin/addchain/blob/3d88f7689d76cd7d9afa7cefaac9a0ab6c5ba3c2/alg/contfrac/contfrac.go#L225

Specifically, when run via golangci-lint version 1.38.0 I see:

alg/contfrac/contfrac.go:225:3: reassigned, but never used afterwards (wastedassign)
		s *= 2
		^

I believe this is a false positive.

関数の引数で取得した物を比較でしか使ってないと"reassigned, but never used afterwards"が出る

package a

func p(x int) int {
	return x + 1
}

func f(param int) int {
	useOutOfIf := 1212121
	ret := 0 // want "wasted assignment"
	if false {
		useOutOfIf = 10 // want "wasted assignment"
		useOutOfIf = 10 // want "wasted assignment"

	} else if param == 100 {
		ret = useOutOfIf
	} else if param == 200 {
		useOutOfIf = 100 // want "wasted assignment"
		useOutOfIf = 100
		useOutOfIf = p(useOutOfIf)
		useOutOfIf += 200 // want "reassigned, but never used afterwards"
	}
	useOutOfIf = 12
	println(useOutOfIf)
	useOutOfIf = 192
	useOutOfIf += 100
	useOutOfIf += 200 // want "reassigned, but never used afterwards"
	return ret
}

Ignore variable declaration if it's the zero value

At work, we are working on updating all our linting, and one of the problems we have hit with wastedassign is that code like this gets flagged:

isFoo := false
// ...

isFoo, err := bar.Baz()

Now, I know that you should probably write var isFoo bool ... but it's not technically incorrect, and almost assuredly does not indicate an error, just someone who is used to other languages. It would be great if there were a way to avoid flagging that kind of a line so that we can use it for the other cases when it is much more likely to indicate a bug.

Document difference to `ineffassign` linter?

wastedassign seems similar to ineffassign which is included in golangci-lint 1.0.0 and enabled by default in it.

Would it be possible to document in wastedassign's README what are the differences and/or why would one choose to use one over the other? (Asking here instead of in ineffassign because ineffassign was there first :))

SSA and generics (go1.18)

Currently, SSA is not working with generics.

So your linter produces a panic when it is used with generics.

There is an issue open about that in the Go repository: golang/go#48525

Inside golangci-lint, we have disabled your linters: golangci/golangci-lint#2649

You have 2 solutions:

waiting for a version of SSA that will support generics
dropping the SSA analyzers and using something else to analyze the code.

Related to golang/go#50558

連続ループがあると無限待ちになる気がする

気がする

要調査

if-elseif文の対応が甘い

https://twitter.com/sanpo_shiho/status/1301736211086602241

これのせいでSuccsでジャンプした時にelse ifがあると一気に飛ぶ可能性がある物全てに飛べない

func f(param int) int {
	useOutOfIf := 1212121 // want "wasted assignment"
	ret := 0 // want "wasted assignment"
	if false {
		useOutOfIf = 10 // want "wasted assignment"
		useOutOfIf = 10 // want "reassigned, but never used afterwards"

		return 0
	} else if param == 100 {
		ret = useOutOfIf
	} else if param == 200 {
		useOutOfIf = 100 // want "wasted assignment"
		useOutOfIf = 100
		useOutOfIf = p(useOutOfIf)
		useOutOfIf += 200 // want "reassigned, but never used afterwards"
	}
	useOutOfIf = 12
	println(useOutOfIf)
	useOutOfIf = 192
	useOutOfIf += 100
	useOutOfIf += 200 // want "reassigned, but never used afterwards"
	return ret
}

この場合上からBlockを1, 2, 3, 4, 5とすると

本来人間的視点からすると1のSuccsに2, 3, 4全てが入っていて欲しいが、2, 3しか入っていない

そのため、1→4という動きが本来あるはずが、一度3を通って、1→3→4となるため、3に代入があったりすると無駄に'wasted assignment'が発生する（2行目では本来"wasted assignment"が出て欲しくない）（else if param == 100の句で使用しているので）

false-positive: reassign inside an infinite loop and not use after exiting loop

package main

func main() {
	var err error
	var rest []byte
	for {
		rest, err = bar(rest)

		if err == nil {
			break
		}
	}
}

func bar(d []byte) ([]byte, error) {
	return nil, nil
}

foo.go:7:3: reassigned, but never used afterwards (wastedassign)
                rest, err = bar(rest)
                ^

complex c style for loop false positive

The for loop in the following read function triggers a false positive:

func read(r io.Reader, b []byte) (err error) {
        for i, n := 0, 0; i != len(b); i += n {
                n, err = r.Read(b[i:])
                if err != nil {
                        return err
                }
        }
        return nil
}

Yes, I now know that I should just do io.ReadFull instead; but the reproducer stands.

False positive in loops

Hello!

	var i int
	for res := range results {
		suite.requireEqualResults(expected[i], res)
		i++ // reassigned, but never used afterwards (wastedassign)
	}

Panic case

Hi!

Please look at this simple example:

func didPanic(fn func()) (result bool) {
	result = true // Handle `panic(nil)` case.
	defer func() { recover() }()
	fn()
	return false
}

assigned to result, but reassigned without using the value (wastedassign)
        result = true
        ^

golangci-lintに対応したい

https://golangci-lint.run/contributing/new-linters/

False positive with switch and function call