Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: build constraints not respected for new files #37101

Closed
myitcv opened this issue Feb 7, 2020 · 4 comments
Closed

x/tools/gopls: build constraints not respected for new files #37101

myitcv opened this issue Feb 7, 2020 · 4 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Feb 7, 2020

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

$ go version
go version devel +b7689f5aa3 Fri Jan 31 06:02:00 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200205190317-6e8b36d2c76b
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20200205190317-6e8b36d2c76b

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/.vim/plugged/govim/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 -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build540934445=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Starting with an empty module, create (and save) the following files in sequence:

-- main.go --
package main

import (
	"fmt"
)

func main() {
	fmt.Println(hello())
}
-- hello_linux.go --
package main

func hello() string {
	return "linux"
}
-- hello_darwin.go --
package main

func hello() string {
	return "darwin"
}

What did you expect to see?

No diagnostic errors.

What did you see instead?

PublishDiagnostics callback: &protocol.PublishDiagnosticsParams{
    URI:         "file:///home/myitcv/gostuff/src/github.com/myitcv/playground/hello_darwin.go",
    Version:     57,
    Diagnostics: {
        {
            Range: protocol.Range{
                Start: protocol.Position{Line:2, Character:5},
                End:   protocol.Position{Line:2, Character:10},
            },
            Severity:           1,
            Code:               nil,
            Source:             "compiler",
            Message:            "hello redeclared in this block",
            Tags:               nil,
            RelatedInformation: nil,
        },
    },
}
PublishDiagnostics callback: &protocol.PublishDiagnosticsParams{
    URI:         "file:///home/myitcv/gostuff/src/github.com/myitcv/playground/hello_linux.go",
    Version:     65,
    Diagnostics: {
        {
            Range: protocol.Range{
                Start: protocol.Position{Line:2, Character:5},
                End:   protocol.Position{Line:2, Character:10},
            },
            Severity:           1,
            Code:               nil,
            Source:             "compiler",
            Message:            "other declaration of hello",
            Tags:               nil,
            RelatedInformation: nil,
        },
    },
}

If you close and restart the editor (or indeed gopls) the diagnostics go away.


cc @stamblerre

FYI @leitzler

@heschi
Copy link
Contributor

@heschi heschi commented Sep 2, 2020

@heschi heschi self-assigned this Sep 2, 2020
@heschi
Copy link
Contributor

@heschi heschi commented Sep 2, 2020

Fixing this would involve copying a couple hundred lines of (go/build.Context).MatchFile somewhere, since it doesn't accept file content, only a file name. Then we need to know which tags are active, which involves either some gnarly argument parsing or a call to go list. @stamblerre has expressed concerns about the performance cost of doing that on each packages.Load call. To fix that, we'd need to either put most of the logic into gopls rather than go/packages, or pass data into go/packages so that it doesn't have to re-fetch it every time. go/packages already does a go env call to read stuff that gopls already knows, so that could be a net win.

@heschi heschi removed their assignment Sep 2, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Sep 25, 2020

Now that native overlays will be added in 1.16 (#41598), I don't think it makes sense to fix this issue in go/packages' overlay processing. Let's move this issue out of the v1.0 milestone and check back in after 1.16 is released.

@stamblerre stamblerre closed this Sep 25, 2020
@stamblerre stamblerre reopened this Sep 25, 2020
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Sep 25, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default Nov 13, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Nov 13, 2020

I've just confirmed that this is fixed with Go 1.16. Closing.

@stamblerre stamblerre closed this Nov 13, 2020
vscode-go: gopls by default automation moved this from Needs Triage to Done Nov 13, 2020
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants