diff --git a/docs/development-guide.md b/docs/development-guide.md index 42527cc1e..27efd8a60 100644 --- a/docs/development-guide.md +++ b/docs/development-guide.md @@ -16,9 +16,9 @@ convenience - it proxies to the `package.json` files in the `vscode-client` and This guide presumes you have the following dependencies installed: - [`yarn`][yarn]. -- [`node`][node] (v6 or newer) +- [`node`][node] (v14 or newer) - `g++` -- `make` +- `make` (optional) ## Initial setup @@ -85,6 +85,20 @@ reload your vscode window to re-launch the server. yarn run reinstall-server ``` +If you for some reason cannot get access to logs through the client, +then you can hack the `server/util/logger` with: + +```typescript +const fs = require('fs') +const util = require('util') +const log_file = fs.createWriteStream(`/tmp/bash-language-server-debug.log`, { + flags: 'w', +}) + +// inside log function +log_file.write(`${severity} ${util.format(message)}\n`) +``` + ## Performance To analyze the performance of the extension or server using the Chrome inspector: diff --git a/server/CHANGELOG.md b/server/CHANGELOG.md index d43c2addc..210ac6088 100644 --- a/server/CHANGELOG.md +++ b/server/CHANGELOG.md @@ -1,5 +1,9 @@ # Bash Language Server +## 4.6.1 + +- Fix the ShellCheck code action feature that for some clients did not return any code actions. https://github.com/bash-lsp/bash-language-server/pull/700 + ## 4.6.0 - Support parsing `: "${VARIABLE:="default"}"` as a variable definition https://github.com/bash-lsp/bash-language-server/pull/693 diff --git a/server/package.json b/server/package.json index 88bd399cf..d7813d368 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.6.0", + "version": "4.6.1", "main": "./out/server.js", "typings": "./out/server.d.ts", "bin": { diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 4b21a0761..de421817f 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -177,29 +177,30 @@ describe('server', () => { const fixableDiagnostic = diagnostics.filter(({ code }) => code === 'SC2086')[0] expect(fixableDiagnostic).toMatchInlineSnapshot(` - Object { - "code": "SC2086", - "codeDescription": Object { - "href": "https://www.shellcheck.net/wiki/SC2086", - }, - "message": "Double quote to prevent globbing and word splitting.", - "range": Object { - "end": Object { - "character": 13, - "line": 55, - }, - "start": Object { - "character": 5, - "line": 55, - }, - }, - "severity": 3, - "source": "shellcheck", - "tags": undefined, - } - `) - - // TODO: we could find the diagnostics and then use the range to test the code action + Object { + "code": "SC2086", + "codeDescription": Object { + "href": "https://www.shellcheck.net/wiki/SC2086", + }, + "data": Object { + "id": "shellcheck|2086|55:5-55:13", + }, + "message": "Double quote to prevent globbing and word splitting.", + "range": Object { + "end": Object { + "character": 13, + "line": 55, + }, + "start": Object { + "character": 5, + "line": 55, + }, + }, + "severity": 3, + "source": "shellcheck", + "tags": undefined, + } + `) const onCodeAction = connection.onCodeAction.mock.calls[0][0] diff --git a/server/src/server.ts b/server/src/server.ts index 53a8ef76d..d2794d320 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -4,7 +4,6 @@ import { isDeepStrictEqual } from 'node:util' import * as TurndownService from 'turndown' import * as LSP from 'vscode-languageserver/node' -import { CodeAction } from 'vscode-languageserver/node' import { TextDocument } from 'vscode-languageserver-textdocument' import Analyzer from './analyser' @@ -13,7 +12,7 @@ import * as config from './config' import Executables from './executables' import { initializeParser } from './parser' import * as ReservedWords from './reserved-words' -import { Linter } from './shellcheck' +import { Linter, LintingResult } from './shellcheck' import { SNIPPETS } from './snippets' import { BashCompletionItem, CompletionItemDataType } from './types' import { uniqueBasedOnHash } from './util/array' @@ -37,7 +36,9 @@ export default class BashServer { private executables: Executables private linter?: Linter private workspaceFolder: string | null - private uriToCodeActions: { [uri: string]: CodeAction[] | undefined } = {} + private uriToCodeActions: { + [uri: string]: LintingResult['codeActions'] | undefined + } = {} private constructor({ analyzer, @@ -385,41 +386,15 @@ export default class BashServer { // ============================== private async onCodeAction(params: LSP.CodeActionParams): Promise { - const codeActions = this.uriToCodeActions[params.textDocument.uri] + const codeActionsForUri = this.uriToCodeActions[params.textDocument.uri] || {} - if (!codeActions) { - return [] - } - - const getDiagnosticsFingerPrint = (diagnostics?: LSP.Diagnostic[]): string[] => - diagnostics - ?.map(({ code, source, range }) => - code !== undefined && source && range - ? JSON.stringify({ - code, - source, - range, - }) - : null, - ) - .filter((fingerPrint): fingerPrint is string => fingerPrint != null) || [] + const codeActions = params.context.diagnostics + .map(({ data }) => codeActionsForUri[data?.id]) + .filter((action): action is LSP.CodeAction => action != null) - const paramsDiagnosticsKeys = getDiagnosticsFingerPrint(params.context.diagnostics) - - // find actions that match the paramsDiagnosticsKeys - const actions = codeActions.filter((action) => { - const actionDiagnosticsKeys = getDiagnosticsFingerPrint(action.diagnostics) - // actions without diagnostics are always returned - if (actionDiagnosticsKeys.length === 0) { - return true - } - - return actionDiagnosticsKeys.some((actionDiagnosticKey) => - paramsDiagnosticsKeys.includes(actionDiagnosticKey), - ) - }) + logger.debug(`onCodeAction: found ${codeActions.length} code action(s)`) - return actions + return codeActions } private onCompletion(params: LSP.TextDocumentPositionParams): BashCompletionItem[] { diff --git a/server/src/shellcheck/__tests__/index.test.ts b/server/src/shellcheck/__tests__/index.test.ts index 78d280432..27d7621be 100644 --- a/server/src/shellcheck/__tests__/index.test.ts +++ b/server/src/shellcheck/__tests__/index.test.ts @@ -54,7 +54,7 @@ describe('linter', () => { }) expect(result).toEqual({ - codeActions: [], + codeActions: {}, diagnostics: [], }) @@ -76,14 +76,17 @@ describe('linter', () => { const [result] = await getLintingResult({ document: textToDoc(shell) }) expect(result).toMatchInlineSnapshot(` Object { - "codeActions": Array [ - Object { + "codeActions": Object { + "shellcheck|2086|1:5-1:9": Object { "diagnostics": Array [ Object { "code": "SC2086", "codeDescription": Object { "href": "https://www.shellcheck.net/wiki/SC2086", }, + "data": Object { + "id": "shellcheck|2086|1:5-1:9", + }, "message": "Double quote to prevent globbing and word splitting.", "range": Object { "end": Object { @@ -135,13 +138,16 @@ describe('linter', () => { "kind": "quickfix", "title": "Apply fix for SC2086", }, - ], + }, "diagnostics": Array [ Object { "code": "SC2154", "codeDescription": Object { "href": "https://www.shellcheck.net/wiki/SC2154", }, + "data": Object { + "id": "shellcheck|2154|1:5-1:9", + }, "message": "foo is referenced but not assigned.", "range": Object { "end": Object { @@ -162,6 +168,9 @@ describe('linter', () => { "codeDescription": Object { "href": "https://www.shellcheck.net/wiki/SC2086", }, + "data": Object { + "id": "shellcheck|2086|1:5-1:9", + }, "message": "Double quote to prevent globbing and word splitting.", "range": Object { "end": Object { @@ -197,7 +206,7 @@ describe('linter', () => { const result = await promises[promises.length - 1] expect(result).toEqual({ - codeActions: [], + codeActions: {}, diagnostics: [], }) }) @@ -209,7 +218,7 @@ describe('linter', () => { }) expect(result).toEqual({ - codeActions: [], + codeActions: {}, diagnostics: [], }) }) @@ -222,13 +231,16 @@ describe('linter', () => { expect(result).toMatchInlineSnapshot(` Object { - "codeActions": Array [], + "codeActions": Object {}, "diagnostics": Array [ Object { "code": "SC1091", "codeDescription": Object { "href": "https://www.shellcheck.net/wiki/SC1091", }, + "data": Object { + "id": "shellcheck|1091|3:7-3:19", + }, "message": "Not following: shellcheck/sourced.sh: openBinaryFile: does not exist (No such file or directory)", "range": Object { "end": Object { @@ -249,6 +261,9 @@ describe('linter', () => { "codeDescription": Object { "href": "https://www.shellcheck.net/wiki/SC2154", }, + "data": Object { + "id": "shellcheck|2154|5:6-5:10", + }, "message": "foo is referenced but not assigned.", "range": Object { "end": Object { @@ -276,7 +291,7 @@ describe('linter', () => { sourcePaths: [path.resolve(FIXTURE_FOLDER)], }) expect(result).toEqual({ - codeActions: [], + codeActions: {}, diagnostics: [], }) }) diff --git a/server/src/shellcheck/index.ts b/server/src/shellcheck/index.ts index aadba925d..5f4f207bf 100644 --- a/server/src/shellcheck/index.ts +++ b/server/src/shellcheck/index.ts @@ -23,9 +23,9 @@ type LinterOptions = { cwd?: string } -type LintingResult = { +export type LintingResult = { diagnostics: LSP.Diagnostic[] - codeActions: LSP.CodeAction[] + codeActions: Record } export class Linter { @@ -53,7 +53,7 @@ export class Linter { additionalShellCheckArguments: string[] = [], ): Promise { if (!this._canLint) { - return { diagnostics: [], codeActions: [] } + return { diagnostics: [], codeActions: {} } } const { uri } = document @@ -80,7 +80,7 @@ export class Linter { if (shellDialect && !SUPPORTED_BASH_DIALECTS.includes(shellDialect)) { // We found a dialect that isn't supported by ShellCheck. - return { diagnostics: [], codeActions: [] } + return { diagnostics: [], codeActions: {} } } // NOTE: that ShellCheck actually does shebang parsing, but we manually @@ -99,7 +99,7 @@ export class Linter { ) if (!this._canLint) { - return { diagnostics: [], codeActions: [] } + return { diagnostics: [], codeActions: {} } } // Clean up the debounced function @@ -180,40 +180,37 @@ export class Linter { } } -function mapShellCheckResult({ - uri, - result, -}: { - uri: string - result: ShellCheckResult -}): { - diagnostics: LSP.Diagnostic[] - codeActions: LSP.CodeAction[] -} { - const diagnostics: LSP.Diagnostic[] = [] - const codeActions: LSP.CodeAction[] = [] +function mapShellCheckResult({ uri, result }: { uri: string; result: ShellCheckResult }) { + const diagnostics: LintingResult['diagnostics'] = [] + const codeActions: LintingResult['codeActions'] = {} for (const comment of result.comments) { - const start: LSP.Position = { - line: comment.line - 1, - character: comment.column - 1, - } - const end: LSP.Position = { - line: comment.endLine - 1, - character: comment.endColumn - 1, - } + const range = LSP.Range.create( + { + line: comment.line - 1, + character: comment.column - 1, + }, + { + line: comment.endLine - 1, + character: comment.endColumn - 1, + }, + ) + + const id = `shellcheck|${comment.code}|${range.start.line}:${range.start.character}-${range.end.line}:${range.end.character}` const diagnostic: LSP.Diagnostic = { message: comment.message, severity: LEVEL_TO_SEVERITY[comment.level] || LSP.DiagnosticSeverity.Error, code: `SC${comment.code}`, source: 'shellcheck', - range: LSP.Range.create(start, end), + range, codeDescription: { href: `https://www.shellcheck.net/wiki/SC${comment.code}`, }, tags: CODE_TO_TAGS[comment.code], - // NOTE: we could use the 'data' property this enable easier fingerprinting + data: { + id, + }, } diagnostics.push(diagnostic) @@ -225,7 +222,7 @@ function mapShellCheckResult({ }) if (codeAction) { - codeActions.push(codeAction) + codeActions[id] = codeAction } }