diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index e2d68c655..c6a4c3fff 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -14,8 +14,8 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install shellcheck (used for testing) - run: sudo apt-get install -y shellcheck + - name: Install shellcheck and shfmt (used for testing) + run: sudo apt-get install -y shellcheck shfmt - uses: pnpm/action-setup@v2 with: diff --git a/README.md b/README.md index 94e2126d6..d52e8845f 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Bash Language Server -Bash language server that brings an IDE-like experience for bash scripts to most editors. This is based on the [Tree Sitter parser][tree-sitter-bash] and supports [explainshell][explainshell] and [shellcheck][shellcheck]. +Bash language server that brings an IDE-like experience for bash scripts to most editors. This is based on the [Tree Sitter parser][tree-sitter-bash] and supports [explainshell][explainshell], [shellcheck][shellcheck] and [shfmt][shfmt]. Documentation around configuration variables can be found in the [config.ts](https://github.com/bash-lsp/bash-language-server/blob/main/server/src/config.ts) file. @@ -15,6 +15,7 @@ Documentation around configuration variables can be found in the [config.ts](htt - Documentation for symbols on hover - Workspace symbols - Rename symbol +- Format document To be implemented: @@ -24,7 +25,11 @@ To be implemented: ### Dependencies -As a dependency, we recommend that you first install shellcheck [shellcheck][shellcheck] to enable linting: https://github.com/koalaman/shellcheck#installing . If shellcheck is installed, bash-language-server will automatically call it to provide linting and code analysis each time the file is updated (with debounce time or 500ms). +As a dependency, we recommend that you first install shellcheck [shellcheck][shellcheck] to enable linting: https://github.com/koalaman/shellcheck#installing . If shellcheck is installed, bash-language-server will automatically call it to provide linting and code analysis each time the file is updated (with debounce time of 500ms). + +If you want your shell scripts to be formatted consistently, you can install [shfmt][shfmt]. If +`shfmt` is installed then your documents will be formatted whenever you take the 'format document' +action. In most editors this can be configured to happen automatically when files are saved. ### Bash language server @@ -197,6 +202,7 @@ Please see [docs/development-guide][dev-guide] for more information. [sublime-text-lsp]: https://packagecontrol.io/packages/LSP-bash [explainshell]: https://explainshell.com/ [shellcheck]: https://www.shellcheck.net/ +[shfmt]: https://github.com/mvdan/sh#shfmt [languageclient-neovim]: https://github.com/autozimu/LanguageClient-neovim [nvim-lspconfig]: https://github.com/neovim/nvim-lspconfig [vim-lsp]: https://github.com/prabirshrestha/vim-lsp diff --git a/server/CHANGELOG.md b/server/CHANGELOG.md index e4ef00f46..cad493c66 100644 --- a/server/CHANGELOG.md +++ b/server/CHANGELOG.md @@ -1,5 +1,9 @@ # Bash Language Server +## 5.3.0 + +- Add support for formatting using shfmt (if installed). https://github.com/bash-lsp/bash-language-server/pull/1136 + ## 5.2.0 - Upgrade tree-sitter-bash from 0.20.7 to 0.22.5 https://github.com/bash-lsp/bash-language-server/pull/1148 diff --git a/server/package.json b/server/package.json index 28a93dbc2..b8238e2ad 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": "5.2.0", + "version": "5.3.0", "main": "./out/server.js", "typings": "./out/server.d.ts", "bin": { diff --git a/server/src/__tests__/__snapshots__/server.test.ts.snap b/server/src/__tests__/__snapshots__/server.test.ts.snap index 0b6575d3a..ff2550c5c 100644 --- a/server/src/__tests__/__snapshots__/server.test.ts.snap +++ b/server/src/__tests__/__snapshots__/server.test.ts.snap @@ -909,6 +909,7 @@ exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceE "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/shell-directive.bash": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/source.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/sourced.sh": [], + "file://__REPO_ROOT_FOLDER__/testing/fixtures/shfmt.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/sourcing.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/sourcing2.sh": [], }, @@ -1001,6 +1002,7 @@ exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceE "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/shell-directive.bash": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/source.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/shellcheck/sourced.sh": [], + "file://__REPO_ROOT_FOLDER__/testing/fixtures/shfmt.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/sourcing.sh": [], "file://__REPO_ROOT_FOLDER__/testing/fixtures/sourcing2.sh": [], }, diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index f8e4c0320..8d4aa357e 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -16,7 +16,7 @@ import { Logger } from '../util/logger' const CURRENT_URI = 'dummy-uri.sh' // if you add a .sh file to testing/fixtures, update this value -const FIXTURE_FILES_MATCHING_GLOB = 18 +const FIXTURE_FILES_MATCHING_GLOB = 19 const defaultConfig = getDefaultConfiguration() diff --git a/server/src/__tests__/config.test.ts b/server/src/__tests__/config.test.ts index a83526a0d..1d02dcf00 100644 --- a/server/src/__tests__/config.test.ts +++ b/server/src/__tests__/config.test.ts @@ -13,6 +13,13 @@ describe('ConfigSchema', () => { "logLevel": "info", "shellcheckArguments": [], "shellcheckPath": "shellcheck", + "shfmt": { + "binaryNextLine": false, + "caseIndent": false, + "funcNextLine": false, + "path": "shfmt", + "spaceRedirects": false, + }, } `) }) @@ -25,6 +32,13 @@ describe('ConfigSchema', () => { includeAllWorkspaceSymbols: true, shellcheckArguments: ' -e SC2001 -e SC2002 ', shellcheckPath: '', + shfmt: { + binaryNextLine: true, + caseIndent: true, + funcNextLine: true, + path: 'myshfmt', + spaceRedirects: true, + }, }), ).toMatchInlineSnapshot(` { @@ -41,6 +55,13 @@ describe('ConfigSchema', () => { "SC2002", ], "shellcheckPath": "", + "shfmt": { + "binaryNextLine": true, + "caseIndent": true, + "funcNextLine": true, + "path": "myshfmt", + "spaceRedirects": true, + }, } `) }) @@ -67,12 +88,20 @@ describe('getConfigFromEnvironmentVariables', () => { "logLevel": "info", "shellcheckArguments": [], "shellcheckPath": "shellcheck", + "shfmt": { + "binaryNextLine": false, + "caseIndent": false, + "funcNextLine": false, + "path": "shfmt", + "spaceRedirects": false, + }, } `) }) it('preserves an empty string', () => { process.env = { SHELLCHECK_PATH: '', + SHFMT_PATH: '', EXPLAINSHELL_ENDPOINT: '', } const { config } = getConfigFromEnvironmentVariables() @@ -86,6 +115,13 @@ describe('getConfigFromEnvironmentVariables', () => { "logLevel": "info", "shellcheckArguments": [], "shellcheckPath": "", + "shfmt": { + "binaryNextLine": false, + "caseIndent": false, + "funcNextLine": false, + "path": "", + "spaceRedirects": false, + }, } `) }) @@ -94,6 +130,8 @@ describe('getConfigFromEnvironmentVariables', () => { process.env = { SHELLCHECK_PATH: '/path/to/shellcheck', SHELLCHECK_ARGUMENTS: '-e SC2001', + SHFMT_PATH: '/path/to/shfmt', + SHFMT_CASE_INDENT: 'true', EXPLAINSHELL_ENDPOINT: 'localhost:8080', GLOB_PATTERN: '*.*', BACKGROUND_ANALYSIS_MAX_FILES: '1', @@ -113,6 +151,13 @@ describe('getConfigFromEnvironmentVariables', () => { "SC2001", ], "shellcheckPath": "/path/to/shellcheck", + "shfmt": { + "binaryNextLine": false, + "caseIndent": true, + "funcNextLine": false, + "path": "/path/to/shfmt", + "spaceRedirects": false, + }, } `) }) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 37ee108ed..5d130101f 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -85,6 +85,7 @@ describe('server', () => { ], }, "definitionProvider": true, + "documentFormattingProvider": true, "documentHighlightProvider": true, "documentSymbolProvider": true, "hoverProvider": true, diff --git a/server/src/config.ts b/server/src/config.ts index c39b6413c..1e39903b7 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -40,6 +40,25 @@ export const ConfigSchema = z.object({ // Controls the executable used for ShellCheck linting information. An empty string will disable linting. shellcheckPath: z.string().trim().default('shellcheck'), + + shfmt: z + .object({ + // Controls the executable used for Shfmt formatting. An empty string will disable formatting + path: z.string().trim().default('shfmt'), + + // Allow boolean operators (like && and ||) to start a line. + binaryNextLine: z.boolean().default(false), + + // Indent patterns in case statements. + caseIndent: z.boolean().default(false), + + // Place function opening braces on a separate line. + funcNextLine: z.boolean().default(false), + + // Follow redirection operators with a space. + spaceRedirects: z.boolean().default(false), + }) + .default({}), }) export type Config = z.infer @@ -57,6 +76,13 @@ export function getConfigFromEnvironmentVariables(): { logLevel: process.env[LOG_LEVEL_ENV_VAR], shellcheckArguments: process.env.SHELLCHECK_ARGUMENTS, shellcheckPath: process.env.SHELLCHECK_PATH, + shfmt: { + path: process.env.SHFMT_PATH, + binaryNextLine: toBoolean(process.env.SHFMT_BINARY_NEXT_LINE), + caseIndent: toBoolean(process.env.SHFMT_CASE_INDENT), + funcNextLine: toBoolean(process.env.SHFMT_FUNC_NEXT_LINE), + spaceRedirects: toBoolean(process.env.SHFMT_SPACE_REDIRECTS), + }, } const environmentVariablesUsed = Object.entries(rawConfig) diff --git a/server/src/server.ts b/server/src/server.ts index 8b70dc258..4e5f0c8df 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -13,6 +13,7 @@ import Executables from './executables' import { initializeParser } from './parser' import * as ReservedWords from './reserved-words' import { Linter, LintingResult } from './shellcheck' +import { Formatter } from './shfmt' import { SNIPPETS } from './snippets' import { BashCompletionItem, CompletionItemDataType } from './types' import { uniqueBasedOnHash } from './util/array' @@ -35,6 +36,7 @@ export default class BashServer { private documents: LSP.TextDocuments = new LSP.TextDocuments(TextDocument) private executables: Executables private linter?: Linter + private formatter?: Formatter private workspaceFolder: string | null private uriToCodeActions: { [uri: string]: LintingResult['codeActions'] | undefined @@ -46,6 +48,7 @@ export default class BashServer { connection, executables, linter, + formatter, workspaceFolder, }: { analyzer: Analyzer @@ -53,6 +56,7 @@ export default class BashServer { connection: LSP.Connection executables: Executables linter?: Linter + formatter?: Formatter workspaceFolder: string | null }) { this.analyzer = analyzer @@ -60,6 +64,7 @@ export default class BashServer { this.connection = connection this.executables = executables this.linter = linter + this.formatter = formatter this.workspaceFolder = workspaceFolder this.config = {} as any // NOTE: configured in updateConfiguration this.updateConfiguration(config.getDefaultConfiguration(), true) @@ -130,6 +135,7 @@ export default class BashServer { workDoneProgress: false, }, renameProvider: { prepareProvider: true }, + documentFormattingProvider: true, } } @@ -172,6 +178,7 @@ export default class BashServer { connection.onWorkspaceSymbol(this.onWorkspaceSymbol.bind(this)) connection.onPrepareRename(this.onPrepareRename.bind(this)) connection.onRenameRequest(this.onRenameRequest.bind(this)) + connection.onDocumentFormatting(this.onDocumentFormatting.bind(this)) /** * The initialized notification is sent from the client to the server after @@ -272,6 +279,14 @@ export default class BashServer { this.linter = new Linter({ executablePath: shellcheckPath }) } + const shfmtPath = this.config.shfmt?.path + if (!shfmtPath) { + logger.info('Shfmt formatting is disabled as "shfmt.path" was not set') + this.formatter = undefined + } else { + this.formatter = new Formatter({ executablePath: shfmtPath }) + } + this.analyzer.setEnableSourceErrorDiagnostics( this.config.enableSourceErrorDiagnostics, ) @@ -806,6 +821,26 @@ export default class BashServer { } return edits } + + private async onDocumentFormatting( + params: LSP.DocumentFormattingParams, + ): Promise { + if (this.formatter) { + try { + const document = this.documents.get(params.textDocument.uri) + if (!document) { + logger.error(`Error getting document: ${params.textDocument.uri}`) + return null + } + + return await this.formatter.format(document, params.options, this.config.shfmt) + } catch (err) { + logger.error(`Error while formatting: ${err}`) + } + } + + return null + } } /** diff --git a/server/src/shfmt/__tests__/index.test.ts b/server/src/shfmt/__tests__/index.test.ts new file mode 100644 index 000000000..3bc3e3c3a --- /dev/null +++ b/server/src/shfmt/__tests__/index.test.ts @@ -0,0 +1,425 @@ +import { FormattingOptions } from 'vscode-languageserver/node' +import { TextDocument } from 'vscode-languageserver-textdocument' + +import { FIXTURE_DOCUMENT, FIXTURE_FOLDER } from '../../../../testing/fixtures' +import { Logger } from '../../util/logger' +import { Formatter } from '../index' + +jest.spyOn(Logger.prototype, 'log').mockImplementation(() => { + // noop +}) +const loggerWarn = jest.spyOn(Logger.prototype, 'warn') + +const FIXTURE_DOCUMENT_URI = `file://${FIXTURE_FOLDER}/foo.sh` +function textToDoc(txt: string) { + return TextDocument.create(FIXTURE_DOCUMENT_URI, 'bar', 0, txt) +} + +async function getFormattingResult({ + document, + executablePath = 'shfmt', + formatOptions, + shfmtConfig, +}: { + document: TextDocument + executablePath?: string + formatOptions?: FormattingOptions + shfmtConfig?: Record +}): Promise<[Awaited>, Formatter]> { + const formatter = new Formatter({ + executablePath, + }) + const result = await formatter.format(document, formatOptions, shfmtConfig) + return [result, formatter] +} + +describe('formatter', () => { + it('defaults canFormat to true', () => { + expect(new Formatter({ executablePath: 'foo' }).canFormat).toBe(true) + }) + + it('should set canFormat to false when formatting fails', async () => { + const [result, formatter] = await getFormattingResult({ + document: textToDoc(''), + executablePath: 'foo', + }) + + expect(result).toEqual([]) + + expect(formatter.canFormat).toBe(false) + expect(loggerWarn).toBeCalledWith( + expect.stringContaining( + 'Shfmt: disabling formatting as no executable was found at path', + ), + ) + }) + + it('should format when shfmt is present', async () => { + const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format using tabs when insertSpaces is false', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 4, insertSpaces: false }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format using spaces when insertSpaces is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 3, insertSpaces: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with operators at the start of the line when binaryNextLine is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { binaryNextLine: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary \\ + && echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with case patterns indented when caseIndent is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { caseIndent: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with function opening braces on a separate line when funcNextLine is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { funcNextLine: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects >/dev/null + + function next() + { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with redirect operators followed by a space when spaceRedirects is true', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { spaceRedirects: true }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary && + echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects > /dev/null + + function next() { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with all options enabled when multiple config settings are combined', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: { + binaryNextLine: true, + caseIndent: true, + funcNextLine: true, + spaceRedirects: true, + }, + }) + expect(result).toMatchInlineSnapshot(` + [ + { + "newText": "#!/bin/bash + set -ueo pipefail + + if [ -z "$arg" ]; then + echo indent + fi + + echo binary \\ + && echo next line + + case "$arg" in + a) + echo case indent + ;; + esac + + echo space redirects > /dev/null + + function next() + { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) +}) diff --git a/server/src/shfmt/index.ts b/server/src/shfmt/index.ts new file mode 100644 index 000000000..d10f5033a --- /dev/null +++ b/server/src/shfmt/index.ts @@ -0,0 +1,115 @@ +import { spawn } from 'child_process' +import * as LSP from 'vscode-languageserver/node' +import { TextDocument, TextEdit } from 'vscode-languageserver-textdocument' + +import { logger } from '../util/logger' + +type FormatterOptions = { + executablePath: string + cwd?: string +} + +export class Formatter { + private cwd: string + public executablePath: string + private _canFormat: boolean + + constructor({ cwd, executablePath }: FormatterOptions) { + this._canFormat = true + this.cwd = cwd || process.cwd() + this.executablePath = executablePath + } + + public get canFormat(): boolean { + return this._canFormat + } + + public async format( + document: TextDocument, + formatOptions?: LSP.FormattingOptions | null, + shfmtConfig?: Record | null, + ): Promise { + if (!this._canFormat) { + return [] + } + + return this.executeFormat(document, formatOptions, shfmtConfig) + } + + private async executeFormat( + document: TextDocument, + formatOptions?: LSP.FormattingOptions | null, + shfmtConfig?: Record | null, + ): Promise { + const documentText = document.getText() + + const result = await this.runShfmt(documentText, formatOptions, shfmtConfig) + + if (!this._canFormat) { + return [] + } + + return [ + { + range: LSP.Range.create( + LSP.Position.create(0, 0), + LSP.Position.create(Number.MAX_VALUE, Number.MAX_VALUE), + ), + newText: result, + }, + ] + } + + private async runShfmt( + documentText: string, + formatOptions?: LSP.FormattingOptions | null, + shfmtConfig?: Record | null, + ): Promise { + const indentation: number = formatOptions?.insertSpaces ? formatOptions.tabSize : 0 + const args: string[] = [`-i=${indentation}`] // --indent + if (shfmtConfig?.binaryNextLine) args.push('-bn') // --binary-next-line + if (shfmtConfig?.caseIndent) args.push('-ci') // --case-indent + if (shfmtConfig?.funcNextLine) args.push('-fn') // --func-next-line + if (shfmtConfig?.spaceRedirects) args.push('-sr') // --space-redirects + + logger.debug(`Shfmt: running "${this.executablePath} ${args.join(' ')}"`) + + let out = '' + let err = '' + const proc = new Promise((resolve, reject) => { + const proc = spawn(this.executablePath, [...args, '-'], { cwd: this.cwd }) + proc.on('error', reject) + proc.on('close', resolve) + proc.stdout.on('data', (data) => (out += data)) + proc.stderr.on('data', (data) => (err += data)) + proc.stdin.on('error', () => { + // NOTE: Ignore STDIN errors in case the process ends too quickly, before we try to + // write. If we write after the process ends without this, we get an uncatchable EPIPE. + // This is solved in Node >= 15.1 by the "on('spawn', ...)" event, but we need to + // support earlier versions. + }) + proc.stdin.end(documentText) + }) + + // NOTE: do we care about exit code? 0 means "ok", 1 possibly means "errors", + // but the presence of parseable errors in the output is also sufficient to + // distinguish. + let exit + try { + exit = await proc + } catch (e) { + // TODO: we could do this up front? + if ((e as any).code === 'ENOENT') { + // shfmt path wasn't found, don't try to format any more: + logger.warn( + `Shfmt: disabling formatting as no executable was found at path '${this.executablePath}'`, + ) + this._canFormat = false + return '' + } + throw new Error(`Shfmt: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`) + } + + return out + } +} diff --git a/testing/fixtures.ts b/testing/fixtures.ts index 009174836..2bee6ef8f 100644 --- a/testing/fixtures.ts +++ b/testing/fixtures.ts @@ -34,6 +34,7 @@ export const FIXTURE_URI = { 'shellcheck', 'shell-directive.bash', )}`, + SHFMT: `file://${path.join(FIXTURE_FOLDER, 'shfmt.sh')}`, SOURCING: `file://${path.join(FIXTURE_FOLDER, 'sourcing.sh')}`, SOURCING2: `file://${path.join(FIXTURE_FOLDER, 'sourcing2.sh')}`, RENAMING: `file://${path.join(FIXTURE_FOLDER, 'renaming.sh')}`, diff --git a/testing/fixtures/shfmt.sh b/testing/fixtures/shfmt.sh new file mode 100644 index 000000000..ff14200f8 --- /dev/null +++ b/testing/fixtures/shfmt.sh @@ -0,0 +1,21 @@ +#!/bin/bash +set -ueo pipefail + +if [ -z "$arg" ]; then +echo indent +fi + +echo binary&& +echo next line + +case "$arg" in +a) +echo case indent +;; +esac + +echo space redirects> /dev/null + +function next(){ +echo line +} diff --git a/vscode-client/__tests__/config.test.ts b/vscode-client/__tests__/config.test.ts index d1c753fb7..d6dfea9ab 100644 --- a/vscode-client/__tests__/config.test.ts +++ b/vscode-client/__tests__/config.test.ts @@ -4,6 +4,18 @@ import { LOG_LEVELS } from '../../server/src/util/logger' const defaultConfig = getDefaultConfiguration() +function flattenObjectKeys(obj: Record, prefix = '') { + return Object.keys(obj).reduce((flattenedKeys: string[], key) => { + const pre = prefix.length ? `${prefix}.` : '' + if (typeof obj[key] === 'object' && obj[key] !== null && !Array.isArray(obj[key])) { + flattenedKeys.push(...flattenObjectKeys(obj[key], pre + key)) + } else { + flattenedKeys.push(pre + key) + } + return flattenedKeys + }, []) +} + describe('config', () => { const configProperties = packageJson.contributes.configuration.properties @@ -18,7 +30,7 @@ describe('config', () => { .map((key) => key.replace(/^bashIde\./, '')) .sort() - const defaultConfigKeys = Object.keys(defaultConfig).sort() + const defaultConfigKeys = flattenObjectKeys(defaultConfig).sort() expect(configKeys).toEqual(defaultConfigKeys) }) diff --git a/vscode-client/package.json b/vscode-client/package.json index 20e0c6539..7c164ccf5 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -77,6 +77,31 @@ "type": "string", "default": "", "description": "Additional ShellCheck arguments. Note that we already add the following arguments: --shell, --format, --external-sources." + }, + "bashIde.shfmt.path": { + "type": "string", + "default": "shfmt", + "description": "Controls the executable used for Shfmt formatting. An empty string will disable formatting." + }, + "bashIde.shfmt.binaryNextLine": { + "type": "boolean", + "default": false, + "description": "Allow boolean operators (like && and ||) to start a line." + }, + "bashIde.shfmt.caseIndent": { + "type": "boolean", + "default": false, + "description": "Indent patterns in case statements." + }, + "bashIde.shfmt.funcNextLine": { + "type": "boolean", + "default": false, + "description": "Place function opening braces on a separate line." + }, + "bashIde.shfmt.spaceRedirects": { + "type": "boolean", + "default": false, + "description": "Follow redirection operators with a space." } } } diff --git a/vscode-client/src/extension.ts b/vscode-client/src/extension.ts index 2f2ab3af9..dd65470f9 100644 --- a/vscode-client/src/extension.ts +++ b/vscode-client/src/extension.ts @@ -70,6 +70,9 @@ export async function activate(context: ExtensionContext) { } } -export function deactivate() { +export function deactivate(): Thenable | undefined { + if (!client) { + return undefined + } return client.stop() }