From 0f76c91ef9df02f4fd520480ba7292119dd3a18a Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 11:59:52 +0100 Subject: [PATCH 1/6] Run ShellCheck when server boots with an open file --- server/src/__tests__/server.test.ts | 2 +- server/src/server.ts | 29 +++++++++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index b61e536fa..33aef8c4d 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -517,7 +517,7 @@ describe('server', () => { const { connection, server } = await initializeServer() const document = FIXTURE_DOCUMENT.COMMENT_DOC - await server.onDocumentContentChange(document) + await server.analyzeAndLintDocument(document) expect(connection.sendDiagnostics).toHaveBeenCalledTimes(1) const { diagnostics } = connection.sendDiagnostics.mock.calls[0][0] diff --git a/server/src/server.ts b/server/src/server.ts index 638f28044..7971e6e22 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -92,11 +92,20 @@ export default class BashServer { * care about. */ public register(connection: LSP.Connection): void { + const hasConfigurationCapability = !!this.clientCapabilities?.workspace?.configuration + + let currentDocument: TextDocument | null = null + let initialized = false // Whether the client finished initializing + this.documents.listen(this.connection) - this.documents.onDidChangeContent(async ({ document }) => { + + this.documents.onDidChangeContent(({ document }) => { // The content of a text document has changed. This event is emitted // when the text document first opened or when its content has changed. - await this.onDocumentContentChange(document) + currentDocument = document + if (initialized) { + this.analyzeAndLintDocument(document) + } }) this.documents.onDidClose((event) => { @@ -114,9 +123,6 @@ export default class BashServer { connection.onCompletionResolve(this.onCompletionResolve.bind(this)) connection.onCodeAction(this.onCodeAction.bind(this)) - const hasConfigurationCapability = !!this.clientCapabilities?.workspace?.configuration - let initialized = false - /** * The initialized notification is sent from the client to the server after * the client received the result of the initialize request but before the @@ -162,6 +168,11 @@ export default class BashServer { } initialized = true + if (currentDocument) { + // If we already have a document, analyze it now that we're initialized + // and the linter is ready. + this.analyzeAndLintDocument(currentDocument) + } // NOTE: we do not block the server initialization on this background analysis. return { backgroundAnalysisCompleted: this.startBackgroundAnalysis() } @@ -173,7 +184,7 @@ export default class BashServer { if (configChanged && initialized) { this.connection.console.log('Configuration changed') this.startBackgroundAnalysis() - // TODO: we should trigger the linter again + // TODO: we should trigger the linter again for the current file } }) @@ -211,10 +222,9 @@ export default class BashServer { } /** - * The content of a text document has changed. This event is emitted - * when the text document first opened or when its content has changed. + * Analyze and lint the given document. */ - public async onDocumentContentChange(document: TextDocument) { + public async analyzeAndLintDocument(document: TextDocument) { const { uri } = document let diagnostics: LSP.Diagnostic[] = [] @@ -399,7 +409,6 @@ export default class BashServer { ) { const shellDocumentation = await getShellDocumentation({ word }) if (shellDocumentation) { - // eslint-disable-next-line no-console return { contents: getMarkdownContent(shellDocumentation) } } } else { From 8ce79039ec702a17e2064176ba254414c5b9b6cc Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 12:01:05 +0100 Subject: [PATCH 2/6] Call it workspaceFolder internally --- server/src/server.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 7971e6e22..b5ecf8c21 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -32,7 +32,7 @@ export default class BashServer { private documents: LSP.TextDocuments = new LSP.TextDocuments(TextDocument) private executables: Executables private linter?: Linter - private rootPath: string | null | undefined + private workspaceFolder: string | null | undefined private uriToCodeActions: { [uri: string]: CodeAction[] | undefined } = {} private constructor({ @@ -41,14 +41,14 @@ export default class BashServer { connection, executables, linter, - rootPath, + workspaceFolder, }: { analyzer: Analyzer capabilities: LSP.ClientCapabilities connection: LSP.Connection executables: Executables linter?: Linter - rootPath: string | null | undefined + workspaceFolder: string | null | undefined }) { this.analyzer = analyzer this.clientCapabilities = capabilities @@ -56,7 +56,7 @@ export default class BashServer { this.connection = connection this.executables = executables this.linter = linter - this.rootPath = rootPath + this.workspaceFolder = workspaceFolder } /** * Initialize the server based on a connection to the client and the protocols @@ -64,10 +64,11 @@ export default class BashServer { */ public static async initialize( connection: LSP.Connection, - { rootPath, capabilities }: LSP.InitializeParams, + { rootPath, rootUri, capabilities }: LSP.InitializeParams, ): // TODO: use workspaceFolders instead of rootPath Promise { const { PATH } = process.env + const workspaceFolder = rootUri || rootPath if (!PATH) { throw new Error('Expected PATH environment variable to be set') @@ -83,7 +84,7 @@ export default class BashServer { capabilities, connection, executables, - rootPath, + workspaceFolder, }) } @@ -192,12 +193,12 @@ export default class BashServer { } private async startBackgroundAnalysis(): Promise<{ filesParsed: number }> { - const { rootPath } = this - if (rootPath) { + const { workspaceFolder } = this + if (workspaceFolder) { return this.analyzer.initiateBackgroundAnalysis({ globPattern: this.config.globPattern, backgroundAnalysisMaxFiles: this.config.backgroundAnalysisMaxFiles, - rootPath, + rootPath: workspaceFolder, }) } From f9ee30afa85d68c75af5af484b576f204555c214 Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 12:09:55 +0100 Subject: [PATCH 3/6] Allow disabling ShellCheck while running --- server/src/server.ts | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index b5ecf8c21..5b29d882c 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -132,16 +132,18 @@ export default class BashServer { * register capabilities. The initialized notification may only be sent once. */ connection.onInitialized(async () => { - const { config: newConfig, environmentVariablesUsed } = + const { config: environmentConfig, environmentVariablesUsed } = config.getConfigFromEnvironmentVariables() - this.config = newConfig - - if (environmentVariablesUsed.length > 0) { - connection.console.warn( - `Environment variable configuration is being deprecated, please use workspace configuration. The following environment variables were used: ${environmentVariablesUsed.join( - ', ', - )}`, - ) + if (environmentConfig) { + this.updateConfiguration(environmentConfig) + + if (environmentVariablesUsed.length > 0) { + connection.console.warn( + `Environment variable configuration is being deprecated, please use workspace configuration. The following environment variables were used: ${environmentVariablesUsed.join( + ', ', + )}`, + ) + } } if (hasConfigurationCapability) { @@ -158,16 +160,6 @@ export default class BashServer { this.connection.console.log('Configuration loaded from client') } - const { shellcheckPath } = this.config - if (!shellcheckPath) { - connection.console.info('ShellCheck linting is disabled.') - } else { - this.linter = new Linter({ - console: connection.console, - executablePath: shellcheckPath, - }) - } - initialized = true if (currentDocument) { // If we already have a document, analyze it now that we're initialized @@ -212,6 +204,20 @@ export default class BashServer { if (!isDeepStrictEqual(this.config, newConfig)) { this.config = newConfig + + const { shellcheckPath } = this.config + if (!shellcheckPath) { + this.connection.console.log( + 'ShellCheck linting is disabled as "shellcheckPath" was not set', + ) + this.linter = undefined + } else { + this.linter = new Linter({ + console: this.connection.console, + executablePath: shellcheckPath, + }) + } + return true } } catch (err) { From c83dd9aadcd839eb40dea08746aaf6429c09b6a3 Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 12:10:11 +0100 Subject: [PATCH 4/6] Analyze and lint current file when changing config --- server/src/server.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/server.ts b/server/src/server.ts index 5b29d882c..48f507aca 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -177,7 +177,10 @@ export default class BashServer { if (configChanged && initialized) { this.connection.console.log('Configuration changed') this.startBackgroundAnalysis() - // TODO: we should trigger the linter again for the current file + if (currentDocument) { + this.uriToCodeActions[currentDocument.uri] = undefined + this.analyzeAndLintDocument(currentDocument) + } } }) From 1e419a08dabc8eefb445813e9e4778e3b64a4f88 Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 12:11:34 +0100 Subject: [PATCH 5/6] Bump server version --- server/CHANGELOG.md | 2 +- server/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/CHANGELOG.md b/server/CHANGELOG.md index 6bbef31b1..71d019d9b 100644 --- a/server/CHANGELOG.md +++ b/server/CHANGELOG.md @@ -1,6 +1,6 @@ # Bash Language Server -## 4.0.0 +## 4.0.1 - **Breaking**: Drop support for Node 12, which reached its official end of life on April 30th 2022. Doing so enables new features. https://github.com/bash-lsp/bash-language-server/pull/584 - ShellCheck: support code actions, remove duplicated error codes, add URLs and tags, support parsing dialects (sh, bash, dash, ksh) but still fallback to bash, enable configuring ShellCheck arguments using the `shellcheckArguments` configuration parameter (legacy environment variable: `SHELLCHECK_ARGUMENTS`) diff --git a/server/package.json b/server/package.json index adbab75e6..8f5fb21c4 100644 --- a/server/package.json +++ b/server/package.json @@ -3,7 +3,7 @@ "description": "A language server for Bash", "author": "Mads Hartmann", "license": "MIT", - "version": "4.0.0", + "version": "4.0.1", "publisher": "mads-hartmann", "main": "./out/server.js", "typings": "./out/server.d.ts", From d210c242c24103ca571cc24a6d18f804b4cb46e1 Mon Sep 17 00:00:00 2001 From: skovhus Date: Tue, 6 Dec 2022 12:24:02 +0100 Subject: [PATCH 6/6] Ensure linter is configured for default config --- server/src/server.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 48f507aca..45f0d113c 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -52,11 +52,12 @@ export default class BashServer { }) { this.analyzer = analyzer this.clientCapabilities = capabilities - this.config = config.getDefaultConfiguration() this.connection = connection this.executables = executables this.linter = linter this.workspaceFolder = workspaceFolder + this.config = {} as any // NOTE: configured in updateConfiguration + this.updateConfiguration(config.getDefaultConfiguration()) } /** * Initialize the server based on a connection to the client and the protocols @@ -134,16 +135,14 @@ export default class BashServer { connection.onInitialized(async () => { const { config: environmentConfig, environmentVariablesUsed } = config.getConfigFromEnvironmentVariables() - if (environmentConfig) { - this.updateConfiguration(environmentConfig) - if (environmentVariablesUsed.length > 0) { - connection.console.warn( - `Environment variable configuration is being deprecated, please use workspace configuration. The following environment variables were used: ${environmentVariablesUsed.join( - ', ', - )}`, - ) - } + if (environmentVariablesUsed.length > 0) { + this.updateConfiguration(environmentConfig) + connection.console.warn( + `Environment variable configuration is being deprecated, please use workspace configuration. The following environment variables were used: ${environmentVariablesUsed.join( + ', ', + )}`, + ) } if (hasConfigurationCapability) { @@ -208,6 +207,10 @@ export default class BashServer { if (!isDeepStrictEqual(this.config, newConfig)) { this.config = newConfig + // NOTE: I don't really like this... An alternative would be to pass in the + // shellcheck executable path when linting. We would need to handle + // resetting the canLint flag though. + const { shellcheckPath } = this.config if (!shellcheckPath) { this.connection.console.log(