diff options
author | Bryan C. Mills <bcmills@google.com> | 2019-07-08 18:13:23 -0400 |
---|---|---|
committer | Bryan C. Mills <bcmills@google.com> | 2020-02-28 19:09:53 +0000 |
commit | 5a61de3fe160cc8b327ee893cd74c4d0ce9dc13d (patch) | |
tree | 3dbf0328ac9de682fd44c47cd28b600a2cacbc84 /src/cmd/go/internal/load/pkg.go | |
parent | d11e1f92fc578c5d2e604acfe9ea60d7afb84a0c (diff) | |
download | go-5a61de3fe160cc8b327ee893cd74c4d0ce9dc13d.tar.gz go-5a61de3fe160cc8b327ee893cd74c4d0ce9dc13d.zip |
cmd/go: rationalize errors in internal/load and internal/modload
This change is a non-minimal fix for #32917, but incidentally fixes
several other bugs and makes the error messages much more ergonomic.
Updates #32917
Updates #27122
Updates #28459
Updates #29280
Updates #30590
Updates #37214
Updates #36173
Updates #36587
Fixes #36008
Fixes #30992
Change-Id: Iedb26d2e0963697c130df5d0f72e7f83ec2dcf06
Reviewed-on: https://go-review.googlesource.com/c/go/+/185345
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Diffstat (limited to 'src/cmd/go/internal/load/pkg.go')
-rw-r--r-- | src/cmd/go/internal/load/pkg.go | 91 |
1 files changed, 63 insertions, 28 deletions
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 9bf7c228b7..21dcee1315 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -187,20 +187,17 @@ type PackageInternal struct { Gccgoflags []string // -gccgoflags for this package } +// A NoGoError indicates that no Go files for the package were applicable to the +// build for that package. +// +// That may be because there were no files whatsoever, or because all files were +// excluded, or because all non-excluded files were test sources. type NoGoError struct { Package *Package } func (e *NoGoError) Error() string { - // Count files beginning with _ and ., which we will pretend don't exist at all. - dummy := 0 - for _, name := range e.Package.IgnoredGoFiles { - if strings.HasPrefix(name, "_") || strings.HasPrefix(name, ".") { - dummy++ - } - } - - if len(e.Package.IgnoredGoFiles) > dummy { + if len(e.Package.constraintIgnoredGoFiles()) > 0 { // Go files exist, but they were ignored due to build constraints. return "build constraints exclude all Go files in " + e.Package.Dir } @@ -213,6 +210,23 @@ func (e *NoGoError) Error() string { return "no Go files in " + e.Package.Dir } +// rewordError returns a version of err with trivial layers removed and +// (possibly-wrapped) instances of build.NoGoError replaced with load.NoGoError, +// which more clearly distinguishes sub-cases. +func (p *Package) rewordError(err error) error { + if mErr, ok := err.(*search.MatchError); ok && mErr.Match.IsLiteral() { + err = mErr.Err + } + var noGo *build.NoGoError + if errors.As(err, &noGo) { + if p.Dir == "" && noGo.Dir != "" { + p.Dir = noGo.Dir + } + err = &NoGoError{Package: p} + } + return err +} + // Resolve returns the resolved version of imports, // which should be p.TestImports or p.XTestImports, NOT p.Imports. // The imports in p.TestImports and p.XTestImports are not recursively @@ -313,10 +327,7 @@ type PackageError struct { func (p *PackageError) Error() string { // Import cycles deserve special treatment. - if p.IsImportCycle { - return fmt.Sprintf("%s\npackage %s\n", p.Err, strings.Join(p.ImportStack, "\n\timports ")) - } - if p.Pos != "" { + if p.Pos != "" && !p.IsImportCycle { // Omit import stack. The full path to the file where the error // is the most important thing. return p.Pos + ": " + p.Err.Error() @@ -339,6 +350,8 @@ func (p *PackageError) Error() string { return "package " + strings.Join(stack, "\n\timports ") + ": " + p.Err.Error() } +func (p *PackageError) Unwrap() error { return p.Err } + // PackageError implements MarshalJSON so that Err is marshaled as a string // and non-essential fields are omitted. func (p *PackageError) MarshalJSON() ([]byte, error) { @@ -583,9 +596,10 @@ func loadImport(pre *preload, path, srcDir string, parent *Package, stk *ImportS if !cfg.ModulesEnabled && path != cleanImport(path) { p.Error = &PackageError{ ImportStack: stk.Copy(), - Err: fmt.Errorf("non-canonical import path: %q should be %q", path, pathpkg.Clean(path)), + Err: ImportErrorf(path, "non-canonical import path %q: should be %q", path, pathpkg.Clean(path)), } p.Incomplete = true + setErrorPos(p, importPos) } } @@ -1533,12 +1547,8 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) { } if err != nil { - if _, ok := err.(*build.NoGoError); ok { - err = &NoGoError{Package: p} - } p.Incomplete = true - - setError(base.ExpandScanner(err)) + setError(base.ExpandScanner(p.rewordError(err))) if _, isScanErr := err.(scanner.ErrorList); !isScanErr { return } @@ -1934,13 +1944,22 @@ func (p *Package) InternalXGoFiles() []string { // using absolute paths. "Possibly relevant" means that files are not excluded // due to build tags, but files with names beginning with . or _ are still excluded. func (p *Package) InternalAllGoFiles() []string { - var extra []string + return p.mkAbs(str.StringList(p.constraintIgnoredGoFiles(), p.GoFiles, p.CgoFiles, p.TestGoFiles, p.XTestGoFiles)) +} + +// constraintIgnoredGoFiles returns the list of Go files ignored for reasons +// other than having a name beginning with '.' or '_'. +func (p *Package) constraintIgnoredGoFiles() []string { + if len(p.IgnoredGoFiles) == 0 { + return nil + } + files := make([]string, 0, len(p.IgnoredGoFiles)) for _, f := range p.IgnoredGoFiles { - if f != "" && f[0] != '.' || f[0] != '_' { - extra = append(extra, f) + if f != "" && f[0] != '.' && f[0] != '_' { + files = append(files, f) } } - return p.mkAbs(str.StringList(extra, p.GoFiles, p.CgoFiles, p.TestGoFiles, p.XTestGoFiles)) + return files } // usesSwig reports whether the package needs to run SWIG. @@ -2034,7 +2053,7 @@ func Packages(args []string) []*Package { var pkgs []*Package for _, pkg := range PackagesAndErrors(args) { if pkg.Error != nil { - base.Errorf("can't load package: %s", pkg.Error) + base.Errorf("%v", pkg.Error) continue } pkgs = append(pkgs, pkg) @@ -2092,8 +2111,24 @@ func PackagesAndErrors(patterns []string) []*Package { pkgs = append(pkgs, p) } - // TODO: if len(m.Pkgs) == 0 && len(m.Errs) > 0, should we add a *Package - // with a non-nil Error field? + if len(m.Errs) > 0 { + // In addition to any packages that were actually resolved from the + // pattern, there was some error in resolving the pattern itself. + // Report it as a synthetic package. + p := new(Package) + p.ImportPath = m.Pattern() + p.Error = &PackageError{ + ImportStack: nil, // The error arose from a pattern, not an import. + Err: p.rewordError(m.Errs[0]), + } + p.Incomplete = true + p.Match = append(p.Match, m.Pattern()) + p.Internal.CmdlinePkg = true + if m.IsLiteral() { + p.Internal.CmdlinePkgLiteral = true + } + pkgs = append(pkgs, p) + } } // Now that CmdlinePkg is set correctly, @@ -2129,7 +2164,7 @@ func PackagesForBuild(args []string) []*Package { printed := map[*PackageError]bool{} for _, pkg := range pkgs { if pkg.Error != nil { - base.Errorf("can't load package: %s", pkg.Error) + base.Errorf("%v", pkg.Error) printed[pkg.Error] = true } for _, err := range pkg.DepsErrors { @@ -2139,7 +2174,7 @@ func PackagesForBuild(args []string) []*Package { // Only print each once. if !printed[err] { printed[err] = true - base.Errorf("%s", err) + base.Errorf("%v", err) } } } |