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/imports: doesn't add import for ast.Expr #34199

Closed
muirdm opened this issue Sep 10, 2019 · 6 comments
Closed

x/tools/imports: doesn't add import for ast.Expr #34199

muirdm opened this issue Sep 10, 2019 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@muirdm
Copy link

muirdm commented Sep 10, 2019

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

$ go version
go version go1.13 darwin/amd64

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="/Users/muir/Library/Caches/go-build"
GOENV="/Users/muir/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/muir/projects//go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/muir/projects/tools/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/96/wknz1jg92yl623qgplk6qw2h0000gn/T/go-build439747941=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I ran organize imports on below file:

package main
var _ ast.Expr

What did you expect to see?

I expected it to add an import for "go/ast".

What did you see instead?

No import added. goimports works, and mysteriously organize imports works if you change it to ast.Node.

@gopherbot gopherbot added this to the Unreleased milestone Sep 10, 2019
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Sep 10, 2019
@gopherbot
Copy link

Thank you for filing a gopls issue! Please take a look at the Troubleshooting section of the gopls Wiki page, and make sure that you have provided all of the relevant information here.

@heschi heschi changed the title x/tools/gopls: organize imports doesn't add import for ast.Expr x/tools/imports: doesn't add import for ast.Expr Sep 10, 2019
@heschi
Copy link
Contributor

heschi commented Sep 10, 2019

Not (just) a gopls bug, renamed.

@heschi heschi self-assigned this Sep 10, 2019
@heschi
Copy link
Contributor

heschi commented Sep 10, 2019

Huh.

$ grep 'bytes, type Buffer' go/api/go1.txt
pkg bytes, type Buffer struct
$ grep 'go/ast, type Expr' go/api/go1.txt
pkg go/ast, type Expr interface, End() token.Pos
pkg go/ast, type Expr interface, Pos() token.Pos
pkg go/ast, type Expr interface, unexported methods
...

We take the API files and turn them into zstdlib.go, and use that for the stdlib. So, two bugs: the built in exports list for go/ast is missing Expr, and goimports doesn't look at GOROOT for some reason.

@gopherbot
Copy link

Change https://golang.org/cl/194568 mentions this issue: internal/imports: add all interfaces in mkstdlib

@dmitshur

This comment has been minimized.

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 10, 2019
gopherbot pushed a commit to golang/tools that referenced this issue Sep 10, 2019
In api/*.txt, interface declarations are represented with lines like:
  pkg container/heap, type Interface interface { Len, Less, Pop, Push, Swap }
or, when they have no exported methods:
  pkg go/ast, type Expr interface, unexported methods

The latter form confuses mkstdlib into thinking that it's a method
because of the extra comma, and then it skips the interface entirely.
Running this program is a matter of seconds once per release, so rather
than trying to fix the optimization, just remove it. The parsing logic
doesn't care about the extra lines.

And the corresponding change to the copy in lsp/testdata/unimported.

Updates golang/go#34199

Change-Id: Ic34b8a47537608401e4ef6683617d797f9f50f8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194568
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/194570 mentions this issue: internal/imports: fix scanning GOROOT in module mode

clintjedwards pushed a commit to clintjedwards/tools that referenced this issue Sep 19, 2019
In api/*.txt, interface declarations are represented with lines like:
  pkg container/heap, type Interface interface { Len, Less, Pop, Push, Swap }
or, when they have no exported methods:
  pkg go/ast, type Expr interface, unexported methods

The latter form confuses mkstdlib into thinking that it's a method
because of the extra comma, and then it skips the interface entirely.
Running this program is a matter of seconds once per release, so rather
than trying to fix the optimization, just remove it. The parsing logic
doesn't care about the extra lines.

And the corresponding change to the copy in lsp/testdata/unimported.

Updates golang/go#34199

Change-Id: Ic34b8a47537608401e4ef6683617d797f9f50f8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194568
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
clintjedwards pushed a commit to clintjedwards/tools that referenced this issue Sep 19, 2019
Go 1.13 introduced a module in GOROOT/src. That triggered goimports to
think that it was an invalid module, usable only as a replace target,
because its declared module path "std" didn't match its apparent path
"src". The stdlib is always in scope, so skip the needs-replace check
for GOROOT.

Fixes golang/go#34199

Change-Id: I1199378b940cfedc04e9a4e943c46b9ffdf18446
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194570
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@golang golang locked and limited conversation to collaborators Sep 9, 2020
@rsc rsc unassigned heschi Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants