ShellCheck: Simpler debounce

server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 4.2.3
+
+- Simpler debouncing for ShellCheck tasks where only the last request will return diagnostics https://github.com/bash-lsp/bash-language-server/pull/656
+
 ## 4.2.2
 
 - Reduce CPU usage by introduce a short execution delay and throttling for ShellCheck tasks https://github.com/bash-lsp/bash-language-server/pull/655

server/package.json

@@ -3,8 +3,7 @@
   "description": "A language server for Bash",
   "author": "Mads Hartmann",
   "license": "MIT",
-  "version": "4.2.2",
-  "publisher": "mads-hartmann",
+  "version": "4.2.3",
   "main": "./out/server.js",
   "typings": "./out/server.d.ts",
   "bin": {

server/src/shellcheck/__tests__/index.test.ts

@@ -180,6 +180,27 @@ describe('linter', () => {
     `)
   })
 
+  it('should debounce the lint requests', async () => {
+    const linter = new Linter({
+      console: mockConsole,
+      cwd: FIXTURE_FOLDER,
+      executablePath: 'shellcheck',
+    })
+
+    const lintCalls = 100
+    const promises = [...Array(lintCalls)].map(() =>
+      linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, []),
+    )
+
+    jest.runOnlyPendingTimers()
+
+    const result = await promises[promises.length - 1]
+    expect(result).toEqual({
+      codeActions: [],
+      diagnostics: [],
+    })
+  })
+
   it('should correctly follow sources with correct cwd', async () => {
     const [result] = await getLintingResult({
       cwd: FIXTURE_FOLDER,

server/src/shellcheck/index.ts

@@ -5,7 +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 { debounce } from '../util/async'
 import { analyzeShebang } from '../util/shebang'
 import { CODE_TO_TAGS, LEVEL_TO_SEVERITY } from './config'
 import {
@@ -32,8 +32,8 @@ export class Linter {
   private console: LSP.RemoteConsole
   private cwd: string
   public executablePath: string
-  private uriToDelayer: {
-    [uri: string]: ThrottledDelayer<LintingResult>
+  private uriToDebouncedExecuteLint: {
+    [uri: string]: InstanceType<typeof Linter>['executeLint']
   }
   private _canLint: boolean
 
@@ -42,7 +42,7 @@ export class Linter {
     this.console = console
     this.cwd = cwd || process.cwd()
     this.executablePath = executablePath
-    this.uriToDelayer = Object.create(null)
+    this.uriToDebouncedExecuteLint = Object.create(null)
   }
 
   public get canLint(): boolean {
@@ -59,15 +59,13 @@ export class Linter {
     }
 
     const { uri } = document
-    let delayer = this.uriToDelayer[uri]
-    if (!delayer) {
-      delayer = new ThrottledDelayer<LintingResult>(DEBOUNCE_MS)
-      this.uriToDelayer[uri] = delayer
+    let debouncedExecuteLint = this.uriToDebouncedExecuteLint[uri]
+    if (!debouncedExecuteLint) {
+      debouncedExecuteLint = debounce(this.executeLint.bind(this), DEBOUNCE_MS)
+      this.uriToDebouncedExecuteLint[uri] = debouncedExecuteLint
     }
 
-    return delayer.trigger(() =>
-      this.executeLint(document, sourcePaths, additionalShellCheckArguments),
-    )
+    return debouncedExecuteLint(document, sourcePaths, additionalShellCheckArguments)
   }
 
   private async executeLint(
@@ -85,7 +83,7 @@ export class Linter {
     }
 
     // Clean up the debounced function
-    delete this.uriToDelayer[document.uri]
+    delete this.uriToDebouncedExecuteLint[document.uri]
 
     return mapShellCheckResult({ document, result })
   }

server/src/util/async.ts

@@ -1,190 +1,28 @@
-/* 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(); });
- *   }
+ * A collection of async utilities.
+ * If we ever need to add anything fancy, then https://github.com/vscode-shellcheck/vscode-shellcheck/blob/master/src/utils/async.ts
+ * is a good place to look.
  */
-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)
-        },
-      )
-    })
-  }
-}
+type UnwrapPromise<T> = T extends Promise<infer U> ? U : T
 
 /**
- * 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(); });
- *   }
+ * Debounce a function call by a given amount of time. Only the last call
+ * will be resolved.
+ * Inspired by https://gist.github.com/ca0v/73a31f57b397606c9813472f7493a940
  */
-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>()
-  }
+export const debounce = <F extends (...args: any[]) => any>(
+  func: F,
+  waitForMs: number,
+) => {
+  let timeout: ReturnType<typeof setTimeout> | null = null
+
+  return (...args: Parameters<F>): Promise<UnwrapPromise<ReturnType<F>>> =>
+    new Promise((resolve) => {
+      if (timeout) {
+        clearTimeout(timeout)
+      }
 
-  public override trigger(
-    promiseFactory: ITask<Promise<T>>,
-    delay?: number,
-  ): Promise<Promise<T>> {
-    return super.trigger(() => this.throttler.queue(promiseFactory), delay)
-  }
+      timeout = setTimeout(() => resolve(func(...args)), waitForMs)
+    })
 }