go/packages: do not nullify Fset when NeedSyntax is set
Currently, Fset is initialized when either NeedTypes or NeedSyntax are set in newLoader().
However, later in refine() it is nullified using a different condition that doesn't take NeedSyntax into account.
Use the inverse condition to that of in newLoader() when deciding on whether to nullify Fset or not.
Resolves https://github.com/golang/go/issues/48226.
Change-Id: Ic7c05dfe3337d5cf14aa185350a8e766e224c898
GitHub-Last-Rev: a9068719f659ac11a5846d9b8c46850b2103aa77
GitHub-Pull-Request: golang/tools#506
Reviewed-on: https://go-review.googlesource.com/c/tools/+/601239
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/go/packages/packages.go b/go/packages/packages.go
index d4529c5..0b6bfaf 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -46,7 +46,6 @@
//
// Unfortunately there are a number of open bugs related to
// interactions among the LoadMode bits:
-// - https://github.com/golang/go/issues/48226
// - https://github.com/golang/go/issues/56633
// - https://github.com/golang/go/issues/56677
// - https://github.com/golang/go/issues/58726
@@ -76,7 +75,7 @@
// NeedTypes adds Types, Fset, and IllTyped.
NeedTypes
- // NeedSyntax adds Syntax.
+ // NeedSyntax adds Syntax and Fset.
NeedSyntax
// NeedTypesInfo adds TypesInfo.
@@ -961,12 +960,14 @@
}
if ld.requestedMode&NeedTypes == 0 {
ld.pkgs[i].Types = nil
- ld.pkgs[i].Fset = nil
ld.pkgs[i].IllTyped = false
}
if ld.requestedMode&NeedSyntax == 0 {
ld.pkgs[i].Syntax = nil
}
+ if ld.requestedMode&NeedTypes == 0 && ld.requestedMode&NeedSyntax == 0 {
+ ld.pkgs[i].Fset = nil
+ }
if ld.requestedMode&NeedTypesInfo == 0 {
ld.pkgs[i].TypesInfo = nil
}
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 2a2e5a0..26dbc13 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -2645,6 +2645,42 @@
}
}
+// TestIssue48226 ensures that when NeedSyntax is provided we do not nullify the
+// Fset, which is needed to resolve the syntax tree element positions to files.
+func TestIssue48226(t *testing.T) { testAllOrModulesParallel(t, testIssue48226) }
+func testIssue48226(t *testing.T, exporter packagestest.Exporter) {
+ exported := packagestest.Export(t, exporter, []packagestest.Module{
+ {
+ Name: "golang.org/fake/syntax",
+ Files: map[string]interface{}{
+ "syntax.go": `package test`,
+ },
+ },
+ })
+ defer exported.Cleanup()
+
+ exported.Config.Mode = packages.NeedFiles | packages.NeedSyntax
+
+ initial, err := packages.Load(exported.Config, "golang.org/fake/syntax")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(initial) != 1 {
+ t.Fatalf("exepected 1 package, got %d", len(initial))
+ }
+ pkg := initial[0]
+
+ if len(pkg.Errors) != 0 {
+ t.Fatalf("package has errors: %v", pkg.Errors)
+ }
+
+ fname := pkg.Fset.File(pkg.Syntax[0].Pos()).Name()
+ if filepath.Base(fname) != "syntax.go" {
+ t.Errorf("expected the package declaration position "+
+ "to resolve to \"syntax.go\", got %q instead", fname)
+ }
+}
+
func TestModule(t *testing.T) {
testAllOrModulesParallel(t, testModule)
}
