go/packages: include "unsafe".GoFiles=["unsafe.go"]
This change causes the unsafe package's GoFiles list
to include unsafe.go, and documents the field more precisely.
This file is never compiled, but it is legal Go syntax,
and serves as documentation. It is useful for client
tools to know where to find it.
Also, remove the corresponding workaround in gopls.
Fixes golang/go#59929
Change-Id: I4ef9f4c16c5b5b74ee7a7c4d1f7eb3736f779b91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/491375
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/go/packages/golist.go b/go/packages/golist.go
index aa2ca3b..e84f19d 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -625,7 +625,12 @@
}
if pkg.PkgPath == "unsafe" {
- pkg.GoFiles = nil // ignore fake unsafe.go file
+ pkg.CompiledGoFiles = nil // ignore fake unsafe.go file (#59929)
+ } else if len(pkg.CompiledGoFiles) == 0 {
+ // Work around for pre-go.1.11 versions of go list.
+ // TODO(matloob): they should be handled by the fallback.
+ // Can we delete this?
+ pkg.CompiledGoFiles = pkg.GoFiles
}
// Assume go list emits only absolute paths for Dir.
@@ -663,13 +668,6 @@
response.Roots = append(response.Roots, pkg.ID)
}
- // Work around for pre-go.1.11 versions of go list.
- // TODO(matloob): they should be handled by the fallback.
- // Can we delete this?
- if len(pkg.CompiledGoFiles) == 0 {
- pkg.CompiledGoFiles = pkg.GoFiles
- }
-
// Temporary work-around for golang/go#39986. Parse filenames out of
// error messages. This happens if there are unrecoverable syntax
// errors in the source, so we can't match on a specific error message.
diff --git a/go/packages/packages.go b/go/packages/packages.go
index 0f1505b..632be72 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -308,6 +308,9 @@
TypeErrors []types.Error
// GoFiles lists the absolute file paths of the package's Go source files.
+ // It may include files that should not be compiled, for example because
+ // they contain non-matching build tags, are documentary pseudo-files such as
+ // unsafe/unsafe.go or builtin/builtin.go, or are subject to cgo preprocessing.
GoFiles []string
// CompiledGoFiles lists the absolute file paths of the package's source
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index d0960c8..a89887f 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -217,7 +217,7 @@
id string
wantName string
wantKind string
- wantSrcs string
+ wantSrcs string // = {Go,Other,Embed}Files
wantIgnored string
}{
{"golang.org/fake/a", "a", "package", "a.go", ""},
@@ -227,7 +227,7 @@
{"container/list", "list", "package", "list.go", ""},
{"golang.org/fake/subdir/d", "d", "package", "d.go", ""},
{"golang.org/fake/subdir/d.test", "main", "command", "0.go", ""},
- {"unsafe", "unsafe", "package", "", ""},
+ {"unsafe", "unsafe", "package", "unsafe.go", ""},
} {
p, ok := all[test.id]
if !ok {
@@ -250,10 +250,10 @@
}
if srcs := strings.Join(srcs(p), " "); srcs != test.wantSrcs {
- t.Errorf("%s.Srcs = [%s], want [%s]", test.id, srcs, test.wantSrcs)
+ t.Errorf("%s.{Go,Other,Embed}Files = [%s], want [%s]", test.id, srcs, test.wantSrcs)
}
if ignored := strings.Join(cleanPaths(p.IgnoredFiles), " "); ignored != test.wantIgnored {
- t.Errorf("%s.Srcs = [%s], want [%s]", test.id, ignored, test.wantIgnored)
+ t.Errorf("%s.IgnoredFiles = [%s], want [%s]", test.id, ignored, test.wantIgnored)
}
}
@@ -2788,7 +2788,11 @@
}
func srcs(p *packages.Package) []string {
- return cleanPaths(append(append(p.GoFiles[:len(p.GoFiles):len(p.GoFiles)], p.OtherFiles...), p.EmbedFiles...))
+ var files []string
+ files = append(files, p.GoFiles...)
+ files = append(files, p.OtherFiles...)
+ files = append(files, p.EmbedFiles...)
+ return cleanPaths(files)
}
// cleanPaths attempts to reduce path names to stable forms
diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go
index 939d084..da985a8 100644
--- a/gopls/internal/lsp/cache/load.go
+++ b/gopls/internal/lsp/cache/load.go
@@ -159,32 +159,6 @@
return bug.Errorf("internal error: go/packages returned multiple packages for standalone file")
}
- // Workaround for a bug (?) that has been in go/packages since
- // the outset: Package("unsafe").GoFiles=[], whereas it should
- // include unsafe/unsafe.go. Derive it from builtins.go.
- //
- // This workaround relies on the fact that we always add both
- // builtins and unsafe to the set of scopes in the workspace load.
- //
- // TODO(adonovan): fix upstream in go/packages.
- // (Does this need a proposal? Arguably not.)
- {
- var builtin, unsafe *packages.Package
- for _, pkg := range pkgs {
- switch pkg.ID {
- case "unsafe":
- unsafe = pkg
- case "builtin":
- builtin = pkg
- }
- }
- if builtin != nil && unsafe != nil && len(builtin.GoFiles) == 1 {
- unsafe.GoFiles = []string{
- filepath.Join(filepath.Dir(builtin.GoFiles[0]), "../unsafe/unsafe.go"),
- }
- }
- }
-
moduleErrs := make(map[string][]packages.Error) // module path -> errors
filterFunc := s.view.filterFunc()
newMetadata := make(map[PackageID]*source.Metadata)
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index 1dc13aa..74a07cf 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -862,10 +862,8 @@
// If we're loading anything, ensure we also load builtin,
// since it provides fake definitions (and documentation)
// for types like int that are used everywhere.
- // ("unsafe" is also needed since its sole GoFiles is
- // derived from that of "builtin" via a workaround in load.)
if len(scopes) > 0 {
- scopes = append(scopes, packageLoadScope("builtin"), packageLoadScope("unsafe"))
+ scopes = append(scopes, packageLoadScope("builtin"))
}
loadErr = s.load(ctx, true, scopes...)
diff --git a/gopls/internal/regtest/misc/references_test.go b/gopls/internal/regtest/misc/references_test.go
index 1e14f1b..a85bcc2 100644
--- a/gopls/internal/regtest/misc/references_test.go
+++ b/gopls/internal/regtest/misc/references_test.go
@@ -104,7 +104,7 @@
func TestDefsRefsBuiltins(t *testing.T) {
testenv.NeedsGo1Point(t, 17) // for unsafe.{Add,Slice}
- // TODO(adonovan): add unsafe.SliceData,String,StringData} in later go versions.
+ // TODO(adonovan): add unsafe.{SliceData,String,StringData} in later go versions.
const files = `
-- go.mod --
module example.com
diff --git a/gopls/internal/regtest/misc/workspace_symbol_test.go b/gopls/internal/regtest/misc/workspace_symbol_test.go
index 849743b..7b2866e 100644
--- a/gopls/internal/regtest/misc/workspace_symbol_test.go
+++ b/gopls/internal/regtest/misc/workspace_symbol_test.go
@@ -32,8 +32,6 @@
const K2 = "exclude.go"
`
- // NB: the name K was chosen to avoid spurious
- // matches in the always-present "unsafe" package.
Run(t, files, func(t *testing.T, env *Env) {
env.OpenFile("a.go")
checkSymbols(env, "K", "K1")
@@ -73,7 +71,6 @@
"Fooex", // shorter than Fooest, FooBar, lexically before Fooey
"Fooey", // shorter than Fooest, Foobar
"Fooest",
- "unsafe.Offsetof", // a very fuzzy match
)
})
}
