dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.01k stars 361 forks source link

this value of ok is never used (SA4006): false positive ? #1521

Closed ThierrySMG closed 2 months ago

ThierrySMG commented 2 months ago

Output of the command and what's wrong with the output

...
pkg/envoy/store/system.go:442:10: this value of ok is never used (SA4006)
...

Where we can read the code you're running Staticcheck on https://github.com/cortezaproject/corteza/blob/eb7c39fee5dabcd1a73b2822866cd86f5be2c0b8/server/pkg/envoy/store/system.go#L442C1-L443C1

**the code***

for _, p := range r.refPathRes {
    if ok, err := c(p, f); err != nil {
        return &auxRsp{
            err: err,
        }
    } else if !ok {
        continue
    }
}

Note, I read the Go specification about portability of variable declared inside if and used in else if, I don't found something perfectly clear. I tried to reproduce the case without success. It seems the ok value is used. Note the function c could return all combination of (bool, err).

staticcheck -version

staticcheck (no version)

Note: I compile staticcheck from commit: 0a683c983d783468a36cacffe289440ea4b7b943

staticcheck -debug.version

staticcheck (no version)

Compiled with Go version: go1.22.1
Main module:
    honnef.co/go/tools
Dependencies:
    github.com/BurntSushi/toml@v1.3.2 (sum: h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=)
    golang.org/x/exp/typeparams@v0.0.0-20231108232855-2478ac86f678 (sum: h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=)
    golang.org/x/mod@v0.14.0 (sum: h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=)
    golang.org/x/sys@v0.14.0 (sum: h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=)
    golang.org/x/tools@v0.15.0 (sum: h1:zdAyfUGbYmuVokhzVmghFl2ZJh5QhcfebBgmVPFYA+8=)

go version

go version go1.22.1 darwin/arm64

go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xxxxx/Library/Caches/go-build'
GOENV='/Users/xxxxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xxxxx/go/pkg/mod'
GONOPROXY='git.xxxxx.com'
GONOSUMDB='git.xxxxx.com'
GOOS='darwin'
GOPATH='/Users/xxxxx/go'
GOPRIVATE='git.xxxxx.com'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.1'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/xxxxx/git/go-tools/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/44/mds52zg54j5fxc9pvtvdxzb80000gn/T/go-build2927932397=/tmp/go-build -gno-record-gcc-switches -fno-common'

Exactly which command you ran

staticcheck ./...
dominikh commented 2 months ago

The diagnostic is correct, but a bit subtle.

Your code

for _, p := range r.refPathRes {
    if ok, err := c(p, f); err != nil {
        return &auxRsp{
            err: err,
        }
    } else if !ok {
        continue
    }
}

is identical to

for _, p := range r.refPathRes {
    if _, err := c(p, f); err != nil {
        return &auxRsp{
            err: err,
        }
    }
}

That is, the else branch is entirely superfluous, as all it does is what would happen, anyway: the continuation of the loop.

ThierrySMG commented 2 months ago

Thanks for help ! Sorry for the false positive.