Fix ShellCheck config

server/package.json

@@ -26,8 +26,7 @@
     "urijs": "^1.19.11",
     "vscode-languageserver": "^6.1.1",
     "vscode-languageserver-textdocument": "^1.0.1",
-    "web-tree-sitter": "^0.20.5",
-    "which": "^2.0.2"
+    "web-tree-sitter": "^0.20.5"
   },
   "scripts": {
     "prepublishOnly": "cd ../ && yarn run compile"

server/src/__tests__/linter.test.ts

@@ -2,31 +2,36 @@ import * as path from 'path'
 import * as LSP from 'vscode-languageserver'
 
 import { FIXTURE_DOCUMENT, FIXTURE_FOLDER } from '../../../testing/fixtures'
+import { getMockConnection } from '../../../testing/mocks'
 import { assertShellcheckResult, Linter } from '../linter'
 
+const mockConsole = getMockConnection().console
+
 function textToDoc(txt: string) {
   return LSP.TextDocument.create('foo', 'bar', 0, txt)
 }
 
 describe('linter', () => {
   it('should set canLint to false if executable empty', () => {
-    expect(new Linter({ executablePath: null }).canLint).toBe(false)
+    expect(new Linter({ console: mockConsole, executablePath: null }).canLint).toBe(false)
   })
 
   it('should set canLint to true if executable not empty', () => {
-    expect(new Linter({ executablePath: 'foo' }).canLint).toBe(true)
+    expect(new Linter({ console: mockConsole, executablePath: 'foo' }).canLint).toBe(true)
   })
 
   it('should set canLint to false when linting fails', async () => {
-    jest.spyOn(console, 'error').mockImplementation()
     const executablePath = '77b4d3f6-c87a-11ec-9b62-a3c90f66d29f'
     const linter = new Linter({
+      console: mockConsole,
       executablePath,
     })
     expect(await linter.lint(textToDoc(''), [])).toEqual([])
     expect(linter.canLint).toBe(false)
-    expect(console.error).toBeCalledWith(
-      expect.stringContaining('shellcheck not available at path'),
+    expect(mockConsole.warn).toBeCalledWith(
+      expect.stringContaining(
+        'ShellCheck: disabling linting as no executable was found at path',
+      ),
     )
   })
 
@@ -54,21 +59,26 @@ describe('linter', () => {
       },
     ]
 
-    const linter = new Linter({ executablePath: 'shellcheck' })
+    const linter = new Linter({ console: mockConsole, executablePath: 'shellcheck' })
     const result = await linter.lint(textToDoc(shell), [])
     expect(result).toEqual(expected)
   })
 
   it('should correctly follow sources with correct cwd', async () => {
-    const linter = new Linter({ executablePath: 'shellcheck', cwd: FIXTURE_FOLDER })
+    const linter = new Linter({
+      console: mockConsole,
+      executablePath: 'shellcheck',
+      cwd: FIXTURE_FOLDER,
+    })
     const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [])
     expect(result).toEqual([])
   })
 
   it('should fail to follow sources with incorrect cwd', async () => {
     const linter = new Linter({
-      executablePath: 'shellcheck',
+      console: mockConsole,
       cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
+      executablePath: 'shellcheck',
     })
     // prettier-ignore
     const expected = [
@@ -81,8 +91,9 @@ describe('linter', () => {
 
   it('should follow sources with incorrect cwd if correct path is passed as a workspace path', async () => {
     const linter = new Linter({
-      executablePath: 'shellcheck',
+      console: mockConsole,
       cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
+      executablePath: 'shellcheck',
     })
     const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [
       { uri: `file://${path.resolve(FIXTURE_FOLDER)}`, name: 'fixtures' },

server/src/linter.ts

@@ -1,31 +1,28 @@
 import { spawn } from 'child_process'
 import * as LSP from 'vscode-languageserver'
-import which = require('which')
-
-import * as config from './config'
 
 function formatMessage(comment: ShellcheckComment): string {
   return (comment.code ? `SC${comment.code}: ` : '') + comment.message
 }
 
 type LinterOptions = {
   executablePath: string | null
+  console: LSP.RemoteConsole
   cwd?: string
 }
 
-export async function getLinterExecutablePath(): Promise<string | null> {
-  return config.getShellcheckPath() || (await which('shellcheck'))
-}
-
 export class Linter {
   public executablePath: string | null
   private cwd: string
+  private console: LSP.RemoteConsole
+
   _canLint: boolean
 
-  constructor(opts: LinterOptions) {
-    this.executablePath = opts.executablePath
-    this.cwd = opts.cwd || process.cwd()
-    this._canLint = !!this.executablePath
+  constructor({ console, cwd, executablePath }: LinterOptions) {
+    this.executablePath = executablePath
+    this.cwd = cwd || process.cwd()
+    this._canLint = !!this.executablePath // TODO: any reason to create a linter if the path is null?
+    this.console = console
   }
 
   public get canLint(): boolean {
@@ -105,12 +102,14 @@ export class Linter {
     } catch (e) {
       if ((e as any).code === 'ENOENT') {
         // shellcheck path wasn't found, don't try to lint any more:
-        console.error(`shellcheck not available at path '${this.executablePath}'`)
+        this.console.warn(
+          `ShellCheck: disabling linting as no executable was found at path '${this.executablePath}'`,
+        )
         this._canLint = false
         return { comments: [] }
       }
       throw new Error(
-        `shellcheck failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`,
+        `ShellCheck: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`,
       )
     }
 
@@ -119,7 +118,7 @@ export class Linter {
       raw = JSON.parse(out)
     } catch (e) {
       throw new Error(
-        `shellcheck: json parse failed with error ${e}\nout:\n${out}\nerr:\n${err}`,
+        `ShellCheck: json parse failed with error ${e}\nout:\n${out}\nerr:\n${err}`,
       )
     }
     assertShellcheckResult(raw)

server/src/server.ts

@@ -8,7 +8,7 @@ import Analyzer from './analyser'
 import * as Builtins from './builtins'
 import * as config from './config'
 import Executables from './executables'
-import { getLinterExecutablePath, Linter } from './linter'
+import { Linter } from './linter'
 import { initializeParser } from './parser'
 import * as ReservedWords from './reservedWords'
 import { BashCompletionItem, CompletionItemDataType } from './types'
@@ -30,35 +30,41 @@ export default class BashServer {
     connection: LSP.Connection,
     { rootPath, capabilities }: LSP.InitializeParams,
   ): Promise<BashServer> {
-    const parser = await initializeParser()
-
     const { PATH } = process.env
 
     if (!PATH) {
       throw new Error('Expected PATH environment variable to be set')
     }
 
-    return Promise.all([Executables.fromPath(PATH), getLinterExecutablePath()]).then(
-      ([executables, linterExecutablePath]) => {
-        const analyzer = new Analyzer({ console: connection.console, parser })
-        const linter = new Linter({ executablePath: linterExecutablePath })
+    const parser = await initializeParser()
+    const analyzer = new Analyzer({ console: connection.console, parser })
 
-        let backgroundAnalysisCompleted
-        if (rootPath) {
-          // NOTE: we do not block the server initialization on this background analysis.
-          backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
-        }
+    let backgroundAnalysisCompleted
+    if (rootPath) {
+      // NOTE: we do not block the server initialization on this background analysis.
+      backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
+    }
 
-        return new BashServer({
-          analyzer,
-          backgroundAnalysisCompleted,
-          capabilities,
-          connection,
-          executables,
-          linter,
-        })
-      },
-    )
+    const shellcheckPath = config.getShellcheckPath()
+    const linter = new Linter({
+      console: connection.console,
+      executablePath: shellcheckPath,
+    })
+
+    if (!shellcheckPath) {
+      connection.console.info('ShellCheck linting is disabled.')
+    }
+
+    const executables = await Executables.fromPath(PATH)
+
+    return new BashServer({
+      analyzer,
+      backgroundAnalysisCompleted,
+      capabilities,
+      connection,
+      executables,
+      linter,
+    })
   }
 
   private executables: Executables

server/yarn.lock

@@ -161,11 +161,6 @@ is-number@^7.0.0:
   resolved "https://registry.yarnpkg.com/is-number/-/is-number-7.0.0.tgz#7535345b896734d5f80c4d06c50955527a14f12b"
   integrity sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==
 
-isexe@^2.0.0:
-  version "2.0.0"
-  resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
-  integrity sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==
-
 merge2@^1.3.0:
   version "1.4.1"
   resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae"
@@ -291,10 +286,3 @@ whatwg-url@^5.0.0:
   dependencies:
     tr46 "~0.0.3"
     webidl-conversions "^3.0.0"
-
-which@^2.0.2:
-  version "2.0.2"
-  resolved "https://registry.yarnpkg.com/which/-/which-2.0.2.tgz#7c6a8dd0a636a0327e10b59c9286eee93f3f51b1"
-  integrity sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==
-  dependencies:
-    isexe "^2.0.0"

vscode-client/package.json

@@ -34,7 +34,7 @@
         "bashIde.globPattern": {
           "type": "string",
           "default": "**/*@(.sh|.inc|.bash|.command)",
-          "description": "Glob pattern for finding and parsing shell script files."
+          "description": "Glob pattern for finding and parsing shell script files in the workspace."
         },
         "bashIde.highlightParsingErrors": {
           "type": "boolean",
@@ -44,12 +44,12 @@
         "bashIde.explainshellEndpoint": {
           "type": "string",
           "default": "",
-          "description": "Configure explainshell server in order to get hover documentation on flags and options."
+          "description": "Configure explainshell server endpoint in order to get hover documentation on flags and options."
         },
         "bashIde.shellcheckPath": {
           "type": "string",
-          "default": "",
-          "description": "Configure a custom path for the Shellcheck executable in order to get linting information."
+          "default": "shellcheck",
+          "description": "Controls the executable used for ShellCheck linting information. An empty string will disable linting."
         }
       }
     }