From 031854117f91fbc1265840a04caf2a7a168dd06b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 5 May 2021 17:08:39 -0400 Subject: [PATCH] cmd/go: include packages with InvalidGoFiles when filtering main packages If a package has files with conflicting package names, go/build empirically populates the first name encountered and puts the remaining files in InvalidGoFiles. That foiled our check for packages whose name is either unpopulated or "main", since the "package main" could be found in a source file after the first. Instead, we now treat any package with a nonzero set of InvalidGoFiles as potentially a main package. This biases toward over-reporting errors, but we would rather over-report than under-report. If we fix #45999, we will be able to make these error checks more precise. Updates #42088 Fixes #45827 Fixes #39986 Change-Id: I588314341b17961b38660192c2130678dc03023e Reviewed-on: https://go-review.googlesource.com/c/go/+/317300 Trust: Bryan C. Mills Reviewed-by: Jay Conrod --- src/cmd/go/internal/load/pkg.go | 80 +++++++++++-------- .../script/mod_install_pkg_version.txt | 2 +- .../go/testdata/script/mod_run_nonmain.txt | 18 +++++ .../go/testdata/script/mod_run_pkgerror.txt | 20 ++++- 4 files changed, 84 insertions(+), 36 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_run_nonmain.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 153399d83e6..3c7cd44ee33 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -87,6 +87,7 @@ type PackagePublic struct { CgoFiles []string `json:",omitempty"` // .go source files that import "C" CompiledGoFiles []string `json:",omitempty"` // .go output from running cgo on CgoFiles IgnoredGoFiles []string `json:",omitempty"` // .go source files ignored due to build constraints + InvalidGoFiles []string `json:",omitempty"` // .go source files with detected problems (parse error, wrong package name, and so on) IgnoredOtherFiles []string `json:",omitempty"` // non-.go source files ignored due to build constraints CFiles []string `json:",omitempty"` // .c source files CXXFiles []string `json:",omitempty"` // .cc, .cpp and .cxx source files @@ -144,6 +145,7 @@ func (p *Package) AllFiles() []string { p.CgoFiles, // no p.CompiledGoFiles, because they are from GoFiles or generated by us p.IgnoredGoFiles, + // no p.InvalidGoFiles, because they are from GoFiles p.IgnoredOtherFiles, p.CFiles, p.CXXFiles, @@ -371,6 +373,7 @@ func (p *Package) copyBuild(opts PackageOpts, pp *build.Package) { p.GoFiles = pp.GoFiles p.CgoFiles = pp.CgoFiles p.IgnoredGoFiles = pp.IgnoredGoFiles + p.InvalidGoFiles = pp.InvalidGoFiles p.IgnoredOtherFiles = pp.IgnoredOtherFiles p.CFiles = pp.CFiles p.CXXFiles = pp.CXXFiles @@ -2493,7 +2496,7 @@ func PackagesAndErrors(ctx context.Context, opts PackageOpts, patterns []string) } if opts.MainOnly { - pkgs = mainPackagesOnly(pkgs, patterns) + pkgs = mainPackagesOnly(pkgs, matches) } // Now that CmdlinePkg is set correctly, @@ -2547,50 +2550,63 @@ func CheckPackageErrors(pkgs []*Package) { // mainPackagesOnly filters out non-main packages matched only by arguments // containing "..." and returns the remaining main packages. // +// Packages with missing, invalid, or ambiguous names may be treated as +// possibly-main packages. +// // mainPackagesOnly sets a non-main package's Error field and returns it if it // is named by a literal argument. // // mainPackagesOnly prints warnings for non-literal arguments that only match // non-main packages. -func mainPackagesOnly(pkgs []*Package, patterns []string) []*Package { - matchers := make([]func(string) bool, len(patterns)) - for i, p := range patterns { - if strings.Contains(p, "...") { - matchers[i] = search.MatchPattern(p) +func mainPackagesOnly(pkgs []*Package, matches []*search.Match) []*Package { + treatAsMain := map[string]bool{} + for _, m := range matches { + if m.IsLiteral() { + for _, path := range m.Pkgs { + treatAsMain[path] = true + } } } - matchedPkgs := make([]*Package, 0, len(pkgs)) - mainCount := make([]int, len(patterns)) - nonMainCount := make([]int, len(patterns)) + var mains []*Package for _, pkg := range pkgs { - if pkg.Name == "main" || (pkg.Incomplete && pkg.Name == "") { - matchedPkgs = append(matchedPkgs, pkg) - for i := range patterns { - if matchers[i] != nil && matchers[i](pkg.ImportPath) { - mainCount[i]++ - } - } - } else { - for i := range patterns { - if matchers[i] == nil && patterns[i] == pkg.ImportPath { - if pkg.Error == nil { - pkg.Error = &PackageError{Err: &mainPackageError{importPath: pkg.ImportPath}} - } - matchedPkgs = append(matchedPkgs, pkg) - } else if matchers[i] != nil && matchers[i](pkg.ImportPath) { - nonMainCount[i]++ - } - } + if pkg.Name == "main" { + treatAsMain[pkg.ImportPath] = true + mains = append(mains, pkg) + continue } - } - for i, p := range patterns { - if matchers[i] != nil && mainCount[i] == 0 && nonMainCount[i] > 0 { - fmt.Fprintf(os.Stderr, "go: warning: %q matched no main packages\n", p) + + if len(pkg.InvalidGoFiles) > 0 { // TODO(#45999): && pkg.Name == "", but currently go/build sets pkg.Name arbitrarily if it is ambiguous. + // The package has (or may have) conflicting names, and we can't easily + // tell whether one of them is "main". So assume that it could be, and + // report an error for the package. + treatAsMain[pkg.ImportPath] = true + } + if treatAsMain[pkg.ImportPath] { + if pkg.Error == nil { + pkg.Error = &PackageError{Err: &mainPackageError{importPath: pkg.ImportPath}} + } + mains = append(mains, pkg) } } - return matchedPkgs + for _, m := range matches { + if m.IsLiteral() || len(m.Pkgs) == 0 { + continue + } + foundMain := false + for _, path := range m.Pkgs { + if treatAsMain[path] { + foundMain = true + break + } + } + if !foundMain { + fmt.Fprintf(os.Stderr, "go: warning: %q matched only non-main packages\n", m.Pattern()) + } + } + + return mains } type mainPackageError struct { diff --git a/src/cmd/go/testdata/script/mod_install_pkg_version.txt b/src/cmd/go/testdata/script/mod_install_pkg_version.txt index 2c14ef737b3..fd02392af1b 100644 --- a/src/cmd/go/testdata/script/mod_install_pkg_version.txt +++ b/src/cmd/go/testdata/script/mod_install_pkg_version.txt @@ -143,7 +143,7 @@ stderr '^go: warning: "example.com/cmd/nomatch\.\.\." matched no packages$' # If a wildcard matches only non-main packges, we should see a different warning. go install example.com/cmd/err...@v1.0.0 -stderr '^go: warning: "example.com/cmd/err\.\.\." matched no main packages$' +stderr '^go: warning: "example.com/cmd/err\.\.\." matched only non-main packages$' # 'go install pkg@version' should report errors if the module contains diff --git a/src/cmd/go/testdata/script/mod_run_nonmain.txt b/src/cmd/go/testdata/script/mod_run_nonmain.txt new file mode 100644 index 00000000000..036755d2d1a --- /dev/null +++ b/src/cmd/go/testdata/script/mod_run_nonmain.txt @@ -0,0 +1,18 @@ +! go run $PWD +! stderr 'no packages loaded' +stderr '^package example.net/nonmain is not a main package$' + +! go run . +stderr '^package example.net/nonmain is not a main package$' + +! go run ./... +stderr '^go: warning: "\./\.\.\." matched only non-main packages$' +stderr '^go run: no packages loaded from \./\.\.\.$' + +-- go.mod -- +module example.net/nonmain + +go 1.17 +-- nonmain.go -- +// Package nonmain is not a main package. +package nonmain diff --git a/src/cmd/go/testdata/script/mod_run_pkgerror.txt b/src/cmd/go/testdata/script/mod_run_pkgerror.txt index fd7060aaa86..48f900dd346 100644 --- a/src/cmd/go/testdata/script/mod_run_pkgerror.txt +++ b/src/cmd/go/testdata/script/mod_run_pkgerror.txt @@ -1,12 +1,26 @@ +# https://golang.org/issue/39986: files reported as invalid by go/build should +# be listed in InvalidGoFiles. + +go list -e -f '{{.Incomplete}}{{"\n"}}{{.Error}}{{"\n"}}{{.InvalidGoFiles}}{{"\n"}}' . +stdout '^true\nfound packages m \(m\.go\) and main \(main\.go\) in '$PWD'\n\[main.go\]\n' + + # https://golang.org/issue/45827: 'go run .' should report the same package -# errors as 'go build'. +# errors as 'go build' and 'go list'. ! go build stderr '^found packages m \(m\.go\) and main \(main\.go\) in '$PWD'$' +! go list . +stderr '^found packages m \(m\.go\) and main \(main\.go\) in '$PWD'$' + ! go run . - # TODO(#45827): This error should match the above. -stderr '^go run: no packages loaded from \.$' +! stderr 'no packages loaded' +stderr '^found packages m \(m\.go\) and main \(main\.go\) in '$PWD'$' + +! go run ./... +! stderr 'no packages loaded' +stderr '^found packages m \(m\.go\) and main \(main\.go\) in '$PWD'$' -- go.mod -- module m