search

dominikh / go-tools

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

Issues with rangefunc in 1.23rc1 projects #1569

Closed matta closed 4 months ago

matta commented 4 months ago

Basic issue

  1. statichceck built at HEAD complains about rangefunc usage within the standard library.
  2. With GOEXPERIMENT=rangefunc set, staticcheck panics.

The second issue is a problem because currently gopls v0.16 says it supports rangefunc, but appears to require GOEXPERIMENT=rangefunc in 1.23rc1 projects before it stops reporting errors for them. See https://github.com/golang/go/issues/68248

Steps and more information:

Create a project with go.mod

module example.com/m

go 1.23rc1

and main.go:

package main

import (
    "fmt"
    "maps"
)

func main() {
    m := make(map[string]int)
    m["a"] = 1
    m["b"] = 2

    for k, v := range maps.All(m) {
        fmt.Println(k, v)
    }
}

Then install statichceck HEAD:

$ go install honnef.co/go/tools/cmd/staticcheck@dec278f

Then see two different kinds of failures. The first is with an empty GOEXPERIMENT.

$ GOEXPERIMENT= staticcheck                                                                                                                                                                 ~/scratch/maps-all
../../sdk/go1.23rc1/src/maps/iter.go:51:20: cannot range over seq (variable of type iter.Seq2[K, V]) (compile)
../../sdk/go1.23rc1/src/slices/iter.go:51:17: cannot range over seq (variable of type iter.Seq[E]) (compile)

Notice with the above: it is complaining about iter.go in the standard library, not the use of rangefunc in main.go.

$ GOEXPERIMENT=rangefunc staticcheck                                                                                                                                                        ~/scratch/maps-all
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x70fb72]

goroutine 231 [running]:
honnef.co/go/tools/go/ir.memberFromObject(0xc00028e630, {0x0, 0x0}, {0xa484b0, 0xc0002514d0}, {0x0, 0x0})
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/go/ir/create.go:53 +0x52
honnef.co/go/tools/go/ir.membersFromDecl(0xc00028e630, {0xa49ba0?, 0xc0002514d0}, {0x0, 0x0})
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/go/ir/create.go:165 +0x11c
honnef.co/go/tools/go/ir.(*Program).CreatePackage(0xc0004204e0, 0xc000282ae0, {0xc000121300, 0x1, 0x1}, 0xc000282b40, 0x0)
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/go/ir/create.go:212 +0x845
honnef.co/go/tools/internal/passes/buildir.run(0xc0001fa540)
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/internal/passes/buildir/buildir.go:85 +0x16e
honnef.co/go/tools/lintcmd/runner.(*analyzerRunner).do(0xc0005a78c0, {0xa4c178?, 0xc00039f2c0})
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/lintcmd/runner/runner.go:986 +0x71b
honnef.co/go/tools/lintcmd/runner.genericHandle({0xa4c178, 0xc00039f2c0}, {0xa4c178?, 0xc00039f180?}, 0xc000282ea0, 0xc0003f23b0, 0xc000528270)
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/lintcmd/runner/runner.go:811 +0x11f
created by honnef.co/go/tools/lintcmd/runner.(*subrunner).runAnalyzers in goroutine 97
    /home/matt/go/pkg/mod/honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9/lintcmd/runner/runner.go:1055 +0x6a6

Additional info

$ statichceck --version
staticcheck (devel, v0.5.0-0.dev.0.20240624124555-dec278f2f0d9)
$ staticcheck (devel, v0.5.0-0.dev.0.20240624124555-dec278f2f0d9)

Compiled with Go version: go1.22.3 X:rangefunc
Main module:
    honnef.co/go/tools@v0.5.0-0.dev.0.20240624124555-dec278f2f0d9 (sum: h1:QlANIrV9ZGfD4TrObBkwYAoRVO4u/PvISpkw8gCl1/Y=)
Dependencies:
    github.com/BurntSushi/toml@v1.4.1-0.20240526193622-a339e1f7089c (sum: h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs=)
    golang.org/x/exp/typeparams@v0.0.0-20231108232855-2478ac86f678 (sum: h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=)
    golang.org/x/mod@v0.17.0 (sum: h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA=)
    golang.org/x/sync@v0.7.0 (sum: h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=)
    golang.org/x/tools@v0.21.1-0.20240531212143-b6235391adb3 (sum: h1:SHq4Rl+B7WvyM4XODon1LXtP7gcG49+7Jubt1gWWswY=)
$ go version
go version go1.23rc1 linux/amd64
matta commented 4 months ago

Related issues:

  1. 1494

  2. 1539

  3. 1565 (maybe)

dominikh commented 4 months ago

Note that when you do go install honnef.co/go/tools/cmd/staticcheck@dec278f, it is not upgrading Go to the version in your module. You're actually compiling Staticcheck with go1.22.3 X:rangefunc.

I don't yet know why this panics, and it shouldn't, but it works fine when you build Staticcheck with Go 1.23.

dominikh commented 4 months ago

The crash was the symptom of a missing error check, fixed in 5950450ad4273d602a7f404fe5d80b8851c1336b. f4ee291f27bd904ad3669ffe8bda35cb72e134a0 improves Staticcheck to print a more useful error message when the version of Go Staticcheck was built with is older than the version required by the module under analysis.

That go install is not using your module's Go version is documented in go help install:

If the arguments have version suffixes (like @latest or @v1.0.0), "go install" builds packages in module-aware mode, ignoring the go.mod file in the current directory or any parent directory, if there is one. This is useful for installing executables without affecting the dependencies of the main module.

That this makes the combination of go version + go install confusing is being discussed at https://github.com/golang/go/issues/66518.

matta commented 4 months ago

Note that when you do go install honnef.co/go/tools/cmd/staticcheck@dec278f, it is not upgrading Go to the version in your module. You're actually compiling Staticcheck with go1.22.3 X:rangefunc.

Ahh, I did not realize that these tools needed to be built with a version of go at least as recent as the go module they are analyzing. Makes sense, given that they are probably using huge portions of the compiler infrastructure.

https://github.com/dominikh/go-tools/commit/f4ee291f27bd904ad3669ffe8bda35cb72e134a0 improves Staticcheck to print a more useful error message when the version of Go Staticcheck was built with is older than the version required by the module under analysis.

Nice idea.