qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-09-30 22:48:32 -06:00
Code Releases Activity

go/packages: handle invalid files in overlays

Browse Source 
This change handles a specific case where `go list` returns an error on
a file that lacks a package declaration. If only one package is returned
as a response, we should add that file to that package, since it may
have valid content in an overlay.

This change uses the internal/span package to correctly parse the error
message from `go list`.

Change-Id: I5909c4fd765746df1003685aa915b7c5f9cdcee5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201220
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Rebecca Stambler 2019-10-15 17:11:27 -04:00
parent f0068bd333
commit 6f5e27347a
2 changed files with 140 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

113
go/packages/golist.go
View File

@ -26,6 +26,7 @@ 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.
@ -281,31 +282,42 @@ 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 || (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 err != nil {
var queryErr error
dirResponse, queryErr = driver(cfg, query)
if queryErr != nil {
// Return the original error if the attempt to fall back failed.
return err
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" || 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}
}
}
}
// `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:]...)
}
}
}
// 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
}
}
isRoot := make(map[string]bool, len(dirResponse.Roots))
@ -333,6 +345,63 @@ 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
}
// 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" || 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(`(.*)@([^/\\]*)(.*)`)

49
go/packages/packages_test.go
View File

@ -1079,6 +1079,51 @@ 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) {
// TODO: Enable this test when github.com/golang/go/issues/33374 is resolved.
t.Skip()
@ -2201,10 +2246,14 @@ 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)
}