Skip to content

Commit c7068a4

Browse files
doriabledoriable
authored
Resolve definition and type definition separately for options (#4161)
1 parent 50d143c commit c7068a4

File tree

3 files changed

+158
-91
lines changed

3 files changed

+158
-91
lines changed

‎private/buf/buflsp/file.go‎

Lines changed: 93 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/bufbuild/buf/private/bufpkg/bufcheck"
3434
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
3535
"github.com/bufbuild/buf/private/pkg/storage"
36+
"github.com/bufbuild/protocompile/experimental/ast"
3637
"github.com/bufbuild/protocompile/experimental/ast/predeclared"
3738
"github.com/bufbuild/protocompile/experimental/incremental"
3839
"github.com/bufbuild/protocompile/experimental/incremental/queries"
@@ -315,47 +316,54 @@ func (f *file) IndexSymbols(ctx context.Context) {
315316
}
316317

317318
// TODO: this could use a refactor, probably.
318-
319+
//
319320
// Resolve all unresolved symbols from this file
320321
for _, sym := range unresolved {
321-
ref, ok := sym.kind.(*reference)
322-
if !ok {
323-
// This shouldn't happen, logging a warning
324-
f.lsp.logger.Warn(
325-
"found unresolved non-reference symbol",
326-
slog.String("file", f.uri.Filename()),
327-
slog.Any("symbol", sym),
328-
)
329-
continue
330-
}
331-
file, ok := f.workspace.PathToFile()[ref.def.Span().Path()]
332-
if !ok {
333-
// Check current file
334-
if ref.def.Span().Path() != f.objectInfo.Path() {
335-
// This can happen if this references a predeclared type or if the file we are
336-
// checking has not indexed imports.
322+
switch kind := sym.kind.(type) {
323+
case *reference:
324+
def := f.resolveASTDefinition(kind.def, kind.fullName)
325+
sym.def = def
326+
if def == nil {
327+
// In the case where the symbol is not resolved, we continue
337328
continue
338329
}
339-
file = f
340-
}
341-
def, ok := file.referenceableSymbols[ref.fullName]
342-
if !ok {
343-
// This could happen in the case where we are in the cache for example, and we do not
344-
// have access to a buildable workspace.
345-
continue
346-
}
347-
sym.def = def
348-
referenceable, ok := def.kind.(*referenceable)
349-
if !ok {
330+
referenceable, ok := def.kind.(*referenceable)
331+
if !ok {
332+
// This shouldn't happen, logging a warning
333+
f.lsp.logger.Warn(
334+
"found non-referenceable symbol in index",
335+
slog.String("file", f.uri.Filename()),
336+
slog.Any("symbol", def),
337+
)
338+
continue
339+
}
340+
referenceable.references = append(referenceable.references, sym)
341+
case *option:
342+
def := f.resolveASTDefinition(kind.def, kind.defFullName)
343+
sym.def = def
344+
if def != nil {
345+
referenceable, ok := def.kind.(*referenceable)
346+
if !ok {
347+
// This shouldn't happen, logging a warning
348+
f.lsp.logger.Warn(
349+
"found non-referenceable symbol in index",
350+
slog.String("file", f.uri.Filename()),
351+
slog.Any("symbol", def),
352+
)
353+
} else {
354+
referenceable.references = append(referenceable.references, sym)
355+
}
356+
}
357+
typeDef := f.resolveASTDefinition(kind.typeDef, kind.typeDefFullName)
358+
sym.typeDef = typeDef
359+
default:
350360
// This shouldn't happen, logging a warning
351361
f.lsp.logger.Warn(
352-
"found non-referenceable symbol in index",
362+
"found unresolved non-reference and non-option symbol",
353363
slog.String("file", f.uri.Filename()),
354-
slog.Any("symbol", def),
364+
slog.Any("symbol", sym),
355365
)
356-
continue
357366
}
358-
referenceable.references = append(referenceable.references, sym)
359367
}
360368

361369
// Resolve all references outside of this file to symbols in this file
@@ -364,20 +372,28 @@ func (f *file) IndexSymbols(ctx context.Context) {
364372
continue // ignore self
365373
}
366374
for _, sym := range file.referenceSymbols {
367-
ref, ok := sym.kind.(*reference)
368-
if !ok {
375+
var fullName ir.FullName
376+
switch kind := sym.kind.(type) {
377+
case *reference:
378+
if kind.def.Span().Path() != f.objectInfo.Path() {
379+
continue
380+
}
381+
fullName = kind.fullName
382+
case *option:
383+
if kind.def.Span().Path() != f.objectInfo.Path() {
384+
continue
385+
}
386+
fullName = kind.defFullName
387+
default:
369388
// This shouldn't happen, logging a warning
370389
f.lsp.logger.Warn(
371-
"found unresolved non-reference symbol",
390+
"found unresolved non-reference and non-option symbol",
372391
slog.String("file", f.uri.Filename()),
373392
slog.Any("symbol", sym),
374393
)
375394
continue
376395
}
377-
if ref.def.Span().Path() != f.objectInfo.Path() {
378-
continue
379-
}
380-
def, ok := f.referenceableSymbols[ref.fullName]
396+
def, ok := f.referenceableSymbols[fullName]
381397
if !ok {
382398
// This shouldn't happen, if a symbol is pointing at this file, all definitions
383399
// should be resolved, logging a warning
@@ -524,12 +540,12 @@ func (f *file) irToSymbols(irSymbol ir.Symbol) ([]*symbol, []*symbol) {
524540
resolved = append(resolved, tag)
525541
unresolved = append(unresolved, f.messageToSymbols(irSymbol.AsMember().Options())...)
526542
case ir.SymbolKindExtension:
527-
// TODO: we should figure out if we need to do any resolution here.
543+
// The symbol only includes the extension field name.
528544
ext := &symbol{
529545
ir: irSymbol,
530546
file: f,
531547
span: irSymbol.AsMember().AST().AsExtend().Extendee.Span(),
532-
kind: &static{
548+
kind: &referenceable{
533549
ast: irSymbol.AsMember().AST(),
534550
},
535551
}
@@ -687,19 +703,15 @@ func (f *file) messageToSymbolsHelper(msg ir.MessageValue, index int, parents []
687703
// NOTE: no [ir.Symbol] for option elements
688704
file: f,
689705
span: span,
690-
kind: &reference{
691-
def: element.Field().AST(),
692-
fullName: element.Field().FullName(),
706+
kind: &option{
707+
def: element.Field().AST(),
708+
defFullName: element.Field().FullName(),
709+
typeDef: element.Type().AST(),
710+
typeDefFullName: element.Type().FullName(),
693711
},
694-
isOption: true,
695712
}
696713
symbols = append(symbols, sym)
697714
if !element.AsMessage().IsZero() {
698-
// For message value elements, we use the Type AST.
699-
sym.kind = &reference{
700-
def: element.Type().AST(),
701-
fullName: element.Type().FullName(),
702-
}
703715
symbols = append(symbols, f.messageToSymbolsHelper(element.AsMessage(), index+1, symbols)...)
704716
}
705717

@@ -725,11 +737,12 @@ func (f *file) messageToSymbolsHelper(msg ir.MessageValue, index int, parents []
725737
// NOTE: no [ir.Symbol] for option elements
726738
file: f,
727739
span: component.Span(),
728-
kind: &reference{
729-
def: parentType.Type().AST(),
730-
fullName: parentType.Type().FullName(),
740+
kind: &option{
741+
def: parentType.AsValue().Field().AST(),
742+
defFullName: parentType.AsValue().Field().FullName(),
743+
typeDef: parentType.Type().AST(),
744+
typeDefFullName: parentType.Type().FullName(),
731745
},
732-
isOption: true,
733746
}
734747
symbols = append(symbols, sym)
735748
}
@@ -741,6 +754,23 @@ func (f *file) messageToSymbolsHelper(msg ir.MessageValue, index int, parents []
741754
return symbols
742755
}
743756

757+
// resolveASTDefinition is a helper for resolving the [ast.DeclDef] to the *[symbol], if
758+
// there is a matching indexed *[symbol].
759+
func (f *file) resolveASTDefinition(def ast.DeclDef, defName ir.FullName) *symbol {
760+
file, ok := f.workspace.PathToFile()[def.Span().Path()]
761+
if !ok {
762+
// Check current file
763+
if def.Span().Path() != f.objectInfo.Path() {
764+
// If there is no file for the [ast.DeclDef] span, which can if it is a predeclared
765+
// type, for example, or if the file we are checking has not indexed imports, then
766+
// we return nil.
767+
return nil
768+
}
769+
file = f
770+
}
771+
return file.referenceableSymbols[defName]
772+
}
773+
744774
// SymbolAt finds a symbol in this file at the given cursor position, if one exists.
745775
//
746776
// Returns nil if no symbol is found.
@@ -766,7 +796,15 @@ func (f *file) SymbolAt(ctx context.Context, cursor protocol.Position) *symbol {
766796
}
767797
symbol = before
768798
}
769-
f.lsp.logger.DebugContext(ctx, "symbol at", slog.Int("line", int(cursor.Line)), slog.Int("character", int(cursor.Character)), slog.Any("symbol", symbol))
799+
if symbol != nil {
800+
f.lsp.logger.DebugContext(
801+
ctx,
802+
"symbol at",
803+
slog.Int("line", int(cursor.Line)),
804+
slog.Int("character", int(cursor.Character)),
805+
slog.Any("symbol", symbol),
806+
)
807+
}
770808
return symbol
771809
}
772810

‎private/buf/buflsp/server.go‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -336,12 +336,7 @@ func (s *server) Hover(
336336
ctx context.Context,
337337
params *protocol.HoverParams,
338338
) (*protocol.Hover, error) {
339-
file := s.fileManager.Get(params.TextDocument.URI)
340-
if file == nil {
341-
return nil, nil
342-
}
343-
344-
symbol := file.SymbolAt(ctx, params.Position)
339+
symbol := s.getSymbol(ctx, params.TextDocument.URI, params.Position)
345340
if symbol == nil {
346341
return nil, nil
347342
}
@@ -370,32 +365,26 @@ func (s *server) Definition(
370365
ctx context.Context,
371366
params *protocol.DefinitionParams,
372367
) ([]protocol.Location, error) {
373-
return s.definition(ctx, params.TextDocument.URI, params.Position)
368+
symbol := s.getSymbol(ctx, params.TextDocument.URI, params.Position)
369+
if symbol == nil {
370+
return nil, nil
371+
}
372+
return []protocol.Location{
373+
symbol.Definition(),
374+
}, nil
374375
}
375376

376-
// TypeDefinition is the entry point for go-to-type-definition.
377+
// TypeDefinition is the entry point for go-to type-definition.
377378
func (s *server) TypeDefinition(
378379
ctx context.Context,
379380
params *protocol.TypeDefinitionParams,
380381
) ([]protocol.Location, error) {
381-
return s.definition(ctx, params.TextDocument.URI, params.Position)
382-
}
383-
384-
// definition powers [server.Definition] and [server.TypeDefinition], as they are not meaningfully
385-
// different in protobuf, but users may be used to using either.
386-
func (s *server) definition(ctx context.Context, uri protocol.URI, position protocol.Position) ([]protocol.Location, error) {
387-
file := s.fileManager.Get(uri)
388-
if file == nil {
389-
return nil, nil
390-
}
391-
392-
symbol := file.SymbolAt(ctx, position)
382+
symbol := s.getSymbol(ctx, params.TextDocument.URI, params.Position)
393383
if symbol == nil {
394384
return nil, nil
395385
}
396-
397386
return []protocol.Location{
398-
symbol.Definition(),
387+
symbol.TypeDefinition(),
399388
}, nil
400389
}
401390

@@ -404,11 +393,7 @@ func (s *server) References(
404393
ctx context.Context,
405394
params *protocol.ReferenceParams,
406395
) ([]protocol.Location, error) {
407-
file := s.fileManager.Get(params.TextDocument.URI)
408-
if file == nil {
409-
return nil, nil
410-
}
411-
symbol := file.SymbolAt(ctx, params.Position)
396+
symbol := s.getSymbol(ctx, params.TextDocument.URI, params.Position)
412397
if symbol == nil {
413398
return nil, nil
414399
}
@@ -471,7 +456,8 @@ func (s *server) SemanticTokensFull(
471456
for _, symbol := range file.symbols {
472457
var semanticType uint32
473458
var semanticModifier uint32
474-
if symbol.isOption {
459+
_, ok := symbol.kind.(*option)
460+
if ok {
475461
semanticType = semanticTypeDecorator
476462
} else {
477463
switch symbol.ir.Kind() {
@@ -572,3 +558,17 @@ func (s *server) DocumentSymbol(ctx context.Context, params *protocol.DocumentSy
572558
}
573559
return anyResults, nil
574560
}
561+
562+
// getSymbol is a helper function that gets the *[symbol] for the given [protocol.URI] and
563+
// [protocol.Position].
564+
func (s *server) getSymbol(
565+
ctx context.Context,
566+
uri protocol.URI,
567+
position protocol.Position,
568+
) *symbol {
569+
file := s.fileManager.Get(uri)
570+
if file == nil {
571+
return nil
572+
}
573+
return file.SymbolAt(ctx, position)
574+
}

0 commit comments

Comments
 (0)