From a5375858524b023ced996e5e0a4d098a78b4c727 Mon Sep 17 00:00:00 2001 From: jupblb Date: Fri, 26 Sep 2025 11:32:54 +0200 Subject: [PATCH 1/5] Add type information to local variables in SCIP index For all local symbols (local variables, parameters, etc.), include their Go type information in the SymbolInformation.SignatureDocumentation field. This allows code intelligence features like hover tooltips to show the type of local variables without needing to jump to their definitions. The implementation: - Extends localSymbolInfo struct to store type information alongside symbol ID - Updates createNewLocalSymbol to extract type from types.Object - Includes type string in SignatureDocumentation for each local symbol - Adds display names for better readability in UI Cherry-picked from 980222947666047d464c4b236418b09a53f2f056 (excluding tests) Amp-Thread-ID: https://ampcode.com/threads/T-e6feb9ee-1c7e-4b0b-957e-80033288d4d7 --- internal/visitors/visitor_file.go | 84 ++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 13 deletions(-) diff --git a/internal/visitors/visitor_file.go b/internal/visitors/visitor_file.go index 6411024..07b505f 100644 --- a/internal/visitors/visitor_file.go +++ b/internal/visitors/visitor_file.go @@ -23,6 +23,14 @@ const ( symbolReference = int32(scip.SymbolRole_ReadAccess) ) +// localSymbolInfo contains information about a local symbol including its type +type localSymbolInfo struct { + symbol string + typeString string + name string + kind string // "var" or "const" (only these are valid for local symbols) +} + func NewFileVisitor( doc *document.Document, pkg *packages.Package, @@ -48,7 +56,7 @@ func NewFileVisitor( pkg: pkg, file: file, pkgLookup: pkgLookup, - locals: map[token.Pos]string{}, + locals: map[token.Pos]*localSymbolInfo{}, pkgSymbols: pkgSymbols, globalSymbols: globalSymbols, occurrences: occurrences, @@ -77,8 +85,8 @@ type fileVisitor struct { // soething pkgLookup loader.PackageLookup - // local definition position to symbol - locals map[token.Pos]string + // local definition position to symbol and its type information + locals map[token.Pos]*localSymbolInfo // field definition position to symbol for the package pkgSymbols *lookup.Package @@ -103,13 +111,39 @@ type fileVisitor struct { // Implements ast.Visitor var _ ast.Visitor = &fileVisitor{} -func (f *fileVisitor) createNewLocalSymbol(pos token.Pos) string { +func (f *fileVisitor) createNewLocalSymbol(pos token.Pos, obj types.Object) string { if _, ok := f.locals[pos]; ok { panic("Cannot create a new local symbol for an ident that has already been created") } - f.locals[pos] = fmt.Sprintf("local %d", len(f.locals)) - return f.locals[pos] + symbol := fmt.Sprintf("local %d", len(f.locals)) + var typeString string + var name string + var kind string + + if obj != nil { + name = obj.Name() + + if typ := obj.Type(); typ != nil { + typeString = typ.String() + } + + switch obj.(type) { + case *types.Const: + kind = "const" + case *types.Var: + kind = "var" + } + } + + f.locals[pos] = &localSymbolInfo{ + symbol: symbol, + typeString: typeString, + name: name, + kind: kind, + } + + return symbol } func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { @@ -127,7 +161,7 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { } if node.Name != nil && node.Name.Name != "." { - sym := v.createNewLocalSymbol(node.Name.Pos()) + sym := v.createNewLocalSymbol(node.Name.Pos(), nil) v.NewDefinition(sym, symbols.RangeFromName(v.pkg.Fset.Position(node.Name.Pos()), node.Name.Name, false)) // Save package name override, so that we use the new local symbol @@ -197,7 +231,7 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { // Short circuit on case clauses if obj, ok := v.overrides.caseClauses[node.Pos()]; ok { - sym := v.createNewLocalSymbol(obj.Pos()) + sym := v.createNewLocalSymbol(obj.Pos(), obj) v.NewDefinition(sym, scipRange(position, obj)) return nil } @@ -213,7 +247,7 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { } else if globalSymbol, ok := v.globalSymbols.GetSymbol(v.pkg, def.Pos()); ok { sym = globalSymbol } else { - sym = v.createNewLocalSymbol(def.Pos()) + sym = v.createNewLocalSymbol(def.Pos(), def) } v.NewDefinition(sym, scipRange(position, def)) @@ -228,7 +262,7 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { ) if localSymbol, ok := v.locals[ref.Pos()]; ok { - symbol = localSymbol + symbol = localSymbol.symbol if _, ok := v.overrides.caseClauses[ref.Pos()]; ok { overrideType = v.pkg.TypesInfo.TypeOf(node) @@ -339,9 +373,33 @@ func (v *fileVisitor) ToScipDocument() *scip.Document { documentSymbols := v.pkgSymbols.SymbolsForFile(documentFile) for _, localSymbol := range v.locals { - documentSymbols = append(documentSymbols, &scip.SymbolInformation{ - Symbol: localSymbol, - }) + symbolInfo := &scip.SymbolInformation{ + Symbol: localSymbol.symbol, + DisplayName: localSymbol.name, + } + + var signatureText string + if localSymbol.name != "" { + if localSymbol.kind != "" { + signatureText = fmt.Sprintf("%s ", localSymbol.kind) + } + signatureText += localSymbol.name + } + if localSymbol.typeString != "" { + if signatureText != "" { + signatureText += " " + } + signatureText += localSymbol.typeString + } + + if signatureText != "" { + symbolInfo.SignatureDocumentation = &scip.Document{ + Language: "go", + Text: signatureText, + } + } + + documentSymbols = append(documentSymbols, symbolInfo) } return &scip.Document{ From 7248a5879e9084a1ce66e89edff23ee9cc7e0d84 Mon Sep 17 00:00:00 2001 From: jupblb Date: Mon, 29 Sep 2025 11:13:45 +0200 Subject: [PATCH 2/5] Refactor local symbol handling to use types.Object directly - Move LocalSymbolInfo (renamed to Local) from visitors to lookup package for better package organization alongside Global and Package - Store types.Object directly instead of extracting fields upfront - Extract type information lazily in SignatureText() method - Cleaner separation of concerns and more maintainable code The refactoring preserves all existing functionality while improving code organization and reducing complexity in visitor_file.go Amp-Thread-ID: https://ampcode.com/threads/T-654bed0b-94e2-4563-a7fd-d730d7eb28d9 --- internal/lookup/local.go | 41 +++++++++++++++++++ internal/visitors/visitor_file.go | 68 +++++++------------------------ 2 files changed, 55 insertions(+), 54 deletions(-) create mode 100644 internal/lookup/local.go diff --git a/internal/lookup/local.go b/internal/lookup/local.go new file mode 100644 index 0000000..0dd74db --- /dev/null +++ b/internal/lookup/local.go @@ -0,0 +1,41 @@ +package lookup + +import ( + "go/types" + "strings" +) + +// Local contains information about a local symbol +type Local struct { + Symbol string + Obj types.Object +} + +// SignatureText builds a concise signature string for this local symbol. +func (l *Local) SignatureText() string { + if l.Obj == nil { + return "" + } + + var parts []string + + // local symbol can only represent const or var + switch l.Obj.(type) { + case *types.Const: + parts = append(parts, "const") + case *types.Var: + parts = append(parts, "var") + } + + if name := l.Obj.Name(); name != "" { + parts = append(parts, name) + } + + if t := l.Obj.Type(); t != nil { + if ts := t.String(); ts != "" { + parts = append(parts, ts) + } + } + + return strings.Join(parts, " ") +} diff --git a/internal/visitors/visitor_file.go b/internal/visitors/visitor_file.go index 07b505f..81398de 100644 --- a/internal/visitors/visitor_file.go +++ b/internal/visitors/visitor_file.go @@ -23,14 +23,6 @@ const ( symbolReference = int32(scip.SymbolRole_ReadAccess) ) -// localSymbolInfo contains information about a local symbol including its type -type localSymbolInfo struct { - symbol string - typeString string - name string - kind string // "var" or "const" (only these are valid for local symbols) -} - func NewFileVisitor( doc *document.Document, pkg *packages.Package, @@ -56,7 +48,7 @@ func NewFileVisitor( pkg: pkg, file: file, pkgLookup: pkgLookup, - locals: map[token.Pos]*localSymbolInfo{}, + locals: map[token.Pos]*lookup.Local{}, pkgSymbols: pkgSymbols, globalSymbols: globalSymbols, occurrences: occurrences, @@ -86,7 +78,7 @@ type fileVisitor struct { pkgLookup loader.PackageLookup // local definition position to symbol and its type information - locals map[token.Pos]*localSymbolInfo + locals map[token.Pos]*lookup.Local // field definition position to symbol for the package pkgSymbols *lookup.Package @@ -117,30 +109,10 @@ func (f *fileVisitor) createNewLocalSymbol(pos token.Pos, obj types.Object) stri } symbol := fmt.Sprintf("local %d", len(f.locals)) - var typeString string - var name string - var kind string - - if obj != nil { - name = obj.Name() - - if typ := obj.Type(); typ != nil { - typeString = typ.String() - } - - switch obj.(type) { - case *types.Const: - kind = "const" - case *types.Var: - kind = "var" - } - } - f.locals[pos] = &localSymbolInfo{ - symbol: symbol, - typeString: typeString, - name: name, - kind: kind, + f.locals[pos] = &lookup.Local{ + Symbol: symbol, + Obj: obj, } return symbol @@ -262,7 +234,7 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { ) if localSymbol, ok := v.locals[ref.Pos()]; ok { - symbol = localSymbol.symbol + symbol = localSymbol.Symbol if _, ok := v.overrides.caseClauses[ref.Pos()]; ok { overrideType = v.pkg.TypesInfo.TypeOf(node) @@ -374,28 +346,16 @@ func (v *fileVisitor) ToScipDocument() *scip.Document { documentSymbols := v.pkgSymbols.SymbolsForFile(documentFile) for _, localSymbol := range v.locals { symbolInfo := &scip.SymbolInformation{ - Symbol: localSymbol.symbol, - DisplayName: localSymbol.name, + Symbol: localSymbol.Symbol, } - var signatureText string - if localSymbol.name != "" { - if localSymbol.kind != "" { - signatureText = fmt.Sprintf("%s ", localSymbol.kind) - } - signatureText += localSymbol.name - } - if localSymbol.typeString != "" { - if signatureText != "" { - signatureText += " " - } - signatureText += localSymbol.typeString - } - - if signatureText != "" { - symbolInfo.SignatureDocumentation = &scip.Document{ - Language: "go", - Text: signatureText, + if obj := localSymbol.Obj; obj != nil { + symbolInfo.DisplayName = obj.Name() + if txt := localSymbol.SignatureText(); txt != "" { + symbolInfo.SignatureDocumentation = &scip.Document{ + Language: "go", + Text: txt, + } } } From c445a4e7d4a2b58a07e2144e748b53b46ff49c56 Mon Sep 17 00:00:00 2001 From: jupblb Date: Mon, 29 Sep 2025 11:23:26 +0200 Subject: [PATCH 3/5] Styling fixes --- internal/visitors/visitor_file.go | 37 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/visitors/visitor_file.go b/internal/visitors/visitor_file.go index 81398de..033f4c0 100644 --- a/internal/visitors/visitor_file.go +++ b/internal/visitors/visitor_file.go @@ -103,14 +103,14 @@ type fileVisitor struct { // Implements ast.Visitor var _ ast.Visitor = &fileVisitor{} -func (f *fileVisitor) createNewLocalSymbol(pos token.Pos, obj types.Object) string { - if _, ok := f.locals[pos]; ok { +func (v *fileVisitor) createNewLocalSymbol(pos token.Pos, obj types.Object) string { + if _, ok := v.locals[pos]; ok { panic("Cannot create a new local symbol for an ident that has already been created") } - symbol := fmt.Sprintf("local %d", len(f.locals)) + symbol := fmt.Sprintf("local %d", len(v.locals)) - f.locals[pos] = &lookup.Local{ + v.locals[pos] = &lookup.Local{ Symbol: symbol, Obj: obj, } @@ -133,16 +133,18 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { } if node.Name != nil && node.Name.Name != "." { - sym := v.createNewLocalSymbol(node.Name.Pos(), nil) - v.NewDefinition(sym, symbols.RangeFromName(v.pkg.Fset.Position(node.Name.Pos()), node.Name.Name, false)) + symName := v.createNewLocalSymbol(node.Name.Pos(), nil) + rangeFromName := symbols.RangeFromName( + v.pkg.Fset.Position(node.Name.Pos()), node.Name.Name, false) + v.NewDefinition(symName, rangeFromName) // Save package name override, so that we use the new local symbol // within this file - v.overrides.pkgNameOverride[newtypes.GetID(importedPackage)] = sym + v.overrides.pkgNameOverride[newtypes.GetID(importedPackage)] = symName } position := v.pkg.Fset.Position(node.Path.Pos()) - v.emitImportReference(v.globalSymbols, v.doc, position, importedPackage) + v.emitImportReference(v.globalSymbols, position, importedPackage) return nil @@ -203,8 +205,8 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { // Short circuit on case clauses if obj, ok := v.overrides.caseClauses[node.Pos()]; ok { - sym := v.createNewLocalSymbol(obj.Pos(), obj) - v.NewDefinition(sym, scipRange(position, obj)) + symName := v.createNewLocalSymbol(obj.Pos(), obj) + v.NewDefinition(symName, scipRange(position, obj)) return nil } @@ -213,16 +215,16 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { // Emit Definition def := info.Defs[node] if def != nil { - var sym string + var symName string if pkgSymbols, ok := v.pkgSymbols.GetSymbol(def.Pos()); ok { - sym = pkgSymbols + symName = pkgSymbols } else if globalSymbol, ok := v.globalSymbols.GetSymbol(v.pkg, def.Pos()); ok { - sym = globalSymbol + symName = globalSymbol } else { - sym = v.createNewLocalSymbol(def.Pos(), def) + symName = v.createNewLocalSymbol(def.Pos(), def) } - v.NewDefinition(sym, scipRange(position, def)) + v.NewDefinition(symName, scipRange(position, def)) } // Emit Reference @@ -291,17 +293,16 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { func (v *fileVisitor) emitImportReference( globalSymbols *lookup.Global, - doc *document.Document, position token.Position, importedPackage *packages.Package, ) { scipRange := symbols.RangeFromName(position, importedPackage.PkgPath, true) - symbol := globalSymbols.GetPkgNameSymbol(importedPackage) - if symbol == nil { + if scipRange == nil { handler.ErrOrPanic("Missing symbol for package path: %s", importedPackage.ID) return } + symbol := globalSymbols.GetPkgNameSymbol(importedPackage) if symbol == nil { handler.ErrOrPanic("Missing symbol information for package: %s", importedPackage.ID) return From fc1daaea055d92474f9b694ca2a11d7c4f1194d7 Mon Sep 17 00:00:00 2001 From: jupblb Date: Mon, 29 Sep 2025 11:27:29 +0200 Subject: [PATCH 4/5] Review fixes --- internal/visitors/visitor_file.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/visitors/visitor_file.go b/internal/visitors/visitor_file.go index 033f4c0..31c0afd 100644 --- a/internal/visitors/visitor_file.go +++ b/internal/visitors/visitor_file.go @@ -48,7 +48,7 @@ func NewFileVisitor( pkg: pkg, file: file, pkgLookup: pkgLookup, - locals: map[token.Pos]*lookup.Local{}, + locals: map[token.Pos]lookup.Local{}, pkgSymbols: pkgSymbols, globalSymbols: globalSymbols, occurrences: occurrences, @@ -78,7 +78,7 @@ type fileVisitor struct { pkgLookup loader.PackageLookup // local definition position to symbol and its type information - locals map[token.Pos]*lookup.Local + locals map[token.Pos]lookup.Local // field definition position to symbol for the package pkgSymbols *lookup.Package @@ -110,7 +110,7 @@ func (v *fileVisitor) createNewLocalSymbol(pos token.Pos, obj types.Object) stri symbol := fmt.Sprintf("local %d", len(v.locals)) - v.locals[pos] = &lookup.Local{ + v.locals[pos] = lookup.Local{ Symbol: symbol, Obj: obj, } @@ -345,14 +345,14 @@ func (v *fileVisitor) ToScipDocument() *scip.Document { } documentSymbols := v.pkgSymbols.SymbolsForFile(documentFile) - for _, localSymbol := range v.locals { + for _, local := range v.locals { symbolInfo := &scip.SymbolInformation{ - Symbol: localSymbol.Symbol, + Symbol: local.Symbol, } - if obj := localSymbol.Obj; obj != nil { + if obj := local.Obj; obj != nil { symbolInfo.DisplayName = obj.Name() - if txt := localSymbol.SignatureText(); txt != "" { + if txt := local.SignatureText(); txt != "" { symbolInfo.SignatureDocumentation = &scip.Document{ Language: "go", Text: txt, From e53bd19cb832df2b8ed9c39aef651243370d2061 Mon Sep 17 00:00:00 2001 From: jupblb Date: Mon, 29 Sep 2025 11:44:46 +0200 Subject: [PATCH 5/5] Pass PkgName object for import aliases and improve signature text - Pass the actual types.PkgName object when creating local symbols for import aliases - Update SignatureText to handle PkgName objects: adds 'import' prefix and shows package path - Now import aliases display as 'import foo fmt' instead of just empty or invalid type Amp-Thread-ID: https://ampcode.com/threads/T-6978c85f-14b7-46c8-ab55-9b642e013371 --- internal/lookup/local.go | 16 ++++++++++++---- internal/visitors/visitor_file.go | 3 ++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/lookup/local.go b/internal/lookup/local.go index 0dd74db..e61f119 100644 --- a/internal/lookup/local.go +++ b/internal/lookup/local.go @@ -19,10 +19,11 @@ func (l *Local) SignatureText() string { var parts []string - // local symbol can only represent const or var switch l.Obj.(type) { case *types.Const: parts = append(parts, "const") + case *types.PkgName: + parts = append(parts, "import") case *types.Var: parts = append(parts, "var") } @@ -31,9 +32,16 @@ func (l *Local) SignatureText() string { parts = append(parts, name) } - if t := l.Obj.Type(); t != nil { - if ts := t.String(); ts != "" { - parts = append(parts, ts) + // For PkgName, append the package path instead of type + if pkgName, isPkgName := l.Obj.(*types.PkgName); isPkgName { + if imported := pkgName.Imported(); imported != nil { + parts = append(parts, imported.Path()) + } + } else { + if t := l.Obj.Type(); t != nil { + if ts := t.String(); ts != "" { + parts = append(parts, ts) + } } } diff --git a/internal/visitors/visitor_file.go b/internal/visitors/visitor_file.go index 31c0afd..930df9e 100644 --- a/internal/visitors/visitor_file.go +++ b/internal/visitors/visitor_file.go @@ -133,7 +133,8 @@ func (v *fileVisitor) Visit(n ast.Node) ast.Visitor { } if node.Name != nil && node.Name.Name != "." { - symName := v.createNewLocalSymbol(node.Name.Pos(), nil) + pkgAlias := v.pkg.TypesInfo.Defs[node.Name] + symName := v.createNewLocalSymbol(node.Name.Pos(), pkgAlias) rangeFromName := symbols.RangeFromName( v.pkg.Fset.Position(node.Name.Pos()), node.Name.Name, false) v.NewDefinition(symName, rangeFromName)