x/tools/go/packages: LoadSyntax and LoadSyntaxAll are now the same

[seebs]

What version of Go are you using (go version)?

N/A; this applies to commit dbeab5a, I think.

Does this issue reproduce with the latest release?

Not exactly.

What operating system and processor architecture are you using (go env)?

N/A.

What did you do?

Ran go get -u on something with a go.mod file referring to an older version of x/tools.

What did you expect to see?

Probably the build succeeding anyway.

What did you see instead?

../../../../pkg/mod/github.com/golangci/golangci-lint@v1.15.0/pkg/lint/load.go:171:2: duplicate case "golang.org/x/tools/go/packages".LoadAllSyntax (value 991) in switch
        previous case at /home/seebs/src/go/pkg/mod/golang.org/x/tools@v0.0.0-20190329151228-23e29df326fe/go/packages/packages.go:98

So the issue is, something elsewhere has a switch to show which of the Load constants something is, and this switch breaks because the patch made LoadSyntaxAll and LoadSyntax be the same value. This... may actually be fine. I don't know whether it's intentional or not, I don't entirely think that the external package (golangci-lint) should be that aware of the internals, but I'm also of the opinion that this feels sort of like a breaking change that probably shouldn't be a breaking change -- if nothing else, it seems like there should be a distinction between these, since there used to be a documented distinction.

This is easily avoided by not running go get -u, but I figured I should note that the change appears to have broken at least one thing somewhere.

[torbjornvatn]

I ran into this today as well, makes my CI/CD pipeline fail.

My issue is also related to golangci-lint

[matloob]

This is now fixed in golangci-lint: golangci/golangci-lint@de1d1ad.

I'm going to close this issue for now, but if another case like this pops up, we can think about putting in a "dummy" bit that does nothing to distinguish LoadSyntax and LoadAllSyntax

[matloob]

(Please reopen if that doesn't seem like a good solution)

[thepudds]

@matloob as far as I can tell, this is still causing problems roughly 2 months later.

go get -u github.com/golangci/golangci-lint/cmd/golangci-lint
...
# github.com/golangci/golangci-lint/pkg/lint
...\go\pkg\mod\github.com\golangci\golangci-lint@v1.16.0\pkg\lint\load.go:171:2: duplicate case "golang.org/x/tools/go/packages".LoadAllSyntax (value 991) in switch
        previous case at ...\go\pkg\mod\golang.org\x\tools@v0.0.0-20190525145741-7be61e1b0e51\go\packages\packages.go:98

You had written in golangci/golangci-lint@de1d1ad#commitcomment-33008947:

I made the change that broke this (sorry!)

LoadAllSyntax and LoadSyntax are now the equal in x/tools/go/packages, and the Go compiler will object to two identical switch cases, so that change breaks the compilation of this code.

It sounded like you had an idea for fixing this within x/tools/go/packages:

we can think about putting in a "dummy" bit that does nothing to distinguish LoadSyntax and LoadAllSyntax

Could that change happen?

If not, could this bug be re-opened, at least until there is a golangci-lint release that solves this, and could you ping the golangci-lint people to encourage them to issue a release that solves this?

Also, I have only briefly tried to tease apart the history here; apologies if I am not fully following the history.

CC @dmitshur

[dmitshur]

as far as I can tell, this is still causing problems roughly 2 months later.

Did the commit golangci/golangci-lint@de1d1ad not fix the issue, or is the problem that there was no new release tag made that includes that commit?

Checked myself (golangci/golangci-lint@v1.16.0...master), the problem is that there's no new release tag.

This seems like an issue that needs to be solved in the golangci-lint project, and not x/tools. Making changes to x/tools won't help if the golangci-lint doesn't update to a newer version and issue a new release. Please let me know if you think there is something else that can be done here.