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

Jump to bottom

x/tools/go/packages: passing NeedSyntax to Load() doesn't populate Syntax field unless NeedTypes is passed #35331

Closed
nevkontakte opened this issue Nov 3, 2019 · 6 comments
Closed

x/tools/go/packages: passing NeedSyntax to Load() doesn't populate Syntax field unless NeedTypes is passed #35331

nevkontakte opened this issue Nov 3, 2019 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@nevkontakte
Copy link
Contributor

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

$ go version
go version go1.12.10 linux/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
 $ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/aleks/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/aleks/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build037965907=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Calling packages.Load() with packages.NeedSyntax doesn't actually populate package's Syntax field, unless packages.NeedTypes is specified at the same time. This behavior seems to contradict documentation:

 // Syntax is the package's syntax trees, for the files listed in CompiledGoFiles.
//
// The NeedSyntax LoadMode bit populates this field for packages matching the patterns.
// If NeedDeps and NeedImports are also set, this field will also be populated
// for dependencies.
Syntax []*ast.File

This makes it impossible to load and parse a package without simultaneously type-checking it, which may not always be desirable.

Minimal example to reproduce 
package main

import (
        "go/token"
        "log"

        "golang.org/x/tools/go/packages"
)

func main() {
        cfg := &packages.Config{
                Mode: packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles |
                   packages.NeedImports | packages.NeedDeps | packages.NeedSyntax,
                Fset: token.NewFileSet(),
        }
        pkgs, err := packages.Load(cfg, "strconv")
        if err != nil {
                log.Fatalf("package.Load() returned error: %s", err)
                return
        }
        if count := packages.PrintErrors(pkgs); count > 0 {
                log.Fatalf("Encountered errors while loading, can't continue.")
                return
        }

        packages.Visit(pkgs, func(pkg *packages.Package) bool {
                log.Printf("Package %q has %d files parsed.", pkg, len(pkg.Syntax))
                return true
        }, nil)
}

What did you expect to see?

 $ go run main.go
2019/11/03 19:36:07 Package "strconv" has 10 files parsed.
2019/11/03 19:36:07 Package "errors" has 1 files parsed.
2019/11/03 19:36:07 Package "internal/bytealg" has 7 files parsed.
2019/11/03 19:36:07 Package "internal/cpu" has 3 files parsed.
2019/11/03 19:36:07 Package "unsafe" has 0 files parsed.
2019/11/03 19:36:07 Package "math" has 45 files parsed.
2019/11/03 19:36:07 Package "math/bits" has 2 files parsed.
2019/11/03 19:36:07 Package "unicode/utf8" has 1 files parsed.

What did you see instead?

 $ go run main.go
2019/11/03 19:35:26 Package "strconv" has 0 files parsed.
2019/11/03 19:35:26 Package "errors" has 0 files parsed.
2019/11/03 19:35:26 Package "internal/bytealg" has 0 files parsed.
2019/11/03 19:35:26 Package "internal/cpu" has 0 files parsed.
2019/11/03 19:35:26 Package "unsafe" has 0 files parsed.
2019/11/03 19:35:26 Package "math" has 0 files parsed.
2019/11/03 19:35:26 Package "math/bits" has 0 files parsed.
2019/11/03 19:35:26 Package "unicode/utf8" has 0 files parsed.
@gopherbot gopherbot added this to the Unreleased milestone Nov 3, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 3, 2019
@nevkontakte nevkontakte mentioned this issue Nov 3, 2019
Closed
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 3, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Nov 3, 2019 •
edited

/cc @matloob per owners.

@gopherbot
Copy link

Change https://golang.org/cl/205160 mentions this issue: go/packages: fix loading of syntax when types not requested

@nevkontakte
Copy link
Contributor Author

Thanks for a super-quick fix!

@matloob
Copy link
Contributor

matloob commented Nov 4, 2019

Thanks for finding the issue!

@nevkontakte
Copy link
Contributor Author

nevkontakte commented Nov 23, 2019 •
edited

Looks like this is not fully fixed... In particular, mode packages.NeedImports | packages.NeedDeps | packages.NeedSyntax doesn't behave the way I'd expect: the Syntax field is only populated for the package itself, but not any of the dependencies.

The example in my original comment currently produces this output:

2019/11/23 17:41:51 Package "strconv" has 10 files parsed.
2019/11/23 17:41:51 Package "errors" has 0 files parsed.
2019/11/23 17:41:51 Package "internal/bytealg" has 0 files parsed.
2019/11/23 17:41:51 Package "internal/cpu" has 0 files parsed.
2019/11/23 17:41:51 Package "unsafe" has 0 files parsed.
2019/11/23 17:41:51 Package "math" has 0 files parsed.
2019/11/23 17:41:51 Package "math/bits" has 0 files parsed.
2019/11/23 17:41:51 Package "unicode/utf8" has 0 files parsed.

@nevkontakte
Copy link
Contributor Author

I think I've found a fix (golang/tools#189).

@golang golang locked and limited conversation to collaborators Nov 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@nevkontakte @dmitshur @gopherbot @matloob