Back to all reviewers

Consider async function contracts

kilo-org/kilocode
Based on 2 comments
TypeScript

Be intentional when marking functions as async, as it changes the function's contract and caller expectations. Avoid using async unnecessarily when you're just returning a promise explicitly, and consider whether making a function async implies that callers should await its completion.

Concurrency TypeScript

Reviewer Prompt

Be intentional when marking functions as async, as it changes the function’s contract and caller expectations. Avoid using async unnecessarily when you’re just returning a promise explicitly, and consider whether making a function async implies that callers should await its completion.

For example, avoid this pattern where async adds no value:

// Unnecessary async - just returning a promise
private static async requestLLMFix(code: string, error: string): Promise<string | null> {
    return somePromiseReturningFunction(code, error)
}

Also consider the broader implications when converting functions to async:

// Before: synchronous activation
export function activate(context: vscode.ExtensionContext) {
    // setup code
    Promise.resolve(vscode.commands.executeCommand("command"))
}

// After: now returns Promise, implying callers should await
export async function activate(context: vscode.ExtensionContext) {
    // setup code  
    await vscode.commands.executeCommand("command")
}

The async version changes the contract - callers now expect to await the activation, which may not be the intended behavior for lifecycle functions.

2
Comments Analyzed
TypeScript
Primary Language
Concurrency
Category

Source Discussions