Fixes to session's handling of empty results #17728
Conversation
ghost
requested a review
from 
src/server/session.ts
Outdated
| @@ -809,7 +809,7 @@ namespace ts.server { | |||
| // The rename info should be the same for every project | |||
| const renameInfo = defaultProject.getLanguageService().getRenameInfo(file, position); | |||
| if (!renameInfo) { | |||
| return undefined; | |||
| return emptyArray; | |||
There was a problem hiding this comment.
Are we confident that callers expecting a protocol.RenameResponseBody won't hit this case?
There was a problem hiding this comment.
D'oh, got the meaning of simplifiedResult backwards.
| @@ -714,7 +714,7 @@ namespace ts.server { | |||
| const documentHighlights = project.getLanguageService().getDocumentHighlights(file, position, args.filesToSearch); | |||
|
|
|||
| if (!documentHighlights) { | |||
| return undefined; | |||
| return emptyArray; | |||
| @@ -624,7 +624,7 @@ namespace ts.server { | |||
|
|
|||
| const definitions = project.getLanguageService().getTypeDefinitionAtPosition(file, position); | |||
| if (!definitions) { | |||
| return undefined; | |||
| return emptyArray; | |||
| @@ -901,7 +901,7 @@ namespace ts.server { | |||
| } | |||
| } | |||
|
|
|||
| private getReferences(args: protocol.FileLocationRequestArgs, simplifiedResult: boolean): protocol.ReferencesResponseBody | ReadonlyArray<ReferencedSymbol> { | |||
| private getReferences(args: protocol.FileLocationRequestArgs, simplifiedResult: boolean): protocol.ReferencesResponseBody | undefined | ReadonlyArray<ReferencedSymbol> { | |||
There was a problem hiding this comment.
So we are okay with returning undefined on success?
There was a problem hiding this comment.
We don't really have a way to return a ReferencesResponseBody if the cursor wasn't at a valid location. It would be nice if we had some way to indicate a non-result in a way that's distinguishable from a crash in services.
| if (simplifiedResult) { | ||
| return mapDefined(completions.entries, entry => { | ||
| return mapDefined(completions && completions.entries, entry => { |
There was a problem hiding this comment.
I would have thought we'd want to return emptyArray when !completions.
There was a problem hiding this comment.
mapDefined always returns a defined result.
|
Assuming we're okay with returning |
Ref: #17165 (review)