1
0
mirror of https://github.com/golang/go synced 2024-11-18 16:14:46 -07:00

go/packages: make sure to request dependencies when we NeedTypesInfo

If NeedTypesInfo, go/packages needs to do typechecking itself so it can
associate type info with the AST. To do so, we need the export data
for dependencies, which means we need to ask for the direct dependencies.
go list has no way to ask for direct dependencies, so we'll set -deps
in the command line to go list. (We need to type check the transitive
dependencies to get export data anyways so this shouldn't be that
much worse than a hypothetical mode that only did work for direct
dependencies. The main issue will be the I/O required (which could (?)
still be a problem?).)

On top of that, we need to make sure that we *don't* ask for sources for
those direct dependencies (which we might have done before) because
that would result in trying to type check the dependency packages
from source, which we'd need *those* packages' dependencies' export
data and transitive sources for and so on.

uggggggggggggggh

Fixes golang/go#32814

Change-Id: I8d95b320beea556bdf44c625607d9e7d2cc188e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184165
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Michael Matloob 2019-06-27 16:41:53 -04:00
parent 2161848f5a
commit 56125e7d70
4 changed files with 45 additions and 10 deletions

View File

@ -831,7 +831,8 @@ func golistargs(cfg *Config, words []string) []string {
fmt.Sprintf("-compiled=%t", cfg.Mode&(NeedCompiledGoFiles|NeedSyntax|NeedTypesInfo|NeedTypesSizes) != 0),
fmt.Sprintf("-test=%t", cfg.Tests),
fmt.Sprintf("-export=%t", usesExportData(cfg)),
fmt.Sprintf("-deps=%t", cfg.Mode&NeedDeps != 0),
fmt.Sprintf("-deps=%t", cfg.Mode&NeedDeps != 0 ||
cfg.Mode&NeedTypesInfo != 0), // Dependencies are required to do typechecking on sources, which is required for the TypesInfo.
// go list doesn't let you pass -test and -find together,
// probably because you'd just get the TestMain.
fmt.Sprintf("-find=%t", !cfg.Tests && cfg.Mode&findFlags == 0),

View File

@ -497,7 +497,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
lpkg := &loaderPackage{
Package: pkg,
needtypes: (ld.Mode&(NeedTypes|NeedTypesInfo) != 0 && rootIndex < 0) || rootIndex >= 0,
needsrc: (ld.Mode&(NeedSyntax|NeedTypesInfo) != 0 && rootIndex < 0) || rootIndex >= 0 ||
needsrc: (ld.Mode&(NeedSyntax|NeedTypesInfo) != 0 && ld.Mode&NeedDeps != 0 && rootIndex < 0) || rootIndex >= 0 ||
len(ld.Overlay) > 0 || // Overlays can invalidate export data. TODO(matloob): make this check fine-grained based on dependencies on overlaid files
pkg.ExportFile == "" && pkg.PkgPath != "unsafe",
}
@ -544,9 +544,10 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
lpkg.color = grey
stack = append(stack, lpkg) // push
stubs := lpkg.Imports // the structure form has only stubs with the ID in the Imports
// If NeedTypesInfo we need dependencies (at least for the roots) to typecheck the package.
// If NeedImports isn't set, the imports fields will all be zeroed out.
// If NeedDeps isn't also set we want to keep the stubs.
if ld.Mode&NeedImports != 0 && ld.Mode&NeedDeps != 0 {
if ld.Mode&NeedTypesInfo != 0 || (ld.Mode&NeedImports != 0 && ld.Mode&NeedDeps != 0) {
lpkg.Imports = make(map[string]*Package, len(stubs))
for importPath, ipkg := range stubs {
var importErr error
@ -565,8 +566,11 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
continue
}
if visit(imp) {
lpkg.needsrc = true
// If !NeedDeps, just fill Imports for the root. No need to recurse further.
if ld.Mode&NeedDeps != 0 {
if visit(imp) {
lpkg.needsrc = true
}
}
lpkg.Imports[importPath] = imp.Package
}
@ -583,7 +587,7 @@ func (ld *loader) refine(roots []string, list ...*Package) ([]*Package, error) {
return lpkg.needsrc
}
if ld.Mode&(NeedImports|NeedDeps) == 0 {
if ld.Mode&(NeedImports|NeedDeps|NeedTypesInfo) == 0 {
// We do this to drop the stub import packages that we are not even going to try to resolve.
for _, lpkg := range initial {
lpkg.Imports = nil
@ -1088,5 +1092,9 @@ func (ld *loader) loadFromExportData(lpkg *loaderPackage) (*types.Package, error
}
func usesExportData(cfg *Config) bool {
return cfg.Mode&NeedExportsFile != 0 || cfg.Mode&NeedTypes != 0 && cfg.Mode&NeedTypesInfo == 0
return cfg.Mode&NeedExportsFile != 0 ||
// If NeedTypes but not NeedTypesInfo we won't typecheck using sources, so we need export data.
(cfg.Mode&NeedTypes != 0 && cfg.Mode&NeedTypesInfo == 0) ||
// If NeedTypesInfo but not NeedDeps, we're typechecking a package using its sources plus its dependencies' export data
(cfg.Mode&NeedTypesInfo != 0 && cfg.Mode&NeedDeps == 0)
}

View File

@ -2119,8 +2119,8 @@ func testAdHocContains(t *testing.T, exporter packagestest.Exporter) {
}
}
func TestNoCcompiler(t *testing.T) { packagestest.TestAll(t, testNoCcompiler) }
func testNoCcompiler(t *testing.T, exporter packagestest.Exporter) {
func TestCgoNoCcompiler(t *testing.T) { packagestest.TestAll(t, testCgoNoCcompiler) }
func testCgoNoCcompiler(t *testing.T, exporter packagestest.Exporter) {
exported := packagestest.Export(t, exporter, []packagestest.Module{{
Name: "golang.org/fake",
Files: map[string]interface{}{
@ -2135,6 +2135,7 @@ const A = http.MethodGet
exported.Config.Env = append(exported.Config.Env, "CGO_ENABLED=1", "CC=doesnotexist")
exported.Config.Mode = packages.LoadAllSyntax
initial, err := packages.Load(exported.Config, "golang.org/fake/a")
if err != nil {
t.Fatal(err)
}
@ -2149,7 +2150,32 @@ const A = http.MethodGet
if got != "\"GET\"" {
t.Errorf("a.A: got %s, want %s", got, "\"GET\"")
}
}
func TestIssue32814(t *testing.T) { packagestest.TestAll(t, testIssue32814) }
func testIssue32814(t *testing.T, exporter packagestest.Exporter) {
exported := packagestest.Export(t, exporter, []packagestest.Module{{
Name: "golang.org/fake",
Files: map[string]interface{}{}}})
defer exported.Cleanup()
exported.Config.Mode = packages.NeedName | packages.NeedTypes | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedTypesSizes
pkgs, err := packages.Load(exported.Config, "fmt")
if err != nil {
t.Fatal(err)
}
if len(pkgs) != 1 && pkgs[0].PkgPath != "fmt" {
t.Fatalf("packages.Load: want [fmt], got %v", pkgs)
}
pkg := pkgs[0]
if len(pkg.Errors) != 0 {
t.Fatalf("Errors for fmt pkg: got %v, want none", pkg.Errors)
}
if !pkg.Types.Complete() {
t.Fatalf("Types.Complete() for fmt pkg: got %v, want true", pkgs[0].Types.Complete())
}
}
func errorMessages(errors []packages.Error) []string {

View File

@ -141,7 +141,7 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
exported := &Exported{
Config: &packages.Config{
Dir: temp,
Env: append(os.Environ(), "GOPACKAGESDRIVER=off"),
Env: append(os.Environ(), "GOPACKAGESDRIVER=off", "GOROOT="), // Clear GOROOT to work around #32849.
Overlay: make(map[string][]byte),
Tests: true,
Mode: packages.LoadImports,