diff --git a/server/package.json b/server/package.json index 4c71af188..de960867e 100644 --- a/server/package.json +++ b/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" diff --git a/server/src/__tests__/linter.test.ts b/server/src/__tests__/linter.test.ts index 5739da532..7fe83ac9e 100644 --- a/server/src/__tests__/linter.test.ts +++ b/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' }, diff --git a/server/src/linter.ts b/server/src/linter.ts index fc71eb2b6..1da86393e 100644 --- a/server/src/linter.ts +++ b/server/src/linter.ts @@ -1,8 +1,5 @@ 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 @@ -10,22 +7,22 @@ function formatMessage(comment: ShellcheckComment): string { type LinterOptions = { executablePath: string | null + console: LSP.RemoteConsole cwd?: string } -export async function getLinterExecutablePath(): Promise { - 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) diff --git a/server/src/server.ts b/server/src/server.ts index ee713047c..e638a181c 100644 --- a/server/src/server.ts +++ b/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 { - 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 diff --git a/server/yarn.lock b/server/yarn.lock index 60a38117e..1f1a27dd3 100644 --- a/server/yarn.lock +++ b/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" diff --git a/vscode-client/package.json b/vscode-client/package.json index c34246d9f..3f0dd2dab 100644 --- a/vscode-client/package.json +++ b/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." } } }