go/packages: expand cases when filenames are parsed from errors
Only add files in errors if either the error's import stack is empty,
or the import stack's top package is the same as the package itself,
so we have more confidence that the error applies to the package.
This is bound to be flaky, but shouldn't be worse than the current
state. (And it unblocks a cl from going into the RC...).
Updates golang/go#40544
Change-Id: Ie21a8abec7150800d3d34b94a7ec90fd40d93fe9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246758
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/go/packages/golist.go b/go/packages/golist.go
index 220d409..b4d232b 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -89,6 +89,10 @@
rootDirsError error
rootDirs map[string]string
+ goVersionOnce sync.Once
+ goVersionError error
+ goVersion string // third field of 'go version'
+
// vendorDirs caches the (non)existence of vendor directories.
vendorDirs map[string]bool
}
@@ -638,7 +642,7 @@
// Temporary work-around for golang/go#39986. Parse filenames out of
// error messages. This happens if there are unrecoverable syntax
// errors in the source, so we can't match on a specific error message.
- if err := p.Error; err != nil && len(err.ImportStack) == 0 && len(pkg.CompiledGoFiles) == 0 {
+ if err := p.Error; err != nil && state.shouldAddFilenameFromError(p) {
addFilenameFromPos := func(pos string) bool {
split := strings.Split(pos, ":")
if len(split) < 1 {
@@ -697,6 +701,58 @@
return &response, nil
}
+func (state *golistState) shouldAddFilenameFromError(p *jsonPackage) bool {
+ if len(p.GoFiles) > 0 || len(p.CompiledGoFiles) > 0 {
+ return false
+ }
+
+ goV, err := state.getGoVersion()
+ if err != nil {
+ return false
+ }
+
+ // On Go 1.14 and earlier, only add filenames from errors if the import stack is empty.
+ // The import stack behaves differently for these versions than newer Go versions.
+ if strings.HasPrefix(goV, "go1.13") || strings.HasPrefix(goV, "go1.14") {
+ return len(p.Error.ImportStack) == 0
+ }
+
+ // On Go 1.15 and later, only parse filenames out of error if there's no import stack,
+ // or the current package is at the top of the import stack. This is not guaranteed
+ // to work perfectly, but should avoid some cases where files in errors don't belong to this
+ // package.
+ return len(p.Error.ImportStack) == 0 || p.Error.ImportStack[len(p.Error.ImportStack)-1] == p.ImportPath
+}
+
+func (state *golistState) getGoVersion() (string, error) {
+ state.goVersionOnce.Do(func() {
+ var b *bytes.Buffer
+ // Invoke go version. Don't use invokeGo because it will supply build flags, and
+ // go version doesn't expect build flags.
+ inv := gocommand.Invocation{
+ Verb: "version",
+ Env: state.cfg.Env,
+ Logf: state.cfg.Logf,
+ }
+ gocmdRunner := state.cfg.gocmdRunner
+ if gocmdRunner == nil {
+ gocmdRunner = &gocommand.Runner{}
+ }
+ b, _, _, state.goVersionError = gocmdRunner.RunRaw(state.cfg.Context, inv)
+ if state.goVersionError != nil {
+ return
+ }
+
+ sp := strings.Split(b.String(), " ")
+ if len(sp) < 3 {
+ state.goVersionError = fmt.Errorf("go version output: expected 'go version <version>', got '%s'", b.String())
+ return
+ }
+ state.goVersion = sp[2]
+ })
+ return state.goVersion, state.goVersionError
+}
+
// getPkgPath finds the package path of a directory if it's relative to a root directory.
func (state *golistState) getPkgPath(dir string) (string, bool, error) {
absDir, err := filepath.Abs(dir)
