Skip to content

Commit 3a915e4

Browse files
committed
gopls/internal/lsp/cache: use local aliases for all source objects
As a step toward inverting the import between cache and source, create additional local aliases in the cache package for all referenced objects in the source package. Subsequent CLs will clean these up, and reverse the import. Change-Id: I914e0b8d54aa15d3d5e9ee20fae2e64bc1e48553 Reviewed-on: https://go-review.googlesource.com/c/tools/+/543717 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent be332ea commit 3a915e4

File tree

18 files changed

+340
-299
lines changed

18 files changed

+340
-299
lines changed

gopls/internal/lsp/cache/analysis.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"golang.org/x/tools/gopls/internal/lsp/frob"
3838
"golang.org/x/tools/gopls/internal/lsp/progress"
3939
"golang.org/x/tools/gopls/internal/lsp/protocol"
40-
"golang.org/x/tools/gopls/internal/lsp/source"
4140
"golang.org/x/tools/gopls/internal/settings"
4241
"golang.org/x/tools/internal/event"
4342
"golang.org/x/tools/internal/event/tag"
@@ -155,7 +154,7 @@ import (
155154
// to the driver package.
156155
// Steps:
157156
// - define a narrow driver.Snapshot interface with only these methods:
158-
// Metadata(PackageID) source.Metadata
157+
// Metadata(PackageID) Metadata
159158
// ReadFile(Context, URI) (file.Handle, error)
160159
// View() *View // for Options
161160
// - share cache.{goVersionRx,parseGoImpl}
@@ -171,7 +170,7 @@ const AnalysisProgressTitle = "Analyzing Dependencies"
171170
// The analyzers list must be duplicate free; order does not matter.
172171
//
173172
// Notifications of progress may be sent to the optional reporter.
174-
func (snapshot *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]unit, analyzers []*settings.Analyzer, reporter *progress.Tracker) ([]*source.Diagnostic, error) {
173+
func (snapshot *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]unit, analyzers []*settings.Analyzer, reporter *progress.Tracker) ([]*Diagnostic, error) {
175174
start := time.Now() // for progress reporting
176175

177176
var tagStr string // sorted comma-separated list of PackageIDs
@@ -436,7 +435,7 @@ func (snapshot *Snapshot) Analyze(ctx context.Context, pkgs map[PackageID]unit,
436435
// begin the analysis you asked for".
437436
// Even if current callers choose to discard the
438437
// results, we should propagate the per-action errors.
439-
var results []*source.Diagnostic
438+
var results []*Diagnostic
440439
for _, root := range roots {
441440
for _, a := range enabled {
442441
// Skip analyzers that were added only to
@@ -504,7 +503,7 @@ func (an *analysisNode) decrefPreds() {
504503
// type-checking and analyzing syntax (miss).
505504
type analysisNode struct {
506505
fset *token.FileSet // file set shared by entire batch (DAG)
507-
m *source.Metadata // metadata for this package
506+
m *Metadata // metadata for this package
508507
files []file.Handle // contents of CompiledGoFiles
509508
analyzers []*analysis.Analyzer // set of analyzers to run
510509
preds []*analysisNode // graph edges:
@@ -784,7 +783,7 @@ func (an *analysisNode) cacheKey() [sha256.Size]byte {
784783
func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
785784
// Parse only the "compiled" Go files.
786785
// Do the computation in parallel.
787-
parsed := make([]*source.ParsedGoFile, len(an.files))
786+
parsed := make([]*ParsedGoFile, len(an.files))
788787
{
789788
var group errgroup.Group
790789
group.SetLimit(4) // not too much: run itself is already called in parallel
@@ -795,7 +794,7 @@ func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
795794
// as cached ASTs require the global FileSet.
796795
// ast.Object resolution is unfortunately an implied part of the
797796
// go/analysis contract.
798-
pgf, err := parseGoImpl(ctx, an.fset, fh, source.ParseFull&^source.SkipObjectResolution, false)
797+
pgf, err := parseGoImpl(ctx, an.fset, fh, ParseFull&^SkipObjectResolution, false)
799798
parsed[i] = pgf
800799
return err
801800
})
@@ -909,7 +908,7 @@ func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
909908
}
910909

911910
// Postcondition: analysisPackage.types and an.exportDeps are populated.
912-
func (an *analysisNode) typeCheck(parsed []*source.ParsedGoFile) *analysisPackage {
911+
func (an *analysisNode) typeCheck(parsed []*ParsedGoFile) *analysisPackage {
913912
m := an.m
914913

915914
if false { // debugging
@@ -964,7 +963,7 @@ func (an *analysisNode) typeCheck(parsed []*source.ParsedGoFile) *analysisPackag
964963
// as parser recovery can be quite lossy (#59888).
965964
typeError := e.(types.Error)
966965
for _, p := range parsed {
967-
if p.ParseErr != nil && source.NodeContains(p.File, typeError.Pos) {
966+
if p.ParseErr != nil && NodeContains(p.File, typeError.Pos) {
968967
return
969968
}
970969
}
@@ -1000,7 +999,7 @@ func (an *analysisNode) typeCheck(parsed []*source.ParsedGoFile) *analysisPackag
1000999
}
10011000

10021001
// (Duplicates logic from check.go.)
1003-
if !source.IsValidImport(an.m.PkgPath, dep.m.PkgPath) {
1002+
if !IsValidImport(an.m.PkgPath, dep.m.PkgPath) {
10041003
return nil, fmt.Errorf("invalid use of internal package %s", importPath)
10051004
}
10061005

@@ -1098,9 +1097,9 @@ func readShallowManifest(export []byte) ([]PackagePath, error) {
10981097
// analysisPackage contains information about a package, including
10991098
// syntax trees, used transiently during its type-checking and analysis.
11001099
type analysisPackage struct {
1101-
m *source.Metadata
1100+
m *Metadata
11021101
fset *token.FileSet // local to this package
1103-
parsed []*source.ParsedGoFile
1102+
parsed []*ParsedGoFile
11041103
files []*ast.File // same as parsed[i].File
11051104
types *types.Package
11061105
compiles bool // package is transitively free of list/parse/type errors

gopls/internal/lsp/cache/check.go

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"golang.org/x/tools/gopls/internal/immutable"
2828
"golang.org/x/tools/gopls/internal/lsp/filecache"
2929
"golang.org/x/tools/gopls/internal/lsp/protocol"
30-
"golang.org/x/tools/gopls/internal/lsp/source"
3130
"golang.org/x/tools/gopls/internal/lsp/source/typerefs"
3231
"golang.org/x/tools/internal/event"
3332
"golang.org/x/tools/internal/event/tag"
@@ -93,8 +92,8 @@ type pkgOrErr struct {
9392
// This is different from having type-checking errors: a failure to type-check
9493
// indicates context cancellation or otherwise significant failure to perform
9594
// the type-checking operation.
96-
func (s *Snapshot) TypeCheck(ctx context.Context, ids ...PackageID) ([]source.Package, error) {
97-
pkgs := make([]source.Package, len(ids))
95+
func (s *Snapshot) TypeCheck(ctx context.Context, ids ...PackageID) ([]Package_, error) {
96+
pkgs := make([]Package_, len(ids))
9897

9998
var (
10099
needIDs []PackageID // ids to type-check
@@ -200,13 +199,13 @@ func (s *Snapshot) resolveImportGraph() (*importGraph, error) {
200199
if err != nil {
201200
return nil, err
202201
}
203-
source.RemoveIntermediateTestVariants(&meta)
202+
RemoveIntermediateTestVariants(&meta)
204203
for _, m := range meta {
205204
openPackages[m.ID] = true
206205
}
207206
}
208207

209-
var openPackageIDs []source.PackageID
208+
var openPackageIDs []PackageID
210209
for id := range openPackages {
211210
openPackageIDs = append(openPackageIDs, id)
212211
}
@@ -543,7 +542,7 @@ func (b *typeCheckBatch) handleSyntaxPackage(ctx context.Context, i int, id Pack
543542

544543
// importPackage loads the given package from its export data in p.exportData
545544
// (which must already be populated).
546-
func (b *typeCheckBatch) importPackage(ctx context.Context, m *source.Metadata, data []byte) (*types.Package, error) {
545+
func (b *typeCheckBatch) importPackage(ctx context.Context, m *Metadata, data []byte) (*types.Package, error) {
547546
ctx, done := event.Start(ctx, "cache.typeCheckBatch.importPackage", tag.Package.Of(string(m.ID)))
548547
defer done()
549548

@@ -606,7 +605,7 @@ func (b *typeCheckBatch) checkPackageForImport(ctx context.Context, ph *packageH
606605
// Parse the compiled go files, bypassing the parse cache as packages checked
607606
// for import are unlikely to get cache hits. Additionally, we can optimize
608607
// parsing slightly by not passing parser.ParseComments.
609-
pgfs := make([]*source.ParsedGoFile, len(ph.localInputs.compiledGoFiles))
608+
pgfs := make([]*ParsedGoFile, len(ph.localInputs.compiledGoFiles))
610609
{
611610
var group errgroup.Group
612611
// Set an arbitrary concurrency limit; we want some parallelism but don't
@@ -713,7 +712,7 @@ func (b *typeCheckBatch) checkPackage(ctx context.Context, ph *packageHandle) (*
713712
// unrecoverable error loading export data.
714713
//
715714
// TODO(rfindley): inline, now that this is only called in one place.
716-
func (b *typeCheckBatch) awaitPredecessors(ctx context.Context, m *source.Metadata) error {
715+
func (b *typeCheckBatch) awaitPredecessors(ctx context.Context, m *Metadata) error {
717716
// await predecessors concurrently, as some of them may be non-syntax
718717
// packages, and therefore will not have been started by the type-checking
719718
// batch.
@@ -730,10 +729,10 @@ func (b *typeCheckBatch) awaitPredecessors(ctx context.Context, m *source.Metada
730729

731730
// importMap returns the map of package path -> package ID relative to the
732731
// specified ID.
733-
func (b *typeCheckBatch) importMap(id PackageID) map[string]source.PackageID {
734-
impMap := make(map[string]source.PackageID)
735-
var populateDeps func(m *source.Metadata)
736-
populateDeps = func(parent *source.Metadata) {
732+
func (b *typeCheckBatch) importMap(id PackageID) map[string]PackageID {
733+
impMap := make(map[string]PackageID)
734+
var populateDeps func(m *Metadata)
735+
populateDeps = func(parent *Metadata) {
737736
for _, id := range parent.DepsByPkgPath {
738737
m := b.handles[id].m
739738
if _, ok := impMap[string(m.PkgPath)]; ok {
@@ -779,7 +778,7 @@ func (b *typeCheckBatch) importMap(id PackageID) map[string]source.PackageID {
779778
// changed (as detected by the depkeys field), then the packageHandle in
780779
// question must also not have changed, and we need not re-evaluate its key.
781780
type packageHandle struct {
782-
m *source.Metadata
781+
m *Metadata
783782

784783
// Local data:
785784

@@ -853,7 +852,7 @@ func (s *Snapshot) getPackageHandles(ctx context.Context, ids []PackageID) (map[
853852
if n.unfinishedSuccs == 0 {
854853
leaves = append(leaves, n)
855854
} else {
856-
n.succs = make(map[source.PackageID]*handleNode, n.unfinishedSuccs)
855+
n.succs = make(map[PackageID]*handleNode, n.unfinishedSuccs)
857856
}
858857
b.nodes[idxID] = n
859858
for _, depID := range m.DepsByPkgPath {
@@ -938,7 +937,7 @@ type packageHandleBuilder struct {
938937
//
939938
// It is used to implement a bottom-up construction of packageHandles.
940939
type handleNode struct {
941-
m *source.Metadata
940+
m *Metadata
942941
idxID typerefs.IndexID
943942
ph *packageHandle
944943
err error
@@ -1210,8 +1209,8 @@ func (b *packageHandleBuilder) evaluatePackageHandle(prevPH *packageHandle, n *h
12101209

12111210
// typerefs returns typerefs for the package described by m and cgfs, after
12121211
// either computing it or loading it from the file cache.
1213-
func (s *Snapshot) typerefs(ctx context.Context, m *source.Metadata, cgfs []file.Handle) (map[string][]typerefs.Symbol, error) {
1214-
imports := make(map[ImportPath]*source.Metadata)
1212+
func (s *Snapshot) typerefs(ctx context.Context, m *Metadata, cgfs []file.Handle) (map[string][]typerefs.Symbol, error) {
1213+
imports := make(map[ImportPath]*Metadata)
12151214
for impPath, id := range m.DepsByImpPath {
12161215
if id != "" {
12171216
imports[impPath] = s.Metadata(id)
@@ -1234,15 +1233,15 @@ func (s *Snapshot) typerefs(ctx context.Context, m *source.Metadata, cgfs []file
12341233

12351234
// typerefData retrieves encoded typeref data from the filecache, or computes it on
12361235
// a cache miss.
1237-
func (s *Snapshot) typerefData(ctx context.Context, id PackageID, imports map[ImportPath]*source.Metadata, cgfs []file.Handle) ([]byte, error) {
1236+
func (s *Snapshot) typerefData(ctx context.Context, id PackageID, imports map[ImportPath]*Metadata, cgfs []file.Handle) ([]byte, error) {
12381237
key := typerefsKey(id, imports, cgfs)
12391238
if data, err := filecache.Get(typerefsKind, key); err == nil {
12401239
return data, nil
12411240
} else if err != filecache.ErrNotFound {
12421241
bug.Reportf("internal error reading typerefs data: %v", err)
12431242
}
12441243

1245-
pgfs, err := s.view.parseCache.parseFiles(ctx, token.NewFileSet(), source.ParseFull&^parser.ParseComments, true, cgfs...)
1244+
pgfs, err := s.view.parseCache.parseFiles(ctx, token.NewFileSet(), ParseFull&^parser.ParseComments, true, cgfs...)
12461245
if err != nil {
12471246
return nil, err
12481247
}
@@ -1260,7 +1259,7 @@ func (s *Snapshot) typerefData(ctx context.Context, id PackageID, imports map[Im
12601259

12611260
// typerefsKey produces a key for the reference information produced by the
12621261
// typerefs package.
1263-
func typerefsKey(id PackageID, imports map[ImportPath]*source.Metadata, compiledGoFiles []file.Handle) file.Hash {
1262+
func typerefsKey(id PackageID, imports map[ImportPath]*Metadata, compiledGoFiles []file.Handle) file.Hash {
12641263
hasher := sha256.New()
12651264

12661265
fmt.Fprintf(hasher, "typerefs: %s\n", id)
@@ -1309,7 +1308,7 @@ type typeCheckInputs struct {
13091308
moduleMode bool
13101309
}
13111310

1312-
func (s *Snapshot) typeCheckInputs(ctx context.Context, m *source.Metadata) (typeCheckInputs, error) {
1311+
func (s *Snapshot) typeCheckInputs(ctx context.Context, m *Metadata) (typeCheckInputs, error) {
13131312
// Read both lists of files of this package.
13141313
//
13151314
// Parallelism is not necessary here as the files will have already been
@@ -1426,7 +1425,7 @@ func typeCheckImpl(ctx context.Context, b *typeCheckBatch, inputs typeCheckInput
14261425

14271426
// Our heuristic for whether to show type checking errors is:
14281427
// + If any file was 'fixed', don't show type checking errors as we
1429-
// can't guarantee that they reference accurate locations in the source.
1428+
// can't guarantee that they reference accurate locations in thesource.
14301429
// + If there is a parse error _in the current file_, suppress type
14311430
// errors in that file.
14321431
// + Otherwise, show type errors even in the presence of parse errors in
@@ -1512,11 +1511,11 @@ func doTypeCheck(ctx context.Context, b *typeCheckBatch, inputs typeCheckInputs)
15121511
// Collect parsed files from the type check pass, capturing parse errors from
15131512
// compiled files.
15141513
var err error
1515-
pkg.goFiles, err = b.parseCache.parseFiles(ctx, b.fset, source.ParseFull, false, inputs.goFiles...)
1514+
pkg.goFiles, err = b.parseCache.parseFiles(ctx, b.fset, ParseFull, false, inputs.goFiles...)
15161515
if err != nil {
15171516
return nil, err
15181517
}
1519-
pkg.compiledGoFiles, err = b.parseCache.parseFiles(ctx, b.fset, source.ParseFull, false, inputs.compiledGoFiles...)
1518+
pkg.compiledGoFiles, err = b.parseCache.parseFiles(ctx, b.fset, ParseFull, false, inputs.compiledGoFiles...)
15201519
if err != nil {
15211520
return nil, err
15221521
}
@@ -1609,7 +1608,7 @@ func (b *typeCheckBatch) typesConfig(ctx context.Context, inputs typeCheckInputs
16091608
// e.g. missing metadata for dependencies in buildPackageHandle
16101609
return nil, missingPkgError(inputs.id, path, inputs.moduleMode)
16111610
}
1612-
if !source.IsValidImport(inputs.pkgPath, depPH.m.PkgPath) {
1611+
if !IsValidImport(inputs.pkgPath, depPH.m.PkgPath) {
16131612
return nil, fmt.Errorf("invalid use of internal package %q", path)
16141613
}
16151614
return b.getImportPackage(ctx, id)
@@ -1637,7 +1636,7 @@ func (b *typeCheckBatch) typesConfig(ctx context.Context, inputs typeCheckInputs
16371636
// of pkg, or to 'requires' declarations in the package's go.mod file.
16381637
//
16391638
// TODO(rfindley): move this to load.go
1640-
func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs file.Source, workspacePackages immutable.Map[PackageID, PackagePath]) ([]*source.Diagnostic, error) {
1639+
func depsErrors(ctx context.Context, m *Metadata, meta *metadataGraph, fs file.Source, workspacePackages immutable.Map[PackageID, PackagePath]) ([]*Diagnostic, error) {
16411640
// Select packages that can't be found, and were imported in non-workspace packages.
16421641
// Workspace packages already show their own errors.
16431642
var relevantErrors []*packagesinternal.PackageError
@@ -1668,12 +1667,12 @@ func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs
16681667

16691668
// Build an index of all imports in the package.
16701669
type fileImport struct {
1671-
cgf *source.ParsedGoFile
1670+
cgf *ParsedGoFile
16721671
imp *ast.ImportSpec
16731672
}
16741673
allImports := map[string][]fileImport{}
16751674
for _, uri := range m.CompiledGoFiles {
1676-
pgf, err := parseGoURI(ctx, fs, uri, source.ParseHeader)
1675+
pgf, err := parseGoURI(ctx, fs, uri, ParseHeader)
16771676
if err != nil {
16781677
return nil, err
16791678
}
@@ -1692,7 +1691,7 @@ func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs
16921691

16931692
// Apply a diagnostic to any import involved in the error, stopping once
16941693
// we reach the workspace.
1695-
var errors []*source.Diagnostic
1694+
var errors []*Diagnostic
16961695
for _, depErr := range relevantErrors {
16971696
for i := len(depErr.ImportStack) - 1; i >= 0; i-- {
16981697
item := depErr.ImportStack[i]
@@ -1709,15 +1708,15 @@ func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs
17091708
if err != nil {
17101709
return nil, err
17111710
}
1712-
diag := &source.Diagnostic{
1711+
diag := &Diagnostic{
17131712
URI: imp.cgf.URI,
17141713
Range: rng,
17151714
Severity: protocol.SeverityError,
1716-
Source: source.TypeError,
1715+
Source: TypeError,
17171716
Message: fmt.Sprintf("error while importing %v: %v", item, depErr.Err),
17181717
SuggestedFixes: fixes,
17191718
}
1720-
if !source.BundleQuickFixes(diag) {
1719+
if !BundleQuickFixes(diag) {
17211720
bug.Reportf("failed to bundle fixes for diagnostic %q", diag.Message)
17221721
}
17231722
errors = append(errors, diag)
@@ -1756,15 +1755,15 @@ func depsErrors(ctx context.Context, m *source.Metadata, meta *metadataGraph, fs
17561755
if err != nil {
17571756
return nil, err
17581757
}
1759-
diag := &source.Diagnostic{
1758+
diag := &Diagnostic{
17601759
URI: pm.URI,
17611760
Range: rng,
17621761
Severity: protocol.SeverityError,
1763-
Source: source.TypeError,
1762+
Source: TypeError,
17641763
Message: fmt.Sprintf("error while importing %v: %v", item, depErr.Err),
17651764
SuggestedFixes: fixes,
17661765
}
1767-
if !source.BundleQuickFixes(diag) {
1766+
if !BundleQuickFixes(diag) {
17681767
bug.Reportf("failed to bundle fixes for diagnostic %q", diag.Message)
17691768
}
17701769
errors = append(errors, diag)
@@ -1781,7 +1780,7 @@ func missingPkgError(from PackageID, pkgPath string, moduleMode bool) error {
17811780
// access to the full snapshot, and could provide more information (such as
17821781
// the initialization error).
17831782
if moduleMode {
1784-
if source.IsCommandLineArguments(from) {
1783+
if IsCommandLineArguments(from) {
17851784
return fmt.Errorf("current file is not included in a workspace module")
17861785
} else {
17871786
// Previously, we would present the initialization error here.

gopls/internal/lsp/cache/cycle_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ import (
88
"sort"
99
"strings"
1010
"testing"
11-
12-
"golang.org/x/tools/gopls/internal/lsp/source"
1311
)
1412

1513
// This is an internal test of the breakImportCycles logic.
1614
func TestBreakImportCycles(t *testing.T) {
1715

18-
type Graph = map[PackageID]*source.Metadata
16+
type Graph = map[PackageID]*Metadata
1917

2018
// cyclic returns a description of a cycle,
2119
// if the graph is cyclic, otherwise "".
@@ -62,11 +60,11 @@ func TestBreakImportCycles(t *testing.T) {
6260
// and the set of edges {a->b, b->c, b->d}.
6361
parse := func(s string) Graph {
6462
m := make(Graph)
65-
makeNode := func(name string) *source.Metadata {
63+
makeNode := func(name string) *Metadata {
6664
id := PackageID(name)
6765
n, ok := m[id]
6866
if !ok {
69-
n = &source.Metadata{
67+
n = &Metadata{
7068
ID: id,
7169
DepsByPkgPath: make(map[PackagePath]PackageID),
7270
}

0 commit comments

Comments
 (0)