diff --git a/go/packages/golist.go b/go/packages/golist.go index 31d669765b..f098905dfb 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -273,43 +273,16 @@ func (state *golistState) runContainsQueries(response *responseDeduper, queries return fmt.Errorf("could not determine absolute path of file= query path %q: %v", query, err) } dirResponse, err := state.createDriverResponse(pattern) - if err != nil || (len(dirResponse.Packages) == 1 && len(dirResponse.Packages[0].Errors) == 1) { - // There was an error loading the package. Try to load the file as an ad-hoc package. - // Usually the error will appear in a returned package, but may not if we're in modules mode - // and the ad-hoc is located outside a module. + + // If there was an error loading the package, or the package is returned + // with errors, try to load the file as an ad-hoc package. + // Usually the error will appear in a returned package, but may not if we're + // in module mode and the ad-hoc is located outside a module. + if err != nil || len(dirResponse.Packages) == 1 && len(dirResponse.Packages[0].GoFiles) == 0 && + len(dirResponse.Packages[0].Errors) == 1 { var queryErr error - dirResponse, queryErr = state.createDriverResponse(query) - if queryErr != nil { - // Return the original error if the attempt to fall back failed. - return err - } - // If we get nothing back from `go list`, try to make this file into its own ad-hoc package. - if len(dirResponse.Packages) == 0 && queryErr == nil { - dirResponse.Packages = append(dirResponse.Packages, &Package{ - ID: "command-line-arguments", - PkgPath: query, - GoFiles: []string{query}, - CompiledGoFiles: []string{query}, - Imports: make(map[string]*Package), - }) - dirResponse.Roots = append(dirResponse.Roots, "command-line-arguments") - } - // Special case to handle issue #33482: - // If this is a file= query for ad-hoc packages where the file only exists on an overlay, - // and exists outside of a module, add the file in for the package. - if len(dirResponse.Packages) == 1 && (dirResponse.Packages[0].ID == "command-line-arguments" || - filepath.ToSlash(dirResponse.Packages[0].PkgPath) == filepath.ToSlash(query)) { - if len(dirResponse.Packages[0].GoFiles) == 0 { - filename := filepath.Join(pattern, filepath.Base(query)) // avoid recomputing abspath - // TODO(matloob): check if the file is outside of a root dir? - for path := range state.cfg.Overlay { - if path == filename { - dirResponse.Packages[0].Errors = nil - dirResponse.Packages[0].GoFiles = []string{path} - dirResponse.Packages[0].CompiledGoFiles = []string{path} - } - } - } + if dirResponse, queryErr = state.adhocPackage(pattern, query); queryErr != nil { + return err // return the original error } } isRoot := make(map[string]bool, len(dirResponse.Roots)) @@ -337,6 +310,49 @@ func (state *golistState) runContainsQueries(response *responseDeduper, queries return nil } +// adhocPackage attempts to load or construct an ad-hoc package for a given +// query, if the original call to the driver produced inadequate results. +func (state *golistState) adhocPackage(pattern, query string) (*driverResponse, error) { + response, err := state.createDriverResponse(query) + if err != nil { + return nil, err + } + // If we get nothing back from `go list`, + // try to make this file into its own ad-hoc package. + // TODO(rstambler): Should this check against the original response? + if len(response.Packages) == 0 { + response.Packages = append(response.Packages, &Package{ + ID: "command-line-arguments", + PkgPath: query, + GoFiles: []string{query}, + CompiledGoFiles: []string{query}, + Imports: make(map[string]*Package), + }) + response.Roots = append(response.Roots, "command-line-arguments") + } + // Handle special cases. + if len(response.Packages) == 1 { + // golang/go#33482: If this is a file= query for ad-hoc packages where + // the file only exists on an overlay, and exists outside of a module, + // add the file to the package and remove the errors. + if response.Packages[0].ID == "command-line-arguments" || + filepath.ToSlash(response.Packages[0].PkgPath) == filepath.ToSlash(query) { + if len(response.Packages[0].GoFiles) == 0 { + filename := filepath.Join(pattern, filepath.Base(query)) // avoid recomputing abspath + // TODO(matloob): check if the file is outside of a root dir? + for path := range state.cfg.Overlay { + if path == filename { + response.Packages[0].Errors = nil + response.Packages[0].GoFiles = []string{path} + response.Packages[0].CompiledGoFiles = []string{path} + } + } + } + } + } + return response, nil +} + // Fields must match go list; // see $GOROOT/src/cmd/go/internal/load/pkg.go. type jsonPackage struct { diff --git a/go/packages/golist_overlay.go b/go/packages/golist_overlay.go index bc73fd69ee..fe77a11b40 100644 --- a/go/packages/golist_overlay.go +++ b/go/packages/golist_overlay.go @@ -116,6 +116,11 @@ func (state *golistState) processGolistOverlay(response *responseDeduper) (modif if isTestFile && !isXTest && testVariantOf != nil { pkg.GoFiles = append(pkg.GoFiles, testVariantOf.GoFiles...) pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, testVariantOf.CompiledGoFiles...) + // Add the package under test and its imports to the test variant. + pkg.forTest = testVariantOf.PkgPath + for k, v := range testVariantOf.Imports { + pkg.Imports[k] = &Package{ID: v.ID} + } } } } diff --git a/go/packages/packages114_test.go b/go/packages/packages114_test.go new file mode 100644 index 0000000000..1b0a824986 --- /dev/null +++ b/go/packages/packages114_test.go @@ -0,0 +1,77 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build go1.14 + +package packages_test + +import ( + "fmt" + "path/filepath" + "testing" + + "golang.org/x/tools/go/packages" + "golang.org/x/tools/go/packages/packagestest" +) + +func TestInvalidFilesInOverlay(t *testing.T) { packagestest.TestAll(t, testInvalidFilesInOverlay) } +func testInvalidFilesInOverlay(t *testing.T, exporter packagestest.Exporter) { + exported := packagestest.Export(t, exporter, []packagestest.Module{ + { + Name: "golang.org/fake", + Files: map[string]interface{}{ + "d/d.go": `package d; import "net/http"; const d = http.MethodGet;`, + "d/util.go": ``, + "d/d_test.go": ``, + }, + }, + }) + defer exported.Cleanup() + + dir := filepath.Dir(filepath.Dir(exported.File("golang.org/fake", "d/d.go"))) + + // Additional tests for test variants. + for i, tt := range []struct { + name string + overlay map[string][]byte + want string // expected value of d.D + + }{ + // Overlay with a test variant. + {"test_variant", + map[string][]byte{ + filepath.Join(dir, "d", "d_test.go"): []byte(`package d; import "testing"; const D = d + "_test"; func TestD(t *testing.T) {};`)}, + `"GET_test"`}, + // Overlay in package. + {"second_file", + map[string][]byte{ + filepath.Join(dir, "d", "util.go"): []byte(`package d; const D = d + "_util";`)}, + `"GET_util"`}, + } { + t.Run(tt.name, func(t *testing.T) { + exported.Config.Overlay = tt.overlay + exported.Config.Mode = packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | + packages.NeedDeps | packages.NeedTypes | packages.NeedTypesSizes + exported.Config.Tests = true + + for f := range tt.overlay { + initial, err := packages.Load(exported.Config, fmt.Sprintf("file=%s", f)) + if err != nil { + t.Fatal(err) + } + d := initial[0] + // Check value of d.D. + dD := constant(d, "D") + if dD == nil { + t.Fatalf("%d. d.D: got nil", i) + } + got := dD.Val().String() + if got != tt.want { + t.Fatalf("%d. d.D: got %s, want %s", i, got, tt.want) + } + } + }) + + } +} diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index 56462e1e93..30ad8c7b55 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -44,16 +44,18 @@ func sourceError(ctx context.Context, fset *token.FileSet, pkg *pkg, e interface if e.Pos == "" { spn = parseGoListError(e.Msg) + + // We may not have been able to parse a valid span. + if _, err := spanToRange(ctx, pkg, spn); err != nil { + return &source.Error{ + URI: spn.URI(), + Message: msg, + Kind: kind, + }, nil + } } else { spn = span.Parse(e.Pos) } - // If the range can't be derived from the parseGoListError function, then we do not have a valid position. - if _, err := spanToRange(ctx, pkg, spn); err != nil && e.Pos == "" { - return &source.Error{ - Message: msg, - Kind: kind, - }, nil - } case *scanner.Error: msg = e.Msg kind = source.ParseError