From 7bca07fe92c72f79fce222609a2fcde7eed97a4f Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 3 Sep 2020 20:00:00 -0500 Subject: [PATCH 1/2] Stop using properties of idents. --- internal/indexer/indexer.go | 38 ++++++++++++++++++++---------------- internal/indexer/moniker.go | 4 ++-- internal/indexer/protocol.go | 2 +- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index f09f598e..ca35a0a4 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -248,7 +248,11 @@ func (i *Indexer) indexDefinitions() { // indexDefinitionsForPackage emits data for each definition within the given package. func (i *Indexer) indexDefinitionsForPackage(p *packages.Package) { for ident, obj := range p.TypesInfo.Defs { - pos, d, ok := i.positionAndDocument(p, ident, obj) + if obj == nil { + continue + } + + pos, d, ok := i.positionAndDocument(p, obj.Pos()) if !ok || !i.markRange(pos) { continue } @@ -268,23 +272,19 @@ func (i *Indexer) indexDefinitionsForPackage(p *packages.Package) { // positionAndDocument returns the position of the given object and the document info object // that contains it. If the given package is not the canonical package for the containing file // in the packagesByFile map, this method returns false. -func (i *Indexer) positionAndDocument(p *packages.Package, ident *ast.Ident, obj types.Object) (token.Position, *DocumentInfo, bool) { - if obj == nil { - return token.Position{}, nil, false - } +func (i *Indexer) positionAndDocument(p *packages.Package, pos token.Pos) (token.Position, *DocumentInfo, bool) { + position := p.Fset.Position(pos) - pos := p.Fset.Position(ident.Pos()) - - if packages := i.packagesByFile[pos.Filename]; len(packages) == 0 || packages[0] != p { + if packages := i.packagesByFile[position.Filename]; len(packages) == 0 || packages[0] != p { return token.Position{}, nil, false } - d, hasDocument := i.documents[pos.Filename] + d, hasDocument := i.documents[position.Filename] if !hasDocument { return token.Position{}, nil, false } - return pos, d, true + return position, d, true } // markRange sets an empty range identifier in the ranges map for the given position. @@ -351,7 +351,7 @@ func (i *Indexer) setDefinitionInfo(ident *ast.Ident, obj types.Object, d *Defin switch v := obj.(type) { case *types.Const: i.constsMutex.Lock() - i.consts[ident.Pos()] = d + i.consts[obj.Pos()] = d i.constsMutex.Unlock() case *types.Func: @@ -361,12 +361,12 @@ func (i *Indexer) setDefinitionInfo(ident *ast.Ident, obj types.Object, d *Defin case *types.Label: i.labelsMutex.Lock() - i.labels[ident.Pos()] = d + i.labels[obj.Pos()] = d i.labelsMutex.Unlock() case *types.PkgName: i.importsMutex.Lock() - i.imports[ident.Pos()] = d + i.imports[obj.Pos()] = d i.importsMutex.Unlock() case *types.TypeName: @@ -376,7 +376,7 @@ func (i *Indexer) setDefinitionInfo(ident *ast.Ident, obj types.Object, d *Defin case *types.Var: i.varsMutex.Lock() - i.vars[ident.Pos()] = d + i.vars[obj.Pos()] = d i.varsMutex.Unlock() } } @@ -391,13 +391,17 @@ func (i *Indexer) indexReferences() { // indexReferencesForPackage emits data for each reference within the given package. func (i *Indexer) indexReferencesForPackage(p *packages.Package) { - for ident, obj := range p.TypesInfo.Uses { - pos, d, ok := i.positionAndDocument(p, ident, obj) + for ident, definitionObj := range p.TypesInfo.Uses { + if definitionObj == nil { + continue + } + + pos, d, ok := i.positionAndDocument(p, ident.Pos()) if !ok { continue } - rangeID, ok := i.indexReference(p, d, ident, pos, obj) + rangeID, ok := i.indexReference(p, d, ident, pos, definitionObj) if !ok { continue } diff --git a/internal/indexer/moniker.go b/internal/indexer/moniker.go index 1edcfbff..377abddf 100644 --- a/internal/indexer/moniker.go +++ b/internal/indexer/moniker.go @@ -123,10 +123,10 @@ func monikerIdentifier(packageDataCache *PackageDataCache, p *packages.Package, return strings.Join([]string{ // Qualify function with receiver stripped of a pointer indicator `*` and its package path strings.TrimPrefix(strings.TrimPrefix(recv.Type().String(), "*"), obj.Pkg().Path()+"."), - ident.String(), + obj.Name(), }, ".") } } - return ident.String() + return obj.Name() } diff --git a/internal/indexer/protocol.go b/internal/indexer/protocol.go index 6213811f..93cd06e4 100644 --- a/internal/indexer/protocol.go +++ b/internal/indexer/protocol.go @@ -24,7 +24,7 @@ func rangeForObject(obj types.Object, ident *ast.Ident, pos token.Position) (pro line := pos.Line - 1 column := pos.Column - 1 - n := len(ident.Name) + n := len(obj.Name()) start := protocol.Pos{Line: line, Character: column + adjustment} end := protocol.Pos{Line: line, Character: column + n - adjustment} From 328fd426f3900a449f252973cc857f74186d8c93 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 3 Sep 2020 20:01:08 -0500 Subject: [PATCH 2/2] Remove unused ident parameters. --- internal/indexer/indexer.go | 48 +++++++++++++++---------------- internal/indexer/moniker.go | 11 ++++--- internal/indexer/moniker_test.go | 17 ++++------- internal/indexer/protocol.go | 3 +- internal/indexer/protocol_test.go | 3 -- 5 files changed, 36 insertions(+), 46 deletions(-) diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index ca35a0a4..5f068424 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -247,7 +247,7 @@ func (i *Indexer) indexDefinitions() { // indexDefinitionsForPackage emits data for each definition within the given package. func (i *Indexer) indexDefinitionsForPackage(p *packages.Package) { - for ident, obj := range p.TypesInfo.Defs { + for _, obj := range p.TypesInfo.Defs { if obj == nil { continue } @@ -257,7 +257,7 @@ func (i *Indexer) indexDefinitionsForPackage(p *packages.Package) { continue } - rangeID := i.indexDefinition(p, pos.Filename, d, ident, pos, obj) + rangeID := i.indexDefinition(p, pos.Filename, d, pos, obj) i.stripedMutex.LockKey(pos.Filename) i.ranges[pos.Filename][pos.Offset] = rangeID @@ -309,7 +309,7 @@ func (i *Indexer) markRange(pos token.Position) bool { } // indexDefinition emits data for the given definition object. -func (i *Indexer) indexDefinition(p *packages.Package, filename string, document *DocumentInfo, ident *ast.Ident, pos token.Position, obj types.Object) uint64 { +func (i *Indexer) indexDefinition(p *packages.Package, filename string, document *DocumentInfo, pos token.Position, obj types.Object) uint64 { // Create a hover result vertex and cache the result identifier keyed by the definition location. // Caching this gives us a big win for package documentation, which is likely to be large and is // repeated at each import and selector within referenced files. @@ -317,7 +317,7 @@ func (i *Indexer) indexDefinition(p *packages.Package, filename string, document return findHoverContents(i.packageDataCache, i.packages, p, obj) }) - rangeID := i.emitter.EmitRange(rangeForObject(obj, ident, pos)) + rangeID := i.emitter.EmitRange(rangeForObject(obj, pos)) resultSetID := i.emitter.EmitResultSet() defResultID := i.emitter.EmitDefinitionResult() @@ -327,14 +327,14 @@ func (i *Indexer) indexDefinition(p *packages.Package, filename string, document _ = i.emitter.EmitTextDocumentHover(resultSetID, hoverResultID) if _, ok := obj.(*types.PkgName); ok { - i.emitImportMoniker(resultSetID, p, ident, obj) + i.emitImportMoniker(resultSetID, p, obj) } - if ident.IsExported() { - i.emitExportMoniker(resultSetID, p, ident, obj) + if obj.Exported() { + i.emitExportMoniker(resultSetID, p, obj) } - i.setDefinitionInfo(ident, obj, &DefinitionInfo{ + i.setDefinitionInfo(obj, &DefinitionInfo{ DocumentID: document.DocumentID, RangeID: rangeID, ResultSetID: resultSetID, @@ -347,7 +347,7 @@ func (i *Indexer) indexDefinition(p *packages.Package, filename string, document // setDefinitionInfo stashes the given definition info indexed by the given object type and name. // This definition info will be accessible by invoking getDefinitionInfo with the same type and // name values (but not necessarily the same object). -func (i *Indexer) setDefinitionInfo(ident *ast.Ident, obj types.Object, d *DefinitionInfo) { +func (i *Indexer) setDefinitionInfo(obj types.Object, d *DefinitionInfo) { switch v := obj.(type) { case *types.Const: i.constsMutex.Lock() @@ -401,7 +401,7 @@ func (i *Indexer) indexReferencesForPackage(p *packages.Package) { continue } - rangeID, ok := i.indexReference(p, d, ident, pos, definitionObj) + rangeID, ok := i.indexReference(p, d, pos, definitionObj) if !ok { continue } @@ -413,12 +413,12 @@ func (i *Indexer) indexReferencesForPackage(p *packages.Package) { } // indexReference emits data for the given reference object. -func (i *Indexer) indexReference(p *packages.Package, document *DocumentInfo, ident *ast.Ident, pos token.Position, obj types.Object) (uint64, bool) { - if def := i.getDefinitionInfo(obj); def != nil { - return i.indexReferenceToDefinition(document, ident, pos, obj, def) +func (i *Indexer) indexReference(p *packages.Package, document *DocumentInfo, pos token.Position, definitionObj types.Object) (uint64, bool) { + if def := i.getDefinitionInfo(definitionObj); def != nil { + return i.indexReferenceToDefinition(document, pos, definitionObj, def) } - return i.indexReferenceToExternalDefinition(p, document, ident, pos, obj) + return i.indexReferenceToExternalDefinition(p, document, pos, definitionObj) } // getDefinitionInfo returns the definition info object for the given object. This requires that @@ -445,8 +445,8 @@ func (i *Indexer) getDefinitionInfo(obj types.Object) *DefinitionInfo { // indexReferenceToDefinition emits data for the given reference object that is defined within // an index target package. -func (i *Indexer) indexReferenceToDefinition(document *DocumentInfo, ident *ast.Ident, pos token.Position, obj types.Object, d *DefinitionInfo) (uint64, bool) { - rangeID := i.ensureRangeFor(ident, pos, obj) +func (i *Indexer) indexReferenceToDefinition(document *DocumentInfo, pos token.Position, definitionObj types.Object, d *DefinitionInfo) (uint64, bool) { + rangeID := i.ensureRangeFor(pos, definitionObj) _ = i.emitter.EmitNext(rangeID, d.ResultSetID) d.m.Lock() @@ -459,8 +459,8 @@ func (i *Indexer) indexReferenceToDefinition(document *DocumentInfo, ident *ast. // indexReferenceToExternalDefinition emits data for the given reference object that is not defined // within an index target package. This definition _may_ be resolvable by scanning dependencies, but // it is not guaranteed. -func (i *Indexer) indexReferenceToExternalDefinition(p *packages.Package, document *DocumentInfo, ident *ast.Ident, pos token.Position, obj types.Object) (uint64, bool) { - definitionPkg := obj.Pkg() +func (i *Indexer) indexReferenceToExternalDefinition(p *packages.Package, document *DocumentInfo, pos token.Position, definitionObj types.Object) (uint64, bool) { + definitionPkg := definitionObj.Pkg() if definitionPkg == nil { return 0, false } @@ -469,11 +469,11 @@ func (i *Indexer) indexReferenceToExternalDefinition(p *packages.Package, docume // (scoped ot the object's package name). Caching this gives us another big win as some // methods imported from other packages are likely to be used many times in a dependent // project (e.g., context.Context, http.Request, etc). - hoverResultID := i.makeCachedHoverResult(definitionPkg, obj, func() []protocol.MarkedString { - return findExternalHoverContents(i.packageDataCache, i.packages, p, obj) + hoverResultID := i.makeCachedHoverResult(definitionPkg, definitionObj, func() []protocol.MarkedString { + return findExternalHoverContents(i.packageDataCache, i.packages, p, definitionObj) }) - rangeID := i.ensureRangeFor(ident, pos, obj) + rangeID := i.ensureRangeFor(pos, definitionObj) refResultID := i.emitter.EmitReferenceResult() _ = i.emitter.EmitTextDocumentReferences(rangeID, refResultID) _ = i.emitter.EmitItemOfReferences(refResultID, []uint64{rangeID}, document.DocumentID) @@ -482,13 +482,13 @@ func (i *Indexer) indexReferenceToExternalDefinition(p *packages.Package, docume _ = i.emitter.EmitTextDocumentHover(rangeID, hoverResultID) } - i.emitImportMoniker(rangeID, p, ident, obj) + i.emitImportMoniker(rangeID, p, definitionObj) return rangeID, true } // ensureRangeFor returns a range identifier for the given object. If a range for the object has // not been emitted, a new vertex is created. -func (i *Indexer) ensureRangeFor(ident *ast.Ident, pos token.Position, obj types.Object) uint64 { +func (i *Indexer) ensureRangeFor(pos token.Position, obj types.Object) uint64 { i.stripedMutex.RLockKey(pos.Filename) rangeID, ok := i.ranges[pos.Filename][pos.Offset] i.stripedMutex.RUnlockKey(pos.Filename) @@ -497,7 +497,7 @@ func (i *Indexer) ensureRangeFor(ident *ast.Ident, pos token.Position, obj types } // Note: we calculate this outside of the critical section - start, end := rangeForObject(obj, ident, pos) + start, end := rangeForObject(obj, pos) i.stripedMutex.LockKey(pos.Filename) defer i.stripedMutex.UnlockKey(pos.Filename) diff --git a/internal/indexer/moniker.go b/internal/indexer/moniker.go index 377abddf..c79c690b 100644 --- a/internal/indexer/moniker.go +++ b/internal/indexer/moniker.go @@ -2,7 +2,6 @@ package indexer import ( "fmt" - "go/ast" "go/types" "strings" @@ -12,7 +11,7 @@ import ( // emitExportMoniker emits an export moniker for the given object linked to the given source // identifier (either a range or a result set identifier). This will also emit links between // the moniker vertex and the package information vertex representing the current module. -func (i *Indexer) emitExportMoniker(sourceID uint64, p *packages.Package, ident *ast.Ident, obj types.Object) { +func (i *Indexer) emitExportMoniker(sourceID uint64, p *packages.Package, obj types.Object) { if i.moduleName == "" { // Unknown dependencies, skip export monikers return @@ -20,7 +19,7 @@ func (i *Indexer) emitExportMoniker(sourceID uint64, p *packages.Package, ident i.addMonikers( "export", - strings.Trim(fmt.Sprintf("%s:%s", monikerPackage(obj), monikerIdentifier(i.packageDataCache, p, ident, obj)), ":"), + strings.Trim(fmt.Sprintf("%s:%s", monikerPackage(obj), monikerIdentifier(i.packageDataCache, p, obj)), ":"), sourceID, i.ensurePackageInformation(i.moduleName, i.moduleVersion), ) @@ -30,14 +29,14 @@ func (i *Indexer) emitExportMoniker(sourceID uint64, p *packages.Package, ident // identifier (either a range or a result set identifier). This will also emit links between // the moniker vertex and the package information vertex representing the dependency containing // the identifier. -func (i *Indexer) emitImportMoniker(sourceID uint64, p *packages.Package, ident *ast.Ident, obj types.Object) { +func (i *Indexer) emitImportMoniker(sourceID uint64, p *packages.Package, obj types.Object) { pkg := monikerPackage(obj) for _, moduleName := range packagePrefixes(pkg) { if moduleVersion, ok := i.dependencies[moduleName]; ok { i.addMonikers( "import", - strings.Trim(fmt.Sprintf("%s:%s", pkg, monikerIdentifier(i.packageDataCache, p, ident, obj)), ":"), + strings.Trim(fmt.Sprintf("%s:%s", pkg, monikerIdentifier(i.packageDataCache, p, obj)), ":"), sourceID, i.ensurePackageInformation(moduleName, moduleVersion), ) @@ -105,7 +104,7 @@ func monikerPackage(obj types.Object) string { // monikerIdentifier returns the identifier suffix used to construct a unique moniker for the given object. // A full moniker has the form `{package prefix}:{identifier suffix}`. The identifier is meant to act as a // qualified type path to the given object (e.g. `StructName.FieldName` or `StructName.MethodName`). -func monikerIdentifier(packageDataCache *PackageDataCache, p *packages.Package, ident *ast.Ident, obj types.Object) string { +func monikerIdentifier(packageDataCache *PackageDataCache, p *packages.Package, obj types.Object) string { if _, ok := obj.(*types.PkgName); ok { // Packages are identified uniquely by their package prefix return "" diff --git a/internal/indexer/moniker_test.go b/internal/indexer/moniker_test.go index a0655027..139aee9a 100644 --- a/internal/indexer/moniker_test.go +++ b/internal/indexer/moniker_test.go @@ -1,7 +1,6 @@ package indexer import ( - "go/ast" "go/constant" "go/token" "go/types" @@ -30,7 +29,7 @@ func TestEmitExportMoniker(t *testing.T) { constant.MakeBool(true), ) - indexer.emitExportMoniker(123, nil, &ast.Ident{Name: "foobar"}, object) + indexer.emitExportMoniker(123, nil, object) monikers := findMonikersByRangeOrReferenceResultID(w.elements, 123) if monikers == nil || len(monikers) < 1 { @@ -78,7 +77,7 @@ func TestEmitImportMoniker(t *testing.T) { constant.MakeBool(true), ) - indexer.emitImportMoniker(123, nil, &ast.Ident{Name: "foobar"}, object) + indexer.emitImportMoniker(123, nil, object) monikers := findMonikersByRangeOrReferenceResultID(w.elements, 123) if monikers == nil || len(monikers) < 1 { @@ -125,9 +124,8 @@ func TestPackagePrefixes(t *testing.T) { func TestMonikerIdentifierBasic(t *testing.T) { packages := getTestPackages(t) p, obj := findUseByName(t, packages, "Score") - ident := &ast.Ident{Name: "Score", NamePos: obj.Pos()} - if identifier := monikerIdentifier(NewPackageDataCache(), p, ident, obj); identifier != "Score" { + if identifier := monikerIdentifier(NewPackageDataCache(), p, obj); identifier != "Score" { t.Errorf("unexpected moniker identifier. want=%q have=%q", "Score", identifier) } } @@ -135,9 +133,8 @@ func TestMonikerIdentifierBasic(t *testing.T) { func TestMonikerIdentifierPackageName(t *testing.T) { packages := getTestPackages(t) p, obj := findUseByName(t, packages, "sync") - ident := &ast.Ident{Name: "sync", NamePos: obj.Pos()} - if identifier := monikerIdentifier(NewPackageDataCache(), p, ident, obj); identifier != "" { + if identifier := monikerIdentifier(NewPackageDataCache(), p, obj); identifier != "" { t.Errorf("unexpected moniker identifier. want=%q have=%q", "", identifier) } } @@ -145,9 +142,8 @@ func TestMonikerIdentifierPackageName(t *testing.T) { func TestMonikerIdentifierSignature(t *testing.T) { packages := getTestPackages(t) p, obj := findDefinitionByName(t, packages, "Doer") - ident := &ast.Ident{Name: "Doer", NamePos: obj.Pos()} - if identifier := monikerIdentifier(NewPackageDataCache(), p, ident, obj); identifier != "TestStruct.Doer" { + if identifier := monikerIdentifier(NewPackageDataCache(), p, obj); identifier != "TestStruct.Doer" { t.Errorf("unexpected moniker identifier. want=%q have=%q", "TestStruct.Doer", identifier) } } @@ -155,9 +151,8 @@ func TestMonikerIdentifierSignature(t *testing.T) { func TestMonikerIdentifierField(t *testing.T) { packages := getTestPackages(t) p, obj := findDefinitionByName(t, packages, "NestedB") - ident := &ast.Ident{Name: "NestedB", NamePos: obj.Pos()} - if identifier := monikerIdentifier(NewPackageDataCache(), p, ident, obj); identifier != "TestStruct.FieldWithAnonymousType.NestedB" { + if identifier := monikerIdentifier(NewPackageDataCache(), p, obj); identifier != "TestStruct.FieldWithAnonymousType.NestedB" { t.Errorf("unexpected moniker identifier. want=%q have=%q", "TestStruct.FieldWithAnonymousType.NestedB", identifier) } } diff --git a/internal/indexer/protocol.go b/internal/indexer/protocol.go index 93cd06e4..a23175d4 100644 --- a/internal/indexer/protocol.go +++ b/internal/indexer/protocol.go @@ -2,7 +2,6 @@ package indexer import ( "bytes" - "go/ast" "go/token" "go/types" "strings" @@ -16,7 +15,7 @@ const languageGo = "go" // rangeForObject transforms the position of the given object (1-indexed) into an LSP range // (0-indexed). If the object is a quoted package name, the leading and trailing quotes are // stripped from the resulting range's bounds. -func rangeForObject(obj types.Object, ident *ast.Ident, pos token.Position) (protocol.Pos, protocol.Pos) { +func rangeForObject(obj types.Object, pos token.Position) (protocol.Pos, protocol.Pos) { adjustment := 0 if pkgName, ok := obj.(*types.PkgName); ok && strings.HasPrefix(pkgName.Name(), `"`) { adjustment = 1 diff --git a/internal/indexer/protocol_test.go b/internal/indexer/protocol_test.go index c5ec6b1d..16030bc1 100644 --- a/internal/indexer/protocol_test.go +++ b/internal/indexer/protocol_test.go @@ -2,7 +2,6 @@ package indexer import ( "encoding/json" - "go/ast" "go/token" "go/types" "testing" @@ -14,7 +13,6 @@ import ( func TestRangeForObject(t *testing.T) { start, end := rangeForObject( types.NewPkgName(token.Pos(42), nil, "foobar", nil), - &ast.Ident{Name: "foobar"}, token.Position{Line: 10, Column: 25}, ) @@ -29,7 +27,6 @@ func TestRangeForObject(t *testing.T) { func TestRangeForObjectWithQuotedNamed(t *testing.T) { start, end := rangeForObject( types.NewPkgName(token.Pos(42), nil, `"foobar"`, nil), - &ast.Ident{Name: `"foobar"`}, token.Position{Line: 10, Column: 25}, )