Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/adapters/code-action-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default class CodeActionAdapter {

const params = CodeActionAdapter.createCodeActionParams(linterAdapter, editor, range, diagnostics)
const actions = await connection.codeAction(params)
return actions.map((action) => CodeActionAdapter.createCodeAction(action, connection))
return (actions || []).map((action) => CodeActionAdapter.createCodeAction(action, connection))
Copy link
Member

@UziTech UziTech Apr 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to codeAction it returns Promise<Array<lsp.Command | lsp.CodeAction>> so a promise resolving to null or undefined is an invalid return type and should probably throw an error (as it currently does) instead of being handled the same as an empty array. Or the return type should be updated to include null and undefined as valid returns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to prefer throwing a valid error instead of failing silently like this. @aminya This is what I was talking about in #132 (comment) when I said "types do not catch runtime errors".

Copy link
Contributor Author

@ayame113 ayame113 Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review!

The spec seems to say that codeAction returns (Command | CodeAction) [] | null. Is it not necessary to raise an error if the language server returns valid null?

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_codeAction
Request:

  • method: ‘textDocument/codeAction’

Response:

  • result: (Command | CodeAction)[] | null

By the way, such a change is also an option.

 public static async getCodeActions(
   ...
-): Promise<atomIde.CodeAction[]> {
+): Promise<atomIde.CodeAction[] | null> {
   if (linterAdapter == null) {
     return []
   }
   assert(serverCapabilities.codeActionProvider, "Must have the textDocument/codeAction capability")
   const params = CodeActionAdapter.createCodeActionParams(linterAdapter, editor, range, diagnostics)
   const actions = await connection.codeAction(params)
-  return (actions || []).map((action) => CodeActionAdapter.createCodeAction(action, connection))
+  if (!actions) {return null}
+  return actions.map((action) => CodeActionAdapter.createCodeAction(action, connection))
 }

Copy link
Member

@UziTech UziTech Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the early null check rather than still mapping over an empty array.

We should change the return type of codeAction if null is a valid response according to the spec.

NVM I see you already did that. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At e7c26b4, I changed it to return null instead of an empty array when actions value is null.

}

private static createCodeAction(
Expand Down
3 changes: 3 additions & 0 deletions lib/adapters/code-highlight-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export default class CodeHighlightAdapter {
): Promise<Range[] | null> {
assert(serverCapabilities.documentHighlightProvider, "Must have the documentHighlight capability")
const highlights = await connection.documentHighlight(Convert.editorToTextDocumentPositionParams(editor, position))
if (!highlights) {
return null
}
return highlights.map((highlight) => {
return Convert.lsRangeToAtomRange(highlight.range)
})
Expand Down
2 changes: 1 addition & 1 deletion lib/adapters/outline-view-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default class OutlineViewAdapter {
connection.documentSymbol({ textDocument: Convert.editorToTextDocumentIdentifier(editor) }, cancellationToken)
)

if (results.length === 0) {
if (!results || results.length === 0) {
return {
outlineTrees: [],
}
Expand Down
2 changes: 1 addition & 1 deletion lib/auto-languageclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ export default class AutoLanguageClient {
* `didChangeWatchedFiles` message filtering, override for custom logic.
*
* @param filePath Path of a file that has changed in the project path
* @returns `false` => message will not be sent to the language server
* @returns `false` => message will not be sent to the language server
*/
protected filterChangeWatchedFiles(_filePath: string): boolean {
return true
Expand Down
111 changes: 58 additions & 53 deletions lib/languageclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,22 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing the {InitializeResult} with details of the server's capabilities.
*/
public initialize(params: lsp.InitializeParams): Promise<lsp.InitializeResult> {
return this._sendRequest("initialize", params)
return this._sendRequest(lsp.InitializeRequest.type, params)
}

/** Public: Send an `initialized` notification to the language server. */
public initialized(): void {
this._sendNotification("initialized", {})
this._sendNotification(lsp.InitializedNotification.type, {})
}

/** Public: Send a `shutdown` request to the language server. */
public shutdown(): Promise<void> {
return this._sendRequest("shutdown")
return this._sendRequest(lsp.ShutdownRequest.type)
}

/** Public: Send an `exit` notification to the language server. */
public exit(): void {
this._sendNotification("exit")
this._sendNotification(lsp.ExitNotification.type)
}

/**
Expand Down Expand Up @@ -126,8 +126,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param method A string containing the name of the request message.
* @param params The method's parameters
*/
public sendCustomRequest(method: string, params?: any[] | object): Promise<any | null> {
return this._sendRequest(method, params)
public sendCustomRequest(method: string, params?: any[] | object): Promise<any> {
return this._sendRequest(new lsp.ProtocolRequestType<typeof params, any, any, void, any>(method), params)
}

/**
Expand All @@ -137,7 +137,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The method's parameters
*/
public sendCustomNotification(method: string, params?: any[] | object): void {
this._sendNotification(method, params)
this._sendNotification(new lsp.ProtocolNotificationType<typeof params, any>(method), params)
}

/**
Expand Down Expand Up @@ -212,7 +212,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {DidChangeConfigurationParams} containing the new configuration.
*/
public didChangeConfiguration(params: lsp.DidChangeConfigurationParams): void {
this._sendNotification("workspace/didChangeConfiguration", params)
this._sendNotification(lsp.DidChangeConfigurationNotification.type, params)
}

/**
Expand All @@ -221,7 +221,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {DidOpenTextDocumentParams} containing the opened text document details.
*/
public didOpenTextDocument(params: lsp.DidOpenTextDocumentParams): void {
this._sendNotification("textDocument/didOpen", params)
this._sendNotification(lsp.DidOpenTextDocumentNotification.type, params)
}

/**
Expand All @@ -231,7 +231,7 @@ export class LanguageClientConnection extends EventEmitter {
* number and actual text changes.
*/
public didChangeTextDocument(params: lsp.DidChangeTextDocumentParams): void {
this._sendNotification("textDocument/didChange", params)
this._sendNotification(lsp.DidChangeTextDocumentNotification.type, params)
}

/**
Expand All @@ -240,7 +240,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {DidCloseTextDocumentParams} containing the opened text document details.
*/
public didCloseTextDocument(params: lsp.DidCloseTextDocumentParams): void {
this._sendNotification("textDocument/didClose", params)
this._sendNotification(lsp.DidCloseTextDocumentNotification.type, params)
}

/**
Expand All @@ -249,7 +249,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {WillSaveTextDocumentParams} containing the to-be-saved text document details and the reason for the save.
*/
public willSaveTextDocument(params: lsp.WillSaveTextDocumentParams): void {
this._sendNotification("textDocument/willSave", params)
this._sendNotification(lsp.WillSaveTextDocumentNotification.type, params)
}

/**
Expand All @@ -259,7 +259,7 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing an {Array} of {TextEdit}s to be applied to the text document before it is saved.
*/
public willSaveWaitUntilTextDocument(params: lsp.WillSaveTextDocumentParams): Promise<lsp.TextEdit[] | null> {
return this._sendRequest("textDocument/willSaveWaitUntil", params)
return this._sendRequest(lsp.WillSaveTextDocumentWaitUntilRequest.type, params)
}

/**
Expand All @@ -268,7 +268,7 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {DidSaveTextDocumentParams} containing the saved text document details.
*/
public didSaveTextDocument(params: lsp.DidSaveTextDocumentParams): void {
this._sendNotification("textDocument/didSave", params)
this._sendNotification(lsp.DidSaveTextDocumentNotification.type, params)
}

/**
Expand All @@ -278,7 +278,7 @@ export class LanguageClientConnection extends EventEmitter {
* the watched files.
*/
public didChangeWatchedFiles(params: lsp.DidChangeWatchedFilesParams): void {
this._sendNotification("workspace/didChangeWatchedFiles", params)
this._sendNotification(lsp.DidChangeWatchedFilesNotification.type, params)
}

/**
Expand All @@ -303,7 +303,7 @@ export class LanguageClientConnection extends EventEmitter {
cancellationToken?: jsonrpc.CancellationToken
): Promise<lsp.CompletionItem[] | lsp.CompletionList | null> {
// Cancel prior request if necessary
return this._sendRequest("textDocument/completion", params, cancellationToken)
return this._sendRequest(lsp.CompletionRequest.type, params, cancellationToken)
}

/**
Expand All @@ -312,8 +312,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {CompletionItem} for which a fully resolved {CompletionItem} is desired.
* @returns A {Promise} containing a fully resolved {CompletionItem}.
*/
public completionItemResolve(params: lsp.CompletionItem): Promise<lsp.CompletionItem | null> {
return this._sendRequest("completionItem/resolve", params)
public completionItemResolve(params: lsp.CompletionItem): Promise<lsp.CompletionItem> {
return this._sendRequest(lsp.CompletionResolveRequest.type, params)
}

/**
Expand All @@ -323,7 +323,7 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing a {Hover}.
*/
public hover(params: lsp.TextDocumentPositionParams): Promise<lsp.Hover | null> {
return this._sendRequest("textDocument/hover", params)
return this._sendRequest(lsp.HoverRequest.type, params)
}

/**
Expand All @@ -333,7 +333,7 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing a {SignatureHelp}.
*/
public signatureHelp(params: lsp.TextDocumentPositionParams): Promise<lsp.SignatureHelp | null> {
return this._sendRequest("textDocument/signatureHelp", params)
return this._sendRequest(lsp.SignatureHelpRequest.type, params)
}

/**
Expand All @@ -346,7 +346,7 @@ export class LanguageClientConnection extends EventEmitter {
public gotoDefinition(
params: lsp.TextDocumentPositionParams
): Promise<lsp.Location | lsp.Location[] | lsp.LocationLink[] | null> {
return this._sendRequest("textDocument/definition", params)
return this._sendRequest(lsp.DefinitionRequest.type, params)
}

/**
Expand All @@ -355,8 +355,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {TextDocumentPositionParams} of a symbol for which all referring {Location}s are desired.
* @returns A {Promise} containing an {Array} of {Location}s that reference this symbol.
*/
public findReferences(params: lsp.ReferenceParams): Promise<lsp.Location[]> {
return this._sendRequest("textDocument/references", params)
public findReferences(params: lsp.ReferenceParams): Promise<lsp.Location[] | null> {
return this._sendRequest(lsp.ReferencesRequest.type, params)
}

/**
Expand All @@ -365,8 +365,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {TextDocumentPositionParams} of a symbol for which all highlights are desired.
* @returns A {Promise} containing an {Array} of {DocumentHighlight}s that can be used to highlight this symbol.
*/
public documentHighlight(params: lsp.TextDocumentPositionParams): Promise<lsp.DocumentHighlight[]> {
return this._sendRequest("textDocument/documentHighlight", params)
public documentHighlight(params: lsp.TextDocumentPositionParams): Promise<lsp.DocumentHighlight[] | null> {
return this._sendRequest(lsp.DocumentHighlightRequest.type, params)
}

/**
Expand All @@ -379,8 +379,8 @@ export class LanguageClientConnection extends EventEmitter {
public documentSymbol(
params: lsp.DocumentSymbolParams,
_cancellationToken?: jsonrpc.CancellationToken
): Promise<lsp.SymbolInformation[] | lsp.DocumentSymbol[]> {
return this._sendRequest("textDocument/documentSymbol", params)
): Promise<lsp.SymbolInformation[] | lsp.DocumentSymbol[] | null> {
return this._sendRequest(lsp.DocumentSymbolRequest.type, params)
}

/**
Expand All @@ -390,8 +390,8 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing an {Array} of {SymbolInformation}s that identify where the query string occurs
* within the workspace.
*/
public workspaceSymbol(params: lsp.WorkspaceSymbolParams): Promise<lsp.SymbolInformation[]> {
return this._sendRequest("workspace/symbol", params)
public workspaceSymbol(params: lsp.WorkspaceSymbolParams): Promise<lsp.SymbolInformation[] | null> {
return this._sendRequest(lsp.WorkspaceSymbolRequest.type, params)
}

/**
Expand All @@ -400,8 +400,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {CodeActionParams} identifying the document, range and context for the code action.
* @returns A {Promise} containing an {Array} of {Commands}s that can be performed against the given documents range.
*/
public codeAction(params: lsp.CodeActionParams): Promise<Array<lsp.Command | lsp.CodeAction>> {
return this._sendRequest("textDocument/codeAction", params)
public codeAction(params: lsp.CodeActionParams): Promise<Array<lsp.Command | lsp.CodeAction> | null> {
return this._sendRequest(lsp.CodeActionRequest.type, params)
}

/**
Expand All @@ -411,8 +411,8 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing an {Array} of {CodeLens}s that associate commands and data with specified ranges
* within the document.
*/
public codeLens(params: lsp.CodeLensParams): Promise<lsp.CodeLens[]> {
return this._sendRequest("textDocument/codeLens", params)
public codeLens(params: lsp.CodeLensParams): Promise<lsp.CodeLens[] | null> {
return this._sendRequest(lsp.CodeLensRequest.type, params)
}

/**
Expand All @@ -421,8 +421,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {CodeLens} identifying the code lens to be resolved with full detail.
* @returns A {Promise} containing the {CodeLens} fully resolved.
*/
public codeLensResolve(params: lsp.CodeLens): Promise<lsp.CodeLens | null> {
return this._sendRequest("codeLens/resolve", params)
public codeLensResolve(params: lsp.CodeLens): Promise<lsp.CodeLens> {
return this._sendRequest(lsp.CodeLensResolveRequest.type, params)
}

/**
Expand All @@ -431,8 +431,8 @@ export class LanguageClientConnection extends EventEmitter {
* @param params The {DocumentLinkParams} identifying the document for which links should be identified.
* @returns A {Promise} containing an {Array} of {DocumentLink}s relating uri's to specific ranges within the document.
*/
public documentLink(params: lsp.DocumentLinkParams): Promise<lsp.DocumentLink[]> {
return this._sendRequest("textDocument/documentLink", params)
public documentLink(params: lsp.DocumentLinkParams): Promise<lsp.DocumentLink[] | null> {
return this._sendRequest(lsp.DocumentLinkRequest.type, params)
}

/**
Expand All @@ -442,7 +442,7 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing the {DocumentLink} fully resolved.
*/
public documentLinkResolve(params: lsp.DocumentLink): Promise<lsp.DocumentLink> {
return this._sendRequest("documentLink/resolve", params)
return this._sendRequest(lsp.DocumentLinkResolveRequest.type, params)
}

/**
Expand All @@ -452,8 +452,8 @@ export class LanguageClientConnection extends EventEmitter {
* formatting preferences.
* @returns A {Promise} containing an {Array} of {TextEdit}s to be applied to the document to correctly reformat it.
*/
public documentFormatting(params: lsp.DocumentFormattingParams): Promise<lsp.TextEdit[]> {
return this._sendRequest("textDocument/formatting", params)
public documentFormatting(params: lsp.DocumentFormattingParams): Promise<lsp.TextEdit[] | null> {
return this._sendRequest(lsp.DocumentFormattingRequest.type, params)
}

/**
Expand All @@ -463,8 +463,8 @@ export class LanguageClientConnection extends EventEmitter {
* additional formatting preferences.
* @returns A {Promise} containing an {Array} of {TextEdit}s to be applied to the document to correctly reformat it.
*/
public documentRangeFormatting(params: lsp.DocumentRangeFormattingParams): Promise<lsp.TextEdit[]> {
return this._sendRequest("textDocument/rangeFormatting", params)
public documentRangeFormatting(params: lsp.DocumentRangeFormattingParams): Promise<lsp.TextEdit[] | null> {
return this._sendRequest(lsp.DocumentRangeFormattingRequest.type, params)
}

/**
Expand All @@ -474,8 +474,8 @@ export class LanguageClientConnection extends EventEmitter {
* typed and at what position as well as additional formatting preferences.
* @returns A {Promise} containing an {Array} of {TextEdit}s to be applied to the document to correctly reformat it.
*/
public documentOnTypeFormatting(params: lsp.DocumentOnTypeFormattingParams): Promise<lsp.TextEdit[]> {
return this._sendRequest("textDocument/onTypeFormatting", params)
public documentOnTypeFormatting(params: lsp.DocumentOnTypeFormattingParams): Promise<lsp.TextEdit[] | null> {
return this._sendRequest(lsp.DocumentOnTypeFormattingRequest.type, params)
}

/**
Expand All @@ -486,8 +486,8 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing an {WorkspaceEdit} that contains a list of {TextEdit}s either on the changes
* property (keyed by uri) or the documentChanges property containing an {Array} of {TextDocumentEdit}s (preferred).
*/
public rename(params: lsp.RenameParams): Promise<lsp.WorkspaceEdit> {
return this._sendRequest("textDocument/rename", params)
public rename(params: lsp.RenameParams): Promise<lsp.WorkspaceEdit | null> {
return this._sendRequest(lsp.RenameRequest.type, params)
}

/**
Expand All @@ -498,7 +498,7 @@ export class LanguageClientConnection extends EventEmitter {
* @returns A {Promise} containing anything.
*/
public executeCommand(params: lsp.ExecuteCommandParams): Promise<any> {
return this._sendRequest("workspace/executeCommand", params)
return this._sendRequest(lsp.ExecuteCommandRequest.type, params)
}

private _onRequest<T extends Extract<keyof KnownRequests, string>>(
Expand All @@ -521,16 +521,21 @@ export class LanguageClientConnection extends EventEmitter {
})
}

private _sendNotification(method: string, args?: object): void {
private _sendNotification<P, RO>(
protocol: lsp.ProtocolNotificationType<P, RO> | lsp.ProtocolNotificationType0<RO>,
args?: P
): void {
const { method } = protocol
this._log.debug(`rpc.sendNotification ${method}`, args)
this._rpc.sendNotification(method, args)
}

private async _sendRequest(
method: string,
args?: object,
private async _sendRequest<P, R, PR, E, RO>(
protocol: lsp.ProtocolRequestType<P, R, PR, E, RO> | lsp.ProtocolRequestType0<R, PR, E, RO>,
args?: P,
cancellationToken?: jsonrpc.CancellationToken
): Promise<any> {
): Promise<R> {
const { method } = protocol
this._log.debug(`rpc.sendRequest ${method} sending`, args)
try {
const start = performance.now()
Expand All @@ -546,7 +551,7 @@ export class LanguageClientConnection extends EventEmitter {

const took = performance.now() - start
this._log.debug(`rpc.sendRequest ${method} received (${Math.floor(took)}ms)`, result)
return result
return result as R
} catch (e) {
const responseError = e as jsonrpc.ResponseError<any>
if (cancellationToken && responseError.code === lsp.LSPErrorCodes.RequestCancelled) {
Expand Down
Loading