diff --git a/server/src/__tests__/config.test.ts b/server/src/__tests__/config.test.ts index 64f6d6b31..e8586b7a7 100644 --- a/server/src/__tests__/config.test.ts +++ b/server/src/__tests__/config.test.ts @@ -15,6 +15,7 @@ describe('ConfigSchema', () => { "shellcheckExternalSources": true, "shellcheckPath": "shellcheck", "shfmt": { + "additionalArguments": [], "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, @@ -66,6 +67,7 @@ describe('ConfigSchema', () => { "shellcheckExternalSources": true, "shellcheckPath": "", "shfmt": { + "additionalArguments": [], "binaryNextLine": true, "caseIndent": true, "funcNextLine": true, @@ -104,6 +106,7 @@ describe('getConfigFromEnvironmentVariables', () => { "shellcheckExternalSources": true, "shellcheckPath": "shellcheck", "shfmt": { + "additionalArguments": [], "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, @@ -136,6 +139,7 @@ describe('getConfigFromEnvironmentVariables', () => { "shellcheckExternalSources": true, "shellcheckPath": "", "shfmt": { + "additionalArguments": [], "binaryNextLine": false, "caseIndent": false, "funcNextLine": false, @@ -177,6 +181,7 @@ describe('getConfigFromEnvironmentVariables', () => { "shellcheckExternalSources": true, "shellcheckPath": "/path/to/shellcheck", "shfmt": { + "additionalArguments": [], "binaryNextLine": false, "caseIndent": true, "funcNextLine": false, diff --git a/server/src/config.ts b/server/src/config.ts index 408b4f667..925b34875 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -2,6 +2,49 @@ import { z } from 'zod' import { DEFAULT_LOG_LEVEL, LOG_LEVEL_ENV_VAR, LOG_LEVELS } from './util/logger' +export const ShfmtConfigSchema = z.object({ + // Controls the executable used for Shfmt formatting. An empty string will disable formatting + path: z.string().trim().default('shfmt'), + + // Additional Shfmt arguments. Note that common arguments can be configured via the other settings. + additionalArguments: z + .preprocess((arg) => { + let argsList: string[] = [] + if (typeof arg === 'string') { + argsList = arg.split(' ') + } else if (Array.isArray(arg)) { + argsList = arg as string[] + } + + return argsList.map((s) => s.trim()).filter((s) => s.length > 0) + }, z.array(z.string())) + .default([]), + + // Ignore shfmt config options in .editorconfig (always use language server config) + ignoreEditorconfig: z.boolean().default(false), + + // Language dialect to use when parsing (bash/posix/mksh/bats). + languageDialect: z.enum(['auto', 'bash', 'posix', 'mksh', 'bats']).default('auto'), + + // 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), + + // (Deprecated) Keep column alignment padding. + keepPadding: z.boolean().default(false), + + // Simplify code before formatting. + simplifyCode: z.boolean().default(false), + + // Follow redirection operators with a space. + spaceRedirects: z.boolean().default(false), +}) + export const ConfigSchema = z.object({ // Maximum number of files to analyze in the background. Set to 0 to disable background analysis. backgroundAnalysisMaxFiles: z.number().int().min(0).default(500), @@ -46,38 +89,10 @@ 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'), - - // Ignore shfmt config options in .editorconfig (always use language server config) - ignoreEditorconfig: z.boolean().default(false), - - // Language dialect to use when parsing (bash/posix/mksh/bats). - languageDialect: z.enum(['auto', 'bash', 'posix', 'mksh', 'bats']).default('auto'), - - // 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), - - // (Deprecated) Keep column alignment padding. - keepPadding: z.boolean().default(false), - - // Simplify code before formatting. - simplifyCode: z.boolean().default(false), - - // Follow redirection operators with a space. - spaceRedirects: z.boolean().default(false), - }) - .default({}), + shfmt: ShfmtConfigSchema.default({}), }) +export type ShfmtConfig = z.infer export type Config = z.infer export function getConfigFromEnvironmentVariables(): { diff --git a/server/src/shfmt/__tests__/index.test.ts b/server/src/shfmt/__tests__/index.test.ts index 6cbb56d22..e74550369 100644 --- a/server/src/shfmt/__tests__/index.test.ts +++ b/server/src/shfmt/__tests__/index.test.ts @@ -2,6 +2,7 @@ import { FormattingOptions } from 'vscode-languageserver/node' import { TextDocument } from 'vscode-languageserver-textdocument' import { FIXTURE_DOCUMENT, FIXTURE_FOLDER } from '../../../../testing/fixtures' +import { ShfmtConfig } from '../../config' import { Logger } from '../../util/logger' import { Formatter } from '../index' @@ -15,6 +16,22 @@ function textToDoc(txt: string) { return TextDocument.create(FIXTURE_DOCUMENT_URI, 'bar', 0, txt) } +/** Fills the test shfmt configuration with default values */ +function makeShfmtConfig(cfg: Partial): ShfmtConfig { + return { + path: cfg.path ?? '', + additionalArguments: cfg.additionalArguments ?? [], + ignoreEditorconfig: cfg.ignoreEditorconfig ?? false, + languageDialect: cfg.languageDialect ?? 'auto', + binaryNextLine: cfg.binaryNextLine ?? false, + caseIndent: cfg.caseIndent ?? false, + funcNextLine: cfg.funcNextLine ?? false, + keepPadding: cfg.keepPadding ?? false, + simplifyCode: cfg.simplifyCode ?? false, + spaceRedirects: cfg.spaceRedirects ?? false, + } +} + async function getFormattingResult({ document, executablePath = 'shfmt', @@ -24,7 +41,7 @@ async function getFormattingResult({ document: TextDocument executablePath?: string formatOptions?: FormattingOptions - shfmtConfig?: Record + shfmtConfig?: ShfmtConfig }): Promise<[Awaited>, Formatter]> { const formatter = new Formatter({ executablePath, @@ -66,7 +83,7 @@ describe('formatter', () => { expect(async () => { await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, - shfmtConfig: { languageDialect: 'posix' }, + shfmtConfig: makeShfmtConfig({ languageDialect: 'posix' }), }) }).rejects.toThrow( /Shfmt: exited with status 1: .*\/testing\/fixtures\/shfmt\.sh:25:14: (the "function" builtin|a command can only contain words and redirects; encountered \()/, @@ -227,7 +244,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { binaryNextLine: true }, + shfmtConfig: makeShfmtConfig({ binaryNextLine: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -279,7 +296,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { caseIndent: true }, + shfmtConfig: makeShfmtConfig({ caseIndent: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -331,7 +348,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { funcNextLine: true }, + shfmtConfig: makeShfmtConfig({ funcNextLine: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -384,7 +401,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { keepPadding: true }, + shfmtConfig: makeShfmtConfig({ keepPadding: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -436,7 +453,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { simplifyCode: true }, + shfmtConfig: makeShfmtConfig({ simplifyCode: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -488,7 +505,7 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { spaceRedirects: true }, + shfmtConfig: makeShfmtConfig({ spaceRedirects: true }), }) expect(result).toMatchInlineSnapshot(` [ @@ -540,14 +557,73 @@ describe('formatter', () => { const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT, formatOptions: { tabSize: 2, insertSpaces: true }, - shfmtConfig: { + shfmtConfig: makeShfmtConfig({ binaryNextLine: true, caseIndent: true, funcNextLine: true, keepPadding: true, simplifyCode: 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 one two three + echo four five six + echo seven eight nine + + [[ $simplify == "simplify" ]] + + echo space redirects > /dev/null + + function next() + { + echo line + } + ", + "range": { + "end": { + "character": 2147483647, + "line": 2147483647, + }, + "start": { + "character": 0, + "line": 0, + }, + }, + }, + ] + `) + }) + + it('should format with a combination of options and additionalArguments', async () => { + const [result] = await getFormattingResult({ + document: FIXTURE_DOCUMENT.SHFMT, + formatOptions: { tabSize: 2, insertSpaces: true }, + shfmtConfig: makeShfmtConfig({ + caseIndent: true, + keepPadding: true, + simplifyCode: true, + spaceRedirects: true, + additionalArguments: ['--binary-next-line', '--func-next-line'], + }), }) expect(result).toMatchInlineSnapshot(` [ @@ -615,11 +691,11 @@ describe('formatter', () => { }) describe('getShfmtArguments()', () => { - const lspShfmtConfig = { + const lspShfmtConfig = makeShfmtConfig({ binaryNextLine: true, funcNextLine: true, - simplifyCode: true, - } + additionalArguments: ['-s'], + }) const lspShfmtArgs = ['-bn', '-fn', '-s'] const formatOptions = { tabSize: 2, insertSpaces: true } @@ -642,7 +718,13 @@ describe('formatter', () => { it('should use language server config', async () => { expect(shfmtArgs).toEqual(expect.arrayContaining(lspShfmtArgs)) - expect(shfmtArgs.length).toEqual(4) // indentation + expect(shfmtArgs.length).toEqual(5) // indentation + }) + + it('should contain additionalArguments', async () => { + expect(shfmtArgs).toEqual( + expect.arrayContaining(lspShfmtConfig.additionalArguments), + ) }) it('should use indentation config from the editor', () => { @@ -669,7 +751,13 @@ describe('formatter', () => { it('should use language server config', () => { expect(shfmtArgs).toEqual(expect.arrayContaining(lspShfmtArgs)) - expect(shfmtArgs.length).toEqual(5) // indentation + filename + expect(shfmtArgs.length).toEqual(6) // indentation + filename + }) + + it('should contain additionalArguments', async () => { + expect(shfmtArgs).toEqual( + expect.arrayContaining(lspShfmtConfig.additionalArguments), + ) }) it('should use indentation config from the editor', () => { @@ -696,7 +784,13 @@ describe('formatter', () => { it('should use language server config', () => { expect(shfmtArgs).toEqual(expect.arrayContaining(lspShfmtArgs)) - expect(shfmtArgs.length).toEqual(5) // indentation + filename + expect(shfmtArgs.length).toEqual(6) // indentation + filename + }) + + it('should contain additionalArguments', async () => { + expect(shfmtArgs).toEqual( + expect.arrayContaining(lspShfmtConfig.additionalArguments), + ) }) it('should use indentation config from the editor', () => { @@ -776,7 +870,7 @@ describe('formatter', () => { it('should use language server config', () => { expect(shfmtArgs).toEqual(expect.arrayContaining(lspShfmtArgs)) - expect(shfmtArgs.length).toEqual(5) // indentation + filename + expect(shfmtArgs.length).toEqual(6) // indentation + filename }) it('should use indentation config from the editor', () => { diff --git a/server/src/shfmt/index.ts b/server/src/shfmt/index.ts index a230d03c7..4ca3d4b4c 100644 --- a/server/src/shfmt/index.ts +++ b/server/src/shfmt/index.ts @@ -3,6 +3,7 @@ import * as editorconfig from 'editorconfig' import * as LSP from 'vscode-languageserver/node' import { DocumentUri, TextDocument, TextEdit } from 'vscode-languageserver-textdocument' +import { ShfmtConfig } from '../config' import { logger } from '../util/logger' type FormatterOptions = { @@ -28,7 +29,7 @@ export class Formatter { public async format( document: TextDocument, formatOptions?: LSP.FormattingOptions | null, - shfmtConfig?: Record | null, + shfmtConfig?: ShfmtConfig | null, ): Promise { if (!this._canFormat) { return [] @@ -40,7 +41,7 @@ export class Formatter { private async executeFormat( document: TextDocument, formatOptions?: LSP.FormattingOptions | null, - shfmtConfig?: Record | null, + shfmtConfig?: ShfmtConfig | null, ): Promise { const result = await this.runShfmt(document, formatOptions, shfmtConfig) @@ -62,9 +63,10 @@ export class Formatter { private async getShfmtArguments( documentUri: DocumentUri, formatOptions?: LSP.FormattingOptions | null, - lspShfmtConfig?: Record | null, + lspShfmtConfig?: ShfmtConfig | null, ): Promise { - const args: string[] = [] + // User-provided additionalArguments should be added before any other arguments + const args: string[] = lspShfmtConfig?.additionalArguments ?? [] // this is the config that we'll use to build args - default to language server config let activeShfmtConfig = { ...lspShfmtConfig } @@ -139,7 +141,7 @@ export class Formatter { private async runShfmt( document: TextDocument, formatOptions?: LSP.FormattingOptions | null, - shfmtConfig?: Record | null, + shfmtConfig?: ShfmtConfig | null, ): Promise { const args = await this.getShfmtArguments(document.uri, formatOptions, shfmtConfig) diff --git a/vscode-client/package.json b/vscode-client/package.json index dd47393a9..d645647db 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -88,6 +88,11 @@ "default": "shfmt", "description": "Controls the executable used for Shfmt formatting. An empty string will disable formatting." }, + "bashIde.shfmt.additionalArguments": { + "type": "string[]", + "default": "[]", + "description": "Additional Shfmt arguments. Note that common arguments can be configured via the other settings." + }, "bashIde.shfmt.ignoreEditorconfig": { "type": "boolean", "default": false,