Skip to content

Commit

Permalink
Delay execution of ShellCheck tasks
Browse files Browse the repository at this point in the history
Based on vscode ThrottledDelayer
  • Loading branch information
skovhus committed Jan 4, 2023
1 parent b55fab8 commit f6ef9ea
Show file tree
Hide file tree
Showing 2 changed files with 229 additions and 8 deletions.
47 changes: 39 additions & 8 deletions server/src/shellcheck/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { spawn } from 'child_process'
import * as LSP from 'vscode-languageserver/node'
import { TextDocument } from 'vscode-languageserver-textdocument'

import { ThrottledDelayer } from '../util/async'
import { analyzeShebang } from '../util/shebang'
import { CODE_TO_TAGS, LEVEL_TO_SEVERITY } from './config'
import {
Expand All @@ -15,24 +16,33 @@ import {
} from './types'

const SUPPORTED_BASH_DIALECTS = ['sh', 'bash', 'dash', 'ksh']

const DEBOUNCE_MS = 300
type LinterOptions = {
executablePath: string
console: LSP.RemoteConsole
cwd?: string
}

type LintingResult = {
diagnostics: LSP.Diagnostic[]
codeActions: LSP.CodeAction[]
}

export class Linter {
public executablePath: string
private cwd: string
private console: LSP.RemoteConsole
private cwd: string
public executablePath: string
private uriToDelayer: {
[uri: string]: ThrottledDelayer<LintingResult>
}
private _canLint: boolean

constructor({ console, cwd, executablePath }: LinterOptions) {
this.executablePath = executablePath
this.cwd = cwd || process.cwd()
this._canLint = true
this.console = console
this.cwd = cwd || process.cwd()
this.executablePath = executablePath
this.uriToDelayer = Object.create(null)
}

public get canLint(): boolean {
Expand All @@ -43,11 +53,28 @@ export class Linter {
document: TextDocument,
sourcePaths: string[],
additionalShellCheckArguments: string[] = [],
): Promise<{ diagnostics: LSP.Diagnostic[]; codeActions: LSP.CodeAction[] }> {
): Promise<LintingResult> {
if (!this._canLint) {
return { diagnostics: [], codeActions: [] }
}

const { uri } = document
let delayer = this.uriToDelayer[uri]
if (!delayer) {
delayer = new ThrottledDelayer<LintingResult>(DEBOUNCE_MS)
this.uriToDelayer[uri] = delayer
}

return delayer.trigger(() =>
this.executeLint(document, sourcePaths, additionalShellCheckArguments),
)
}

private async executeLint(
document: TextDocument,
sourcePaths: string[],
additionalShellCheckArguments: string[] = [],
): Promise<LintingResult> {
const result = await this.runShellCheck(
document,
[...sourcePaths, dirname(fileURLToPath(document.uri))],
Expand All @@ -57,6 +84,9 @@ export class Linter {
return { diagnostics: [], codeActions: [] }
}

// Clean up the debounced function
delete this.uriToDelayer[document.uri]

return mapShellCheckResult({ document, result })
}

Expand Down Expand Up @@ -100,15 +130,15 @@ export class Linter {
proc.stdout.on('data', (data) => (out += data))
proc.stderr.on('data', (data) => (err += data))
proc.stdin.on('error', () => {
// XXX: Ignore STDIN errors in case the process ends too quickly, before we try to
// NOTE: Ignore STDIN errors in case the process ends too quickly, before we try to
// write. If we write after the process ends without this, we get an uncatchable EPIPE.
// This is solved in Node >= 15.1 by the "on('spawn', ...)" event, but we need to
// support earlier versions.
})
proc.stdin.end(documentText)
})

// XXX: do we care about exit code? 0 means "ok", 1 possibly means "errors",
// NOTE: do we care about exit code? 0 means "ok", 1 possibly means "errors",
// but the presence of parseable errors in the output is also sufficient to
// distinguish.
let exit
Expand Down Expand Up @@ -141,6 +171,7 @@ export class Linter {
return ShellCheckResultSchema.parse(raw)
}
}

function mapShellCheckResult({
document,
result,
Expand Down
190 changes: 190 additions & 0 deletions server/src/util/async.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
* Origin: https://github.com/vscode-shellcheck/vscode-shellcheck/blob/master/src/utils/async.ts
*--------------------------------------------------------------------------------------------*/

export interface ITask<T> {
(): T
}

/**
* A helper to prevent accumulation of sequential async tasks.
*
* Imagine a mail man with the sole task of delivering letters. As soon as
* a letter submitted for delivery, he drives to the destination, delivers it
* and returns to his base. Imagine that during the trip, N more letters were submitted.
* When the mail man returns, he picks those N letters and delivers them all in a
* single trip. Even though N+1 submissions occurred, only 2 deliveries were made.
*
* The throttler implements this via the queue() method, by providing it a task
* factory. Following the example:
*
* var throttler = new Throttler();
* var letters = [];
*
* function letterReceived(l) {
* letters.push(l);
* throttler.queue(() => { return makeTheTrip(); });
* }
*/
export class Throttler<T> {
private activePromise: Promise<T> | null
private queuedPromise: Promise<T> | null
private queuedPromiseFactory: ITask<Promise<T>> | null

constructor() {
this.activePromise = null
this.queuedPromise = null
this.queuedPromiseFactory = null
}

public queue(promiseFactory: ITask<Promise<T>>): Promise<T> {
if (this.activePromise) {
this.queuedPromiseFactory = promiseFactory

if (!this.queuedPromise) {
const onComplete = () => {
this.queuedPromise = null

const result = this.queue(this.queuedPromiseFactory!)
this.queuedPromiseFactory = null

return result
}

this.queuedPromise = new Promise<T>((resolve) => {
this.activePromise!.then(onComplete, onComplete).then(resolve)
})
}

return new Promise<T>((resolve, reject) => {
this.queuedPromise!.then(resolve, reject)
})
}

this.activePromise = promiseFactory()

return new Promise<T>((resolve, reject) => {
this.activePromise!.then(
(result: T) => {
this.activePromise = null
resolve(result)
},
(err: any) => {
this.activePromise = null
reject(err)
},
)
})
}
}

/**
* A helper to delay execution of a task that is being requested often.
*
* Following the throttler, now imagine the mail man wants to optimize the number of
* trips proactively. The trip itself can be long, so the he decides not to make the trip
* as soon as a letter is submitted. Instead he waits a while, in case more
* letters are submitted. After said waiting period, if no letters were submitted, he
* decides to make the trip. Imagine that N more letters were submitted after the first
* one, all within a short period of time between each other. Even though N+1
* submissions occurred, only 1 delivery was made.
*
* The delayer offers this behavior via the trigger() method, into which both the task
* to be executed and the waiting period (delay) must be passed in as arguments. Following
* the example:
*
* var delayer = new Delayer(WAITING_PERIOD);
* var letters = [];
*
* function letterReceived(l) {
* letters.push(l);
* delayer.trigger(() => { return makeTheTrip(); });
* }
*/
export class Delayer<T> {
public defaultDelay: number
private timeout: NodeJS.Timer | null
private completionPromise: Promise<T> | null
private onResolve: ((value: T | PromiseLike<T> | undefined) => void) | null
private task: ITask<T> | null

constructor(defaultDelay: number) {
this.defaultDelay = defaultDelay
this.timeout = null
this.completionPromise = null
this.onResolve = null
this.task = null
}

public trigger(task: ITask<T>, delay: number = this.defaultDelay): Promise<T> {
this.task = task
this.cancelTimeout()

if (!this.completionPromise) {
this.completionPromise = new Promise<T | undefined>((resolve) => {
this.onResolve = resolve
}).then(() => {
this.completionPromise = null
this.onResolve = null

const result = this.task!()
this.task = null

return result
})
}

this.timeout = setTimeout(() => {
this.timeout = null
this.onResolve!(undefined)
}, delay)

return this.completionPromise
}

public isTriggered(): boolean {
return this.timeout !== null
}

public cancel(): void {
this.cancelTimeout()

if (this.completionPromise) {
this.completionPromise = null
}
}

private cancelTimeout(): void {
if (this.timeout !== null) {
clearTimeout(this.timeout)
this.timeout = null
}
}
}

/**
* A helper to delay execution of a task that is being requested often, while
* preventing accumulation of consecutive executions, while the task runs.
*
* Simply combine the two mail man strategies from the Throttler and Delayer
* helpers, for an analogy.
*/
export class ThrottledDelayer<T> extends Delayer<Promise<T>> {
private throttler: Throttler<T>

constructor(defaultDelay: number) {
super(defaultDelay)

this.throttler = new Throttler<T>()
}

public override trigger(
promiseFactory: ITask<Promise<T>>,
delay?: number,
): Promise<Promise<T>> {
return super.trigger(() => this.throttler.queue(promiseFactory), delay)
}
}

0 comments on commit f6ef9ea

Please sign in to comment.