SA4006 false positive for variable used in for loop

[jecolasurdo]

System Info

Click to expand!

staticcheck version: `staticcheck 2021.1.1 (v0.2.1)` staticcheck debug.version ``` staticcheck 2021.1.1 (v0.2.1) Compiled with Go version: go1.17 Main module: honnef.co/go/tools@v0.2.1 (sum: h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=) Dependencies: github.com/BurntSushi/toml@v0.3.1 (sum: h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=) golang.org/x/mod@v0.3.0 (sum: h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=) golang.org/x/sys@v0.0.0-20210119212857-b64e53b001e4 (sum: h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=) golang.org/x/tools@v0.1.0 (sum: h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=) golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 (sum: h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=) ``` Go version: `go version go1.17 darwin/amd64` Go Env: ``` GO111MODULE="auto" GOARCH="amd64" GOBIN="" GOCACHE="/Users/joe/Library/Caches/go-build" GOENV="/Users/joe/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/joe/go/pkg/mod" GONOPROXY="xxxxxxxxxxxxxx" GONOSUMDB="xxxxxxxxxxxxxx" GOOS="darwin" GOPATH="/Users/joe/go" GOPRIVATE="xxxxxxxxxxxxxx" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/Cellar/go/1.17/libexec" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/Cellar/go/1.17/libexec/pkg/tool/darwin_amd64" GOVCS="" GOVERSION="go1.17" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/joe/Documents/xxxxxxxxxxxxxx/go.mod" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jg/68cfcdxj6w35t9xh7c0tbtb80000gn/T/go-build1921324886=/tmp/go-build -gno-record-gcc-switches -fno-common" ```

Command / Output

staticcheck ./...
usercontext/internal/rbac/client.go:100:2: this value of policies is never used (SA4006)

Code

func (c *Client) IsAuthorizedForOrgInput(ctx *context.Context, input IsAuthorizedForOrgInput) (bool, error) {
    policies := engine.FlattenRolesToPolicies(c.userInfo.Roles...)
    policies = engine.FilterApplicablePolicies(input.RequestedResource, input.RequestedAction, policies...)
    policies = engine.FilterCompetingEffects(policies...)

    // check if the org is the home org, is a descendent of the user's home org, or is unrelated
    relationship, err := c.GetOrgRelationship(input.OrgID)
    if err != nil {
        return false, err
    }

    var scopeMatcher func(types.Policy) bool
    switch relationship {
    case OrgRelationshipUnknown:
        scopeMatcher = func(p types.Policy) bool { return false }
    case OrgRelationshipUnrelated:
        scopeMatcher = func(p types.Policy) bool { return false }
    case OrgRelationshipHome:
        scopeMatcher = func(p types.Policy) bool {
            return p.Scope.Value == ScopeOrgsRecursive || p.Scope.Value == ScopeOrgsHome
        }
    case OrgRelationshipRelated:
        scopeMatcher = func(p types.Policy) bool { return p.Scope.Value == ScopeOrgsRecursive }
    default:
        return false, fmt.Errorf("unexpected org relationship encountered: '%v'", relationship)
    }

    allowed := false
    denied := false
    for _, policy := range policies {
        if policy.Effect.Value == EffectAllow && scopeMatcher(policy) {
            allowed = true
        } else if policy.Effect.Value == EffectDeny && scopeMatcher(policy) {
            denied = true
        }
    }

    return allowed && !denied, nil
}

Problem

staticcheck complains about line 4 in the sample above, saying that policies is never used, but that's not true. The value is used in the for loop later in the method as for _, policy := range policies {

Expectation

I would expect that the linter not have a problem with this. However, if there is a rationale, it would be nice to get some additional context about how it arrived at this conclusion.

[dominikh]

I cannot reproduce the problem with the provided code and stub functions. This is probably an instance of the issue described in #954. Does c.GetOrgRelationship panic unconditionally?

[jecolasurdo]

Hmm. Interesting. Yes, that method is a stub that just panics at the moment (but in the future will properly return a tuple).

So the linter behavior is technically correct then.

Cases like this might be worth calling out in #954, or even moved to a different category of lints. Staticcheck should make it more clear that transitive behavior (such as unchecked panics) will trigger SA4006.

[dominikh]

or even moved to a different category of lints

I don't think this will be necessary once the check accurately reports why a value isn't being read, such as pointing out where the function returns.

I agree that the current behavior is confusing. It is also related to #807 and #787 and work on improving that continues.

I am going to close this issue because it is effectively a duplicate of the aforementioned issues.

[jecolasurdo]

Sounds good. Thanks!

On Mon, Oct 4, 2021 at 8:22 PM Dominik Honnef @.***> wrote:

or even moved to a different category of lints

I don't think this will be necessary once the check accurately reports why a value isn't being read, such as pointing out where the function returns.

I agree that the current behavior is confusing. It is also related to #807 https://github.com/dominikh/go-tools/issues/807 and #787 https://github.com/dominikh/go-tools/issues/787 and work on improving that continues.

I am going to close this issue because it is effectively a duplicate of the aforementioned issues.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dominikh/go-tools/issues/1090#issuecomment-934026386, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAX27KVTXW775JUIAX2M33TUFJVPHANCNFSM5FKQ7VYA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.