[tools] go/packages: fix load with NeedTypes but not NeedImports

[Gopher Robot (Gerrit)]

Gopher Robot submitted this change.

View Change

1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: go/packages/packages_test.go
Insertions: 1, Deletions: 0.

The diff is too large to show. Please review the diff.
```

Approvals: Austin Clements: Looks good to me, approved Russ Cox: Trusted; Run TryBots; Automatically submit change Gopher Robot: TryBots succeeded kokoro: gopls CI succeeded

go/packages: fix load with NeedTypes but not NeedImports

Fixes golang/go#45584.

Change-Id: I65238cc3bdc640bb044c615a5699e8d3cfa39db0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310512
Trust: Russ Cox <r...@golang.org>
Run-TryBot: Russ Cox <r...@golang.org>
Reviewed-by: Austin Clements <aus...@google.com>
Auto-Submit: Russ Cox <r...@golang.org>
gopls-CI: kokoro <noreply...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M go/packages/packages.go
M go/packages/packages_test.go
2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/go/packages/packages.go b/go/packages/packages.go
index f8a7018..2442845 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -1237,7 +1237,7 @@
 		return nil, fmt.Errorf("reading %s: %v", lpkg.ExportFile, err)
 	}
 	if viewLen != len(view) {
-		log.Fatalf("Unexpected package creation during export data loading")
+		log.Panicf("golang.org/x/tools/go/packages: unexpected new packages during load of %s", lpkg.PkgPath)
 	}
 
 	lpkg.Types = tpkg
@@ -1248,17 +1248,8 @@
 
 // impliedLoadMode returns loadMode with its dependencies.
 func impliedLoadMode(loadMode LoadMode) LoadMode {
-	if loadMode&NeedTypesInfo != 0 && loadMode&NeedImports == 0 {
-		// 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.
-		// NeedImports is used to ask for the direct dependencies.
-		loadMode |= NeedImports
-	}
-
-	if loadMode&NeedDeps != 0 && loadMode&NeedImports == 0 {
-		// With NeedDeps we need to load at least direct dependencies.
-		// NeedImports is used to ask for the direct dependencies.
+	if loadMode&(NeedDeps|NeedTypes|NeedTypesInfo) != 0 {
+		// All these things require knowing the import graph.
 		loadMode |= NeedImports
 	}
 
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index ef0a328..796edb6 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -2843,3 +2843,11 @@
 		return nil
 	})
 }
+
+func TestExportFile(t *testing.T) {
+	// This used to trigger the log.Fatal in loadFromExportData.
+	// See go.dev/issue/45584.
+	cfg := new(packages.Config)
+	cfg.Mode = packages.NeedTypes
+	packages.Load(cfg, "fmt")
+}

To view, visit change 310512. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: tools

Gerrit-Branch: master

Gerrit-Change-Id: I65238cc3bdc640bb044c615a5699e8d3cfa39db0

Gerrit-Change-Number: 310512

Gerrit-PatchSet: 4

Gerrit-Owner: Russ Cox <r...@golang.org>

Gerrit-Reviewer: Austin Clements <aus...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-Reviewer: kokoro <noreply...@google.com>

Gerrit-MessageType: merged