False positive and confusing error message.

[smoser]

I've hit a false positive with a confusing message as well.

When executed on the program below scopelint complains about hasData()'s use use of scanData:

$ go version
go version go1.13.7 linux/amd64

# locally built scopelint from git v0.2.0 (c2a9a4531b64e)
$ scopelint --version
snapshot

$ scopelint mytest.go
mytest.go:31:38: Using the variable on range scope "bool" in function literal
mytest.go:31:54: Using the variable on range scope "Name" in function literal
Found 2 lint problems; failing.

# issue is also seen with golangci-lint.
$ golangci-lint --version
golangci-lint has version 1.23.1 built from 567904e on 2020-01-20T08:00:15Z

$ golangci-lint run --enable-all mytest.go
mytest.go:31:38: Using the variable on range scope `bool` in function literal (scopelint)
    vgs, err := scanData(func(d myData) bool { return d.Name == name })

A few different things can make this warning go away:

• instead of 'var data myData', use 'for _, data := range myList {'
• move the scanData function to below hasData.

mytest.go below:

// mytest.go: demonstrate https://github.com/kyoh86/scopelint/issues/12
package main

import "fmt"

type mySet map[string]myData
type myFilter func(myData) bool
type myData struct {
    Name string
}

func scanData(filter myFilter) (mySet, error) { //nolint:unparam
    var lvs = mySet{}
    var myList = []myData{{Name: "foo"}, {Name: "bar"}}
    var data myData

    for _, data = range myList {
        curName := data.Name
        lv := myData{Name: curName}

        if filter(lv) {
            lvs[name] = lv
        }
    }

    return lvs, nil
}

func hasData(name string) bool {
    // Line below generates warnings.
    vgs, err := scanData(func(d myData) bool { return d.Name == name })
    if err != nil {
        return false
    }

    return len(vgs) != 0
}

func main() {
    if hasData("foo") {
        fmt.Printf("found\n")
    } else {
        fmt.Printf("not found\n")
    }
}

[erikh]

I'm seeing similar problems in code I can't share. I don't think there's an issue with the code.

The code has a function that has this signature:

func (*Obj) InQuery(func(*Query) error) error

Any use of this function's *Query pass-in variable, triggers a misleading scope lint error about the following literal, not the var in question.

I don't believe this function literal is immediately disqualified from scopelint's rules by itself, and it seems to be that way just due to this bug, everything that uses *Query fails in this way.

[kyoh86]

Thanks,

Maybe this is a mistake in the analysis of AST. I'm planning a reimplementation using golang.org/x/tools/go/analysis. Once #11 is resolved, I think it will probably resolve this too.

[feldgendler]

Hi! I see you decided to address this by rewriting the linter completely. Is looppointer ready to use as a replacement for scopelinter?

[feldgendler]

Meanwhile, I'll post the minimal code to reproduce this bug here:

package test

func fn1() {
    var i int
    for _, i = range []int{} {
        _ = i
    }
}

func fn2() {
    _ = func(b bool) {}
}

test.go:11:13: Using the variable on range scope "bool" in function literal
Found 1 lint problems; failing.

The key part is that range is used with = instead of :=. After such a line, innocent code in the same function or even in a different function causes false positives. In this case, the file triggers a false positive in fn2 only if it follows fn1; if you swap the two functions, the false positive goes away.

[feldgendler]

@kyoh86 ^^

This is minimized code to illustrate the issue; we ran into this in production code. Should we switch from scopelint to looppointer?

Thanks a lot in advance!

[kyoh86]

@feldgendler

Sorry, looppointer will have more false-positives.

If you want to switch, I recommend you exportloopref. I want to make it as accurately as possible.