Skip to content

Commit 3736fe8

Browse files
skovhusskovhus
authored
Merge pull request #571 from bash-lsp/shellcheck-config-v2
Fix ShellCheck config
2 parents a8e33fb + 11a39c9 commit 3736fe8

File tree

6 files changed

+66
-63
lines changed

6 files changed

+66
-63
lines changed

‎server/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@
2626
"urijs": "^1.19.11",
2727
"vscode-languageserver": "^6.1.1",
2828
"vscode-languageserver-textdocument": "^1.0.1",
29-
"web-tree-sitter": "^0.20.5",
30-
"which": "^2.0.2"
29+
"web-tree-sitter": "^0.20.5"
3130
},
3231
"scripts": {
3332
"prepublishOnly": "cd ../ && yarn run compile"

‎server/src/__tests__/linter.test.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,36 @@ import * as path from 'path'
22
import * as LSP from 'vscode-languageserver'
33

44
import { FIXTURE_DOCUMENT, FIXTURE_FOLDER } from '../../../testing/fixtures'
5+
import { getMockConnection } from '../../../testing/mocks'
56
import { assertShellcheckResult, Linter } from '../linter'
67

8+
const mockConsole = getMockConnection().console
9+
710
function textToDoc(txt: string) {
811
return LSP.TextDocument.create('foo', 'bar', 0, txt)
912
}
1013

1114
describe('linter', () => {
1215
it('should set canLint to false if executable empty', () => {
13-
expect(new Linter({ executablePath: null }).canLint).toBe(false)
16+
expect(new Linter({ console: mockConsole, executablePath: null }).canLint).toBe(false)
1417
})
1518

1619
it('should set canLint to true if executable not empty', () => {
17-
expect(new Linter({ executablePath: 'foo' }).canLint).toBe(true)
20+
expect(new Linter({ console: mockConsole, executablePath: 'foo' }).canLint).toBe(true)
1821
})
1922

2023
it('should set canLint to false when linting fails', async () => {
21-
jest.spyOn(console, 'error').mockImplementation()
2224
const executablePath = '77b4d3f6-c87a-11ec-9b62-a3c90f66d29f'
2325
const linter = new Linter({
26+
console: mockConsole,
2427
executablePath,
2528
})
2629
expect(await linter.lint(textToDoc(''), [])).toEqual([])
2730
expect(linter.canLint).toBe(false)
28-
expect(console.error).toBeCalledWith(
29-
expect.stringContaining('shellcheck not available at path'),
31+
expect(mockConsole.warn).toBeCalledWith(
32+
expect.stringContaining(
33+
'ShellCheck: disabling linting as no executable was found at path',
34+
),
3035
)
3136
})
3237

@@ -54,21 +59,26 @@ describe('linter', () => {
5459
},
5560
]
5661

57-
const linter = new Linter({ executablePath: 'shellcheck' })
62+
const linter = new Linter({ console: mockConsole, executablePath: 'shellcheck' })
5863
const result = await linter.lint(textToDoc(shell), [])
5964
expect(result).toEqual(expected)
6065
})
6166

6267
it('should correctly follow sources with correct cwd', async () => {
63-
const linter = new Linter({ executablePath: 'shellcheck', cwd: FIXTURE_FOLDER })
68+
const linter = new Linter({
69+
console: mockConsole,
70+
executablePath: 'shellcheck',
71+
cwd: FIXTURE_FOLDER,
72+
})
6473
const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [])
6574
expect(result).toEqual([])
6675
})
6776

6877
it('should fail to follow sources with incorrect cwd', async () => {
6978
const linter = new Linter({
70-
executablePath: 'shellcheck',
79+
console: mockConsole,
7180
cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
81+
executablePath: 'shellcheck',
7282
})
7383
// prettier-ignore
7484
const expected = [
@@ -81,8 +91,9 @@ describe('linter', () => {
8191

8292
it('should follow sources with incorrect cwd if correct path is passed as a workspace path', async () => {
8393
const linter = new Linter({
84-
executablePath: 'shellcheck',
94+
console: mockConsole,
8595
cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
96+
executablePath: 'shellcheck',
8697
})
8798
const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [
8899
{ uri: `file://${path.resolve(FIXTURE_FOLDER)}`, name: 'fixtures' },

‎server/src/linter.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,28 @@
11
import { spawn } from 'child_process'
22
import * as LSP from 'vscode-languageserver'
3-
import which = require('which')
4-
5-
import * as config from './config'
63

74
function formatMessage(comment: ShellcheckComment): string {
85
return (comment.code ? `SC${comment.code}: ` : '') + comment.message
96
}
107

118
type LinterOptions = {
129
executablePath: string | null
10+
console: LSP.RemoteConsole
1311
cwd?: string
1412
}
1513

16-
export async function getLinterExecutablePath(): Promise<string | null> {
17-
return config.getShellcheckPath() || (await which('shellcheck'))
18-
}
19-
2014
export class Linter {
2115
public executablePath: string | null
2216
private cwd: string
17+
private console: LSP.RemoteConsole
18+
2319
_canLint: boolean
2420

25-
constructor(opts: LinterOptions) {
26-
this.executablePath = opts.executablePath
27-
this.cwd = opts.cwd || process.cwd()
28-
this._canLint = !!this.executablePath
21+
constructor({ console, cwd, executablePath }: LinterOptions) {
22+
this.executablePath = executablePath
23+
this.cwd = cwd || process.cwd()
24+
this._canLint = !!this.executablePath // TODO: any reason to create a linter if the path is null?
25+
this.console = console
2926
}
3027

3128
public get canLint(): boolean {
@@ -105,12 +102,14 @@ export class Linter {
105102
} catch (e) {
106103
if ((e as any).code === 'ENOENT') {
107104
// shellcheck path wasn't found, don't try to lint any more:
108-
console.error(`shellcheck not available at path '${this.executablePath}'`)
105+
this.console.warn(
106+
`ShellCheck: disabling linting as no executable was found at path '${this.executablePath}'`,
107+
)
109108
this._canLint = false
110109
return { comments: [] }
111110
}
112111
throw new Error(
113-
`shellcheck failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`,
112+
`ShellCheck: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`,
114113
)
115114
}
116115

@@ -119,7 +118,7 @@ export class Linter {
119118
raw = JSON.parse(out)
120119
} catch (e) {
121120
throw new Error(
122-
`shellcheck: json parse failed with error ${e}\nout:\n${out}\nerr:\n${err}`,
121+
`ShellCheck: json parse failed with error ${e}\nout:\n${out}\nerr:\n${err}`,
123122
)
124123
}
125124
assertShellcheckResult(raw)

‎server/src/server.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import Analyzer from './analyser'
88
import * as Builtins from './builtins'
99
import * as config from './config'
1010
import Executables from './executables'
11-
import { getLinterExecutablePath, Linter } from './linter'
11+
import { Linter } from './linter'
1212
import { initializeParser } from './parser'
1313
import * as ReservedWords from './reservedWords'
1414
import { BashCompletionItem, CompletionItemDataType } from './types'
@@ -30,35 +30,41 @@ export default class BashServer {
3030
connection: LSP.Connection,
3131
{ rootPath, capabilities }: LSP.InitializeParams,
3232
): Promise<BashServer> {
33-
const parser = await initializeParser()
34-
3533
const { PATH } = process.env
3634

3735
if (!PATH) {
3836
throw new Error('Expected PATH environment variable to be set')
3937
}
4038

41-
return Promise.all([Executables.fromPath(PATH), getLinterExecutablePath()]).then(
42-
([executables, linterExecutablePath]) => {
43-
const analyzer = new Analyzer({ console: connection.console, parser })
44-
const linter = new Linter({ executablePath: linterExecutablePath })
39+
const parser = await initializeParser()
40+
const analyzer = new Analyzer({ console: connection.console, parser })
4541

46-
let backgroundAnalysisCompleted
47-
if (rootPath) {
48-
// NOTE: we do not block the server initialization on this background analysis.
49-
backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
50-
}
42+
let backgroundAnalysisCompleted
43+
if (rootPath) {
44+
// NOTE: we do not block the server initialization on this background analysis.
45+
backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
46+
}
5147

52-
return new BashServer({
53-
analyzer,
54-
backgroundAnalysisCompleted,
55-
capabilities,
56-
connection,
57-
executables,
58-
linter,
59-
})
60-
},
61-
)
48+
const shellcheckPath = config.getShellcheckPath()
49+
const linter = new Linter({
50+
console: connection.console,
51+
executablePath: shellcheckPath,
52+
})
53+
54+
if (!shellcheckPath) {
55+
connection.console.info('ShellCheck linting is disabled.')
56+
}
57+
58+
const executables = await Executables.fromPath(PATH)
59+
60+
return new BashServer({
61+
analyzer,
62+
backgroundAnalysisCompleted,
63+
capabilities,
64+
connection,
65+
executables,
66+
linter,
67+
})
6268
}
6369

6470
private executables: Executables

‎server/yarn.lock

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,6 @@ is-number@^7.0.0:
161161
resolved "https://registry.yarnpkg.com/is-number/-/is-number-7.0.0.tgz#7535345b896734d5f80c4d06c50955527a14f12b"
162162
integrity sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==
163163

164-
isexe@^2.0.0:
165-
version "2.0.0"
166-
resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
167-
integrity sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==
168-
169164
merge2@^1.3.0:
170165
version "1.4.1"
171166
resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae"
@@ -291,10 +286,3 @@ whatwg-url@^5.0.0:
291286
dependencies:
292287
tr46 "~0.0.3"
293288
webidl-conversions "^3.0.0"
294-
295-
which@^2.0.2:
296-
version "2.0.2"
297-
resolved "https://registry.yarnpkg.com/which/-/which-2.0.2.tgz#7c6a8dd0a636a0327e10b59c9286eee93f3f51b1"
298-
integrity sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==
299-
dependencies:
300-
isexe "^2.0.0"

‎vscode-client/package.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
"bashIde.globPattern": {
3535
"type": "string",
3636
"default": "**/*@(.sh|.inc|.bash|.command)",
37-
"description": "Glob pattern for finding and parsing shell script files."
37+
"description": "Glob pattern for finding and parsing shell script files in the workspace."
3838
},
3939
"bashIde.highlightParsingErrors": {
4040
"type": "boolean",
@@ -44,12 +44,12 @@
4444
"bashIde.explainshellEndpoint": {
4545
"type": "string",
4646
"default": "",
47-
"description": "Configure explainshell server in order to get hover documentation on flags and options."
47+
"description": "Configure explainshell server endpoint in order to get hover documentation on flags and options."
4848
},
4949
"bashIde.shellcheckPath": {
5050
"type": "string",
51-
"default": "",
52-
"description": "Configure a custom path for the Shellcheck executable in order to get linting information."
51+
"default": "shellcheck",
52+
"description": "Controls the executable used for ShellCheck linting information. An empty string will disable linting."
5353
}
5454
}
5555
}

0 commit comments

Comments
 (0)