Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit f1d5d77

Browse files
author
Christoph Hegemann
authored
refactor: handles Cursor uniformly over all usage provenances (#64319)
This change should not change any behaviour, but it prepares for syntactic/search-based pagination. Both syntactic and search based usages now return a "NextCursor", which is `Some` if there are more results of their provenance to be had. For now they always return `None` which preserves the current behaviour. The logic in `root_resolver` implements a state machine that transitions the cursor through the provenances and makes sure to skip provenances if the cursor in the request starts a certain state. ## Test plan Tests continue to pass, no behavioral changes
1 parent 3680503 commit f1d5d77

File tree

7 files changed

+225
-154
lines changed

7 files changed

+225
-154
lines changed

internal/codeintel/codenav/service.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,9 @@ func (s SyntacticMatch) GetSurroundingContent() string {
11401140
}
11411141

11421142
type SyntacticUsagesResult struct {
1143-
Matches []SyntacticMatch
1143+
Matches []SyntacticMatch
1144+
PreviousSyntacticSearch PreviousSyntacticSearch
1145+
NextCursor core.Option[UsagesCursor]
11441146
}
11451147

11461148
type UsagesForSymbolArgs struct {
@@ -1154,7 +1156,7 @@ func (s *Service) SyntacticUsages(
11541156
ctx context.Context,
11551157
gitTreeTranslator GitTreeTranslator,
11561158
args UsagesForSymbolArgs,
1157-
) (SyntacticUsagesResult, PreviousSyntacticSearch, *SyntacticUsagesError) {
1159+
) (SyntacticUsagesResult, *SyntacticUsagesError) {
11581160
// The `nil` in the second argument is here, because `With` does not work with custom error types.
11591161
ctx, trace, endObservation := s.operations.syntacticUsages.With(ctx, nil, observation.Args{Attrs: []attribute.KeyValue{
11601162
attribute.Int("repoId", int(args.Repo.ID)),
@@ -1166,7 +1168,7 @@ func (s *Service) SyntacticUsages(
11661168

11671169
upload, err := s.getSyntacticUpload(ctx, trace, args)
11681170
if err != nil {
1169-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, err
1171+
return SyntacticUsagesResult{}, err
11701172
}
11711173
index := NewMappedIndexFromTranslator(s.lsifstore, gitTreeTranslator, upload, args.Commit)
11721174
return syntacticUsagesImpl(ctx, trace, s.searchClient, index, args)
@@ -1216,12 +1218,17 @@ func languageFromFilepath(trace observation.TraceLogger, path core.RepoRelPath)
12161218
return langs[0], nil
12171219
}
12181220

1221+
type SearchBasedUsagesResult struct {
1222+
Matches []SearchBasedMatch
1223+
NextCursor core.Option[UsagesCursor]
1224+
}
1225+
12191226
func (s *Service) SearchBasedUsages(
12201227
ctx context.Context,
12211228
gitTreeTranslator GitTreeTranslator,
12221229
args UsagesForSymbolArgs,
12231230
previousSyntacticSearch core.Option[PreviousSyntacticSearch],
1224-
) (matches []SearchBasedMatch, err error) {
1231+
) (_ SearchBasedUsagesResult, err error) {
12251232
ctx, trace, endObservation := s.operations.searchBasedUsages.With(ctx, &err, observation.Args{Attrs: []attribute.KeyValue{
12261233
attribute.Int("repoId", int(args.Repo.ID)),
12271234
attribute.String("commit", string(args.Commit)),
@@ -1241,12 +1248,12 @@ func (s *Service) SearchBasedUsages(
12411248
} else {
12421249
language, err = languageFromFilepath(trace, args.Path)
12431250
if err != nil {
1244-
return nil, err
1251+
return SearchBasedUsagesResult{}, err
12451252
}
12461253

12471254
nameFromGit, err := s.symbolNameFromGit(ctx, args)
12481255
if err != nil {
1249-
return nil, err
1256+
return SearchBasedUsagesResult{}, err
12501257
}
12511258
symbolName = nameFromGit
12521259

internal/codeintel/codenav/service_syntactic_usages_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ func TestSearchBasedUsages_ResultWithoutSymbols(t *testing.T) {
2222
WithFile("path.java", ChunkMatchWithLine(refRange, refRangeLineContent), ChunkMatch(refRange2)).
2323
Build()
2424

25-
usages, err := searchBasedUsagesImpl(
25+
result, err := searchBasedUsagesImpl(
2626
context.Background(), observation.TestTraceLogger(log.NoOp()), mockSearchClient,
2727
UsagesForSymbolArgs{}, "symbol", "Java", core.None[MappedIndex](),
2828
)
2929
require.NoError(t, err)
30-
expectRanges(t, usages, refRange, refRange2)
31-
expectContent(t, usages, refRange, refRangeLineContent)
30+
expectRanges(t, result.Matches, refRange, refRange2)
31+
expectContent(t, result.Matches, refRange, refRangeLineContent)
3232
}
3333

3434
func TestSearchBasedUsages_ResultWithSymbol(t *testing.T) {
@@ -41,13 +41,13 @@ func TestSearchBasedUsages_ResultWithSymbol(t *testing.T) {
4141
WithSymbols("path.java", defRange).
4242
Build()
4343

44-
usages, err := searchBasedUsagesImpl(
44+
result, err := searchBasedUsagesImpl(
4545
context.Background(), observation.TestTraceLogger(log.NoOp()), mockSearchClient,
4646
UsagesForSymbolArgs{}, "symbol", "Java", core.None[MappedIndex](),
4747
)
4848
require.NoError(t, err)
49-
expectRanges(t, usages, refRange, refRange2, defRange)
50-
expectDefinitionRanges(t, usages, defRange)
49+
expectRanges(t, result.Matches, refRange, refRange2, defRange)
50+
expectDefinitionRanges(t, result.Matches, defRange)
5151
}
5252

5353
func TestSearchBasedUsages_SyntacticMatchesGetRemovedFromSearchBasedResults(t *testing.T) {
@@ -61,12 +61,12 @@ func TestSearchBasedUsages_SyntacticMatchesGetRemovedFromSearchBasedResults(t *t
6161
upload, lsifStore := setupUpload(commit, "", doc("path.java", ref("ref", syntacticRange)))
6262
fakeMappedIndex := NewMappedIndexFromTranslator(lsifStore, noopTranslator(), upload, commit)
6363

64-
usages, err := searchBasedUsagesImpl(
64+
result, err := searchBasedUsagesImpl(
6565
context.Background(), observation.TestTraceLogger(log.NoOp()), mockSearchClient,
6666
UsagesForSymbolArgs{}, "symbol", "Java", core.Some(fakeMappedIndex),
6767
)
6868
require.NoError(t, err)
69-
expectRanges(t, usages, commentRange)
69+
expectRanges(t, result.Matches, commentRange)
7070
}
7171

7272
func TestSyntacticUsages(t *testing.T) {
@@ -91,7 +91,7 @@ func TestSyntacticUsages(t *testing.T) {
9191
ref("initial", initialRange)))
9292
fakeMappedIndex := NewMappedIndexFromTranslator(lsifStore, noopTranslator(), upload, commit)
9393

94-
syntacticUsages, _, err := syntacticUsagesImpl(
94+
syntacticUsages, err := syntacticUsagesImpl(
9595
context.Background(), observation.TestTraceLogger(log.NoOp()),
9696
mockSearchClient, fakeMappedIndex, UsagesForSymbolArgs{
9797
Commit: commit,
@@ -119,7 +119,7 @@ func TestSyntacticUsages_DocumentNotInIndex(t *testing.T) {
119119
doc("initial.java",
120120
ref("initial", initialRange)))
121121
fakeMappedIndex := NewMappedIndexFromTranslator(lsifStore, noopTranslator(), upload, commit)
122-
syntacticUsages, _, err := syntacticUsagesImpl(
122+
syntacticUsages, err := syntacticUsagesImpl(
123123
context.Background(), observation.TestTraceLogger(log.NoOp()),
124124
mockSearchClient, fakeMappedIndex, UsagesForSymbolArgs{
125125
Commit: commit,
@@ -158,7 +158,7 @@ func TestSyntacticUsages_IndexCommitTranslated(t *testing.T) {
158158
return r.CompareStrict(editedRange) == 0
159159
}), upload, targetCommit)
160160

161-
syntacticUsages, _, err := syntacticUsagesImpl(
161+
syntacticUsages, err := syntacticUsagesImpl(
162162
context.Background(), observation.TestTraceLogger(log.NoOp()),
163163
mockSearchClient, fakeMappedIndex, UsagesForSymbolArgs{
164164
Commit: targetCommit,

internal/codeintel/codenav/syntactic.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -371,24 +371,24 @@ func syntacticUsagesImpl(
371371
searchClient searchclient.SearchClient,
372372
mappedIndex MappedIndex,
373373
args UsagesForSymbolArgs,
374-
) (SyntacticUsagesResult, PreviousSyntacticSearch, *SyntacticUsagesError) {
374+
) (SyntacticUsagesResult, *SyntacticUsagesError) {
375375
searchSymbol, symErr := symbolAtRange(ctx, mappedIndex, args)
376376
if symErr != nil {
377-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, &SyntacticUsagesError{
377+
return SyntacticUsagesResult{}, &SyntacticUsagesError{
378378
Code: SU_NoSymbolAtRequestedRange,
379379
UnderlyingError: symErr,
380380
}
381381
}
382382
language, langErr := languageFromFilepath(trace, args.Path)
383383
if langErr != nil {
384-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, &SyntacticUsagesError{
384+
return SyntacticUsagesResult{}, &SyntacticUsagesError{
385385
Code: SU_FailedToSearch,
386386
UnderlyingError: langErr,
387387
}
388388
}
389389
symbolName, ok := nameFromGlobalSymbol(searchSymbol)
390390
if !ok {
391-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, &SyntacticUsagesError{
391+
return SyntacticUsagesResult{}, &SyntacticUsagesError{
392392
Code: SU_FailedToSearch,
393393
UnderlyingError: errors.New("can't find syntactic occurrences for locals via search"),
394394
}
@@ -401,7 +401,7 @@ func syntacticUsagesImpl(
401401
}
402402
candidateMatches, searchErr := findCandidateOccurrencesViaSearch(ctx, trace, searchClient, searchCoords)
403403
if searchErr != nil {
404-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, &SyntacticUsagesError{
404+
return SyntacticUsagesResult{}, &SyntacticUsagesError{
405405
Code: SU_FailedToSearch,
406406
UnderlyingError: searchErr,
407407
}
@@ -426,16 +426,20 @@ func syntacticUsagesImpl(
426426
return slices.Concat(results...), nil
427427
})
428428
if err != nil {
429-
return SyntacticUsagesResult{}, PreviousSyntacticSearch{}, &SyntacticUsagesError{
429+
return SyntacticUsagesResult{}, &SyntacticUsagesError{
430430
Code: SU_Fatal,
431431
UnderlyingError: err,
432432
}
433433
}
434434

435-
return SyntacticUsagesResult{Matches: slices.Concat(results...)}, PreviousSyntacticSearch{
436-
MappedIndex: mappedIndex,
437-
SymbolName: symbolName,
438-
Language: language,
435+
return SyntacticUsagesResult{
436+
Matches: slices.Concat(results...),
437+
PreviousSyntacticSearch: PreviousSyntacticSearch{
438+
MappedIndex: mappedIndex,
439+
SymbolName: symbolName,
440+
Language: language,
441+
},
442+
NextCursor: core.None[UsagesCursor](),
439443
}, nil
440444
}
441445

@@ -449,7 +453,7 @@ func searchBasedUsagesImpl(
449453
symbolName string,
450454
language string,
451455
syntacticIndex core.Option[MappedIndex],
452-
) (matches []SearchBasedMatch, err error) {
456+
) (_ SearchBasedUsagesResult, err error) {
453457
searchCoords := searchArgs{
454458
repo: args.Repo.Name,
455459
commit: args.Commit,
@@ -474,7 +478,7 @@ func searchBasedUsagesImpl(
474478
})
475479
wg.Wait()
476480
if matchResults.err != nil {
477-
return nil, matchResults.err
481+
return SearchBasedUsagesResult{}, matchResults.err
478482
}
479483
if symbolResults.err != nil {
480484
trace.Warn("Failed to run symbol search, will not mark any search-based usages as definitions", log.Error(symbolResults.err))
@@ -513,5 +517,8 @@ func searchBasedUsagesImpl(
513517
}
514518
return slices.Concat(results...)
515519
})
516-
return slices.Concat(results...), nil
520+
return SearchBasedUsagesResult{
521+
Matches: slices.Concat(results...),
522+
NextCursor: core.None[UsagesCursor](),
523+
}, nil
517524
}

internal/codeintel/codenav/transport/graphql/iface.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ type CodeNavService interface {
3131
//
3232
// Subsequent calls can pass the returned cursor (if non-empty) via args.Cursor.
3333
PreciseUsages(ctx context.Context, requestState codenav.RequestState, args codenav.UsagesForSymbolResolvedArgs) (_ []shared.UploadUsage, nextCursor core.Option[codenav.UsagesCursor], err error)
34-
SyntacticUsages(context.Context, codenav.GitTreeTranslator, codenav.UsagesForSymbolArgs) (codenav.SyntacticUsagesResult, codenav.PreviousSyntacticSearch, *codenav.SyntacticUsagesError)
35-
SearchBasedUsages(context.Context, codenav.GitTreeTranslator, codenav.UsagesForSymbolArgs, core.Option[codenav.PreviousSyntacticSearch]) ([]codenav.SearchBasedMatch, error)
34+
SyntacticUsages(context.Context, codenav.GitTreeTranslator, codenav.UsagesForSymbolArgs) (codenav.SyntacticUsagesResult, *codenav.SyntacticUsagesError)
35+
SearchBasedUsages(context.Context, codenav.GitTreeTranslator, codenav.UsagesForSymbolArgs, core.Option[codenav.PreviousSyntacticSearch]) (codenav.SearchBasedUsagesResult, error)
3636
}
3737

3838
var _ CodeNavService = &codenav.Service{}

0 commit comments

Comments
 (0)