From 1b72a3995392d8a520eaab9724ea706127e24288 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Wed, 29 May 2024 16:11:28 +0100 Subject: [PATCH 1/4] If SignatureDocumentation is present in SCIP, use it for hover text 1. We prepend a markdown-formatted signature to the hover text if SignatureDocumentation field if present 2. If the field is absent, we fall back to old behaviour of assuming that signature documentation is already present in the hover text --- .../lsifstore/locations_by_position.go | 13 +++- .../lsifstore/locations_by_position_test.go | 63 +++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go index 62d2a2b4d671..1450aa448527 100644 --- a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go +++ b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go @@ -283,7 +283,7 @@ func extractOccurrenceData(document *scip.Document, occurrence *scip.Occurrence) // we should include in reference and implementation searches. if symbol := scip.FindSymbol(document, occurrence.Symbol); symbol != nil { - hoverText = symbol.Documentation + hoverText = symbolHoverText(symbol) for _, rel := range symbol.Relationships { if rel.IsDefinition { @@ -366,8 +366,15 @@ func monikersToString(vs []precise.MonikerData) string { return strings.Join(strs, ", ") } -// -// +func symbolHoverText(symbol *scip.SymbolInformation) []string { + if symbol.SignatureDocumentation != nil { + signature := []string{fmt.Sprintf("```%s\n%s\n```", symbol.SignatureDocumentation.Language, symbol.SignatureDocumentation.Text)} + return append(signature, symbol.Documentation...) + } else { + return symbol.Documentation + } + +} func (s *store) ExtractDefinitionLocationsFromPosition(ctx context.Context, locationKey LocationKey) (_ []shared.Location, _ []string, err error) { return s.extractLocationsFromPosition(ctx, extractDefinitionRanges, symbolExtractDefault, s.operations.getDefinitionLocations, locationKey) diff --git a/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go b/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go index 38e5eb9d3937..72cbfa17f4b3 100644 --- a/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go +++ b/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go @@ -556,6 +556,69 @@ func TestExtractOccurrenceData(t *testing.T) { } }) + t.Run("documentation and signature documentation", func(t *testing.T) { + testCases := []struct { + explanation string + document *scip.Document + occurrence *scip.Occurrence + hoverText []string + }{ + { + explanation: "#1 backwards compatibility: SignatureDocumentation is absent, Documentation is present", + document: &scip.Document{ + Symbols: []*scip.SymbolInformation{ + { + Symbol: "react 17.1 main.go func1", + Documentation: []string{ + "```go\nfunc1()\n```", + "it does the thing", + }, + }, + }, + }, + occurrence: &scip.Occurrence{ + Symbol: "react 17.1 main.go func1", + SymbolRoles: 1, + }, + hoverText: []string{ + "```go\nfunc1()\n```", + "it does the thing", + }, + }, + { + explanation: "#2: SignatureDocumentation is present", + document: &scip.Document{ + Symbols: []*scip.SymbolInformation{ + { + Symbol: "react 17.1 main.go func1", + SignatureDocumentation: &scip.Document{ + Language: "go", + Text: "func1()", + }, + Documentation: []string{ + "it does the thing", + }, + }, + }, + }, + occurrence: &scip.Occurrence{ + Symbol: "react 17.1 main.go func1", + SymbolRoles: 1, + }, + hoverText: []string{ + "```go\nfunc1()\n```", + "it does the thing", + }, + }, + } + + for _, testCase := range testCases { + if diff := cmp.Diff(testCase.hoverText, extractOccurrenceData(testCase.document, testCase.occurrence).hoverText); diff != "" { + t.Errorf("unexpected documentation (-want +got):\n%s -- %s", diff, testCase.explanation) + } + } + }) + t.Run("implementations", func(t *testing.T) { testCases := []struct { explanation string From 2a66753bf81f5e86d6e3daf494476256aa18f82e Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Wed, 29 May 2024 16:24:00 +0100 Subject: [PATCH 2/4] Use new hover text logic in metadata_by_position --- .../codenav/internal/lsifstore/metadata_by_position.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/codeintel/codenav/internal/lsifstore/metadata_by_position.go b/internal/codeintel/codenav/internal/lsifstore/metadata_by_position.go index 738226b63058..43a5d1e4eb20 100644 --- a/internal/codeintel/codenav/internal/lsifstore/metadata_by_position.go +++ b/internal/codeintel/codenav/internal/lsifstore/metadata_by_position.go @@ -94,7 +94,7 @@ func (s *store) GetHover(ctx context.Context, bundleID int, path string, line, c } // Return first match - return strings.Join(symbol.Documentation, "\n"), rangeBySymbol[symbolName], true, nil + return strings.Join(symbolHoverText(symbol), "\n"), rangeBySymbol[symbolName], true, nil } } } From 68340f7236cbb0ad6d6982cb12f76bd38869093a Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Thu, 30 May 2024 08:36:30 +0100 Subject: [PATCH 3/4] Don't render signature if Text/Language are empty --- .../lsifstore/locations_by_position.go | 9 ++++++-- .../lsifstore/locations_by_position_test.go | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go index 1450aa448527..b42c95c00d4f 100644 --- a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go +++ b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go @@ -368,8 +368,13 @@ func monikersToString(vs []precise.MonikerData) string { func symbolHoverText(symbol *scip.SymbolInformation) []string { if symbol.SignatureDocumentation != nil { - signature := []string{fmt.Sprintf("```%s\n%s\n```", symbol.SignatureDocumentation.Language, symbol.SignatureDocumentation.Text)} - return append(signature, symbol.Documentation...) + signature := symbol.SignatureDocumentation + if signature.Text != "" && signature.Language != "" { + signature := []string{fmt.Sprintf("```%s\n%s\n```", symbol.SignatureDocumentation.Language, symbol.SignatureDocumentation.Text)} + return append(signature, symbol.Documentation...) + } else { + return symbol.Documentation + } } else { return symbol.Documentation } diff --git a/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go b/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go index 72cbfa17f4b3..6e74bc346216 100644 --- a/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go +++ b/internal/codeintel/codenav/internal/lsifstore/locations_by_position_test.go @@ -610,6 +610,27 @@ func TestExtractOccurrenceData(t *testing.T) { "it does the thing", }, }, + { + explanation: "#3: SignatureDocumentation is present, but Text/Language are empty", + document: &scip.Document{ + Symbols: []*scip.SymbolInformation{ + { + Symbol: "react 17.1 main.go func1", + SignatureDocumentation: &scip.Document{}, + Documentation: []string{ + "it does the thing", + }, + }, + }, + }, + occurrence: &scip.Occurrence{ + Symbol: "react 17.1 main.go func1", + SymbolRoles: 1, + }, + hoverText: []string{ + "it does the thing", + }, + }, } for _, testCase := range testCases { From 6f9d5c9dc3b9205d98c0e5eede2aa8549a833547 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Thu, 30 May 2024 16:48:12 +0800 Subject: [PATCH 4/4] Simplify if condition --- .../internal/lsifstore/locations_by_position.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go index b42c95c00d4f..7d67e80787ce 100644 --- a/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go +++ b/internal/codeintel/codenav/internal/lsifstore/locations_by_position.go @@ -367,18 +367,11 @@ func monikersToString(vs []precise.MonikerData) string { } func symbolHoverText(symbol *scip.SymbolInformation) []string { - if symbol.SignatureDocumentation != nil { - signature := symbol.SignatureDocumentation - if signature.Text != "" && signature.Language != "" { - signature := []string{fmt.Sprintf("```%s\n%s\n```", symbol.SignatureDocumentation.Language, symbol.SignatureDocumentation.Text)} - return append(signature, symbol.Documentation...) - } else { - return symbol.Documentation - } - } else { - return symbol.Documentation + if sigdoc := symbol.SignatureDocumentation; sigdoc != nil && sigdoc.Text != "" && sigdoc.Language != "" { + signature := []string{fmt.Sprintf("```%s\n%s\n```", sigdoc.Language, sigdoc.Text)} + return append(signature, symbol.Documentation...) } - + return symbol.Documentation } func (s *store) ExtractDefinitionLocationsFromPosition(ctx context.Context, locationKey LocationKey) (_ []shared.Location, _ []string, err error) {