mirror of
https://github.com/golang/go
synced 2024-11-18 17:54:57 -07:00
go/packages: revert "handle invalid files in overlays"
This reverts commit 6f5e27347a, golang.org/cl/201220.
Reason for revert: Produces unusable Package results. See golang/go#35949 and golang/go#35973.
Fixes golang/go#35949.
Change-Id: Ic4632fe7f00366b1edf59840c83160d66499ba12
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209978
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Heschi Kreinick
parent
d79e56da46
commit
427c522ce2
@ -26,7 +26,6 @@ import (
|
||||
"golang.org/x/tools/go/internal/packagesdriver"
|
||||
"golang.org/x/tools/internal/gopathwalk"
|
||||
"golang.org/x/tools/internal/semver"
|
||||
"golang.org/x/tools/internal/span"
|
||||
)
|
||||
|
||||
// debug controls verbose logging.
|
||||
@ -284,42 +283,43 @@ func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, q
|
||||
return fmt.Errorf("could not determine absolute path of file= query path %q: %v", query, err)
|
||||
}
|
||||
dirResponse, err := driver(cfg, pattern)
|
||||
if err != nil {
|
||||
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.
|
||||
var queryErr error
|
||||
if dirResponse, queryErr = adHocPackage(cfg, driver, pattern, query); queryErr != nil {
|
||||
return err // return the original error
|
||||
dirResponse, queryErr = driver(cfg, query)
|
||||
if queryErr != nil {
|
||||
// Return the original error if the attempt to fall back failed.
|
||||
return err
|
||||
}
|
||||
}
|
||||
// `go list` can report errors for files that are not listed as part of a package's GoFiles.
|
||||
// In the case of an invalid Go file, we should assume that it is part of package if only
|
||||
// one package is in the response. The file may have valid contents in an overlay.
|
||||
if len(dirResponse.Packages) == 1 {
|
||||
pkg := dirResponse.Packages[0]
|
||||
for i, err := range pkg.Errors {
|
||||
s := errorSpan(err)
|
||||
if !s.IsValid() {
|
||||
break
|
||||
}
|
||||
if len(pkg.CompiledGoFiles) == 0 {
|
||||
break
|
||||
}
|
||||
dir := filepath.Dir(pkg.CompiledGoFiles[0])
|
||||
filename := filepath.Join(dir, filepath.Base(s.URI().Filename()))
|
||||
if info, err := os.Stat(filename); err != nil || info.IsDir() {
|
||||
break
|
||||
}
|
||||
if !contains(pkg.CompiledGoFiles, filename) {
|
||||
pkg.CompiledGoFiles = append(pkg.CompiledGoFiles, filename)
|
||||
pkg.GoFiles = append(pkg.GoFiles, filename)
|
||||
pkg.Errors = append(pkg.Errors[:i], pkg.Errors[i+1:]...)
|
||||
}
|
||||
// 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")
|
||||
}
|
||||
}
|
||||
// A final attempt to construct an ad-hoc package.
|
||||
if len(dirResponse.Packages) == 1 && len(dirResponse.Packages[0].Errors) == 1 {
|
||||
var queryErr error
|
||||
if dirResponse, queryErr = adHocPackage(cfg, driver, pattern, query); queryErr != nil {
|
||||
return err // return the original error
|
||||
// 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 cfg.Overlay {
|
||||
if path == filename {
|
||||
dirResponse.Packages[0].Errors = nil
|
||||
dirResponse.Packages[0].GoFiles = []string{path}
|
||||
dirResponse.Packages[0].CompiledGoFiles = []string{path}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
isRoot := make(map[string]bool, len(dirResponse.Roots))
|
||||
@ -347,75 +347,6 @@ func runContainsQueries(cfg *Config, driver driver, response *responseDeduper, q
|
||||
return nil
|
||||
}
|
||||
|
||||
// adHocPackage attempts to construct an ad-hoc package given a query that failed.
|
||||
func adHocPackage(cfg *Config, driver driver, pattern, query string) (*driverResponse, error) {
|
||||
// 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.
|
||||
dirResponse, err := driver(cfg, 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.
|
||||
if len(dirResponse.Packages) == 0 && err == 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 cfg.Overlay {
|
||||
if path == filename {
|
||||
dirResponse.Packages[0].Errors = nil
|
||||
dirResponse.Packages[0].GoFiles = []string{path}
|
||||
dirResponse.Packages[0].CompiledGoFiles = []string{path}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return dirResponse, nil
|
||||
}
|
||||
|
||||
func contains(files []string, filename string) bool {
|
||||
for _, f := range files {
|
||||
if f == filename {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// errorSpan attempts to parse a standard `go list` error message
|
||||
// by stripping off the trailing error message.
|
||||
//
|
||||
// It works only on errors whose message is prefixed by colon,
|
||||
// followed by a space (": "). For example:
|
||||
//
|
||||
// attributes.go:13:1: expected 'package', found 'type'
|
||||
//
|
||||
func errorSpan(err Error) span.Span {
|
||||
if err.Pos == "" {
|
||||
input := strings.TrimSpace(err.Msg)
|
||||
msgIndex := strings.Index(input, ": ")
|
||||
if msgIndex < 0 {
|
||||
return span.Parse(input)
|
||||
}
|
||||
return span.Parse(input[:msgIndex])
|
||||
}
|
||||
return span.Parse(err.Pos)
|
||||
}
|
||||
|
||||
// modCacheRegexp splits a path in a module cache into module, module version, and package.
|
||||
var modCacheRegexp = regexp.MustCompile(`(.*)@([^/\\]*)(.*)`)
|
||||
|
||||
|
||||
@ -1099,51 +1099,6 @@ func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestContainsInOverlays(t *testing.T) { packagestest.TestAll(t, testcontainsInOverlays) }
|
||||
func testcontainsInOverlays(t *testing.T, exporter packagestest.Exporter) {
|
||||
// This test checks a specific case where a file is empty on disk.
|
||||
// In this case, `go list` will return the package golang.org/fake/c
|
||||
// with only c.go as a GoFile, with an error message for c2.go.
|
||||
// Since there is only one possible package for c2.go to be a part of,
|
||||
// go/packages will parse the filename out of error and add it to GoFiles and CompiledGoFiles.
|
||||
exported := packagestest.Export(t, exporter, []packagestest.Module{{
|
||||
Name: "golang.org/fake",
|
||||
Files: map[string]interface{}{
|
||||
"c/c.go": `package c; const C = "c"`,
|
||||
"c/c2.go": ``,
|
||||
}}})
|
||||
defer exported.Cleanup()
|
||||
|
||||
dir := filepath.Dir(exported.File("golang.org/fake", "c/c.go"))
|
||||
|
||||
for _, test := range []struct {
|
||||
overlay map[string][]byte
|
||||
want string
|
||||
}{
|
||||
{
|
||||
overlay: map[string][]byte{filepath.Join(dir, "c2.go"): []byte(`nonsense`)},
|
||||
want: "golang.org/fake/c",
|
||||
},
|
||||
} {
|
||||
exported.Config.Overlay = test.overlay
|
||||
exported.Config.Mode = packages.LoadImports
|
||||
exported.Config.Logf = t.Logf
|
||||
for filename := range exported.Config.Overlay {
|
||||
initial, err := packages.Load(exported.Config, "file="+filename)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(initial) == 0 {
|
||||
t.Fatalf("no packages for %s", filename)
|
||||
}
|
||||
pkg := initial[0]
|
||||
if pkg.PkgPath != test.want {
|
||||
t.Errorf("got %s, want %s", pkg.PkgPath, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAdHocPackagesBadImport(t *testing.T) {
|
||||
// This test doesn't use packagestest because we are testing ad-hoc packages,
|
||||
// which are outside of $GOPATH and outside of a module.
|
||||
@ -1210,36 +1165,38 @@ const A = 1
|
||||
|
||||
// Make sure that the user's value of GO111MODULE does not affect test results.
|
||||
for _, go111module := range []string{"off", "auto", "on"} {
|
||||
config := &packages.Config{
|
||||
Dir: tmp,
|
||||
Env: append(os.Environ(), "GOPACKAGESDRIVER=off", fmt.Sprintf("GO111MODULE=%s", go111module)),
|
||||
Mode: packages.LoadAllSyntax,
|
||||
Overlay: map[string][]byte{
|
||||
filename: content,
|
||||
},
|
||||
Logf: t.Logf,
|
||||
}
|
||||
initial, err := packages.Load(config, fmt.Sprintf("file=%s", filename))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(initial) == 0 {
|
||||
t.Fatalf("no packages for %s with GO111MODULE=%s", filename, go111module)
|
||||
}
|
||||
// Check value of a.A.
|
||||
a := initial[0]
|
||||
if a.Errors != nil {
|
||||
t.Fatalf("a: got errors %+v, want no error", err)
|
||||
}
|
||||
aA := constant(a, "A")
|
||||
if aA == nil {
|
||||
t.Errorf("a.A: got nil")
|
||||
return
|
||||
}
|
||||
got := aA.Val().String()
|
||||
if want := "1"; got != want {
|
||||
t.Errorf("a.A: got %s, want %s", got, want)
|
||||
}
|
||||
t.Run("GO111MODULE="+go111module, func(t *testing.T) {
|
||||
config := &packages.Config{
|
||||
Dir: tmp,
|
||||
Env: append(os.Environ(), "GOPACKAGESDRIVER=off", fmt.Sprintf("GO111MODULE=%s", go111module)),
|
||||
Mode: packages.LoadAllSyntax,
|
||||
Overlay: map[string][]byte{
|
||||
filename: content,
|
||||
},
|
||||
Logf: t.Logf,
|
||||
}
|
||||
initial, err := packages.Load(config, fmt.Sprintf("file=%s", filename))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(initial) == 0 {
|
||||
t.Fatalf("no packages for %s", filename)
|
||||
}
|
||||
// Check value of a.A.
|
||||
a := initial[0]
|
||||
if a.Errors != nil {
|
||||
t.Fatalf("a: got errors %+v, want no error", err)
|
||||
}
|
||||
aA := constant(a, "A")
|
||||
if aA == nil {
|
||||
t.Errorf("a.A: got nil")
|
||||
return
|
||||
}
|
||||
got := aA.Val().String()
|
||||
if want := "1"; got != want {
|
||||
t.Errorf("a.A: got %s, want %s", got, want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@ -2273,14 +2230,10 @@ func testAdHocContains(t *testing.T, exporter packagestest.Exporter) {
|
||||
}()
|
||||
|
||||
exported.Config.Mode = packages.NeedImports | packages.NeedFiles
|
||||
exported.Config.Logf = t.Logf
|
||||
pkgs, err := packages.Load(exported.Config, "file="+filename)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(pkgs) == 0 {
|
||||
t.Fatalf("no packages for %s", filename)
|
||||
}
|
||||
if len(pkgs) != 1 && pkgs[0].PkgPath != "command-line-arguments" {
|
||||
t.Fatalf("packages.Load: want [command-line-arguments], got %v", pkgs)
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user