Merge pull request #1136 from chris-reeves/320/format-using-shfmt

Support format using shfmt (fixes #320)

Author: skovhus

.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:

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

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

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": {

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": [],
   },

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()
 

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,
+        },
       }
     `)
   })

server/src/__tests__/server.test.ts

@@ -85,6 +85,7 @@ describe('server', () => {
           ],
         },
         "definitionProvider": true,
+        "documentFormattingProvider": true,
         "documentHighlightProvider": true,
         "documentSymbolProvider": true,
         "hoverProvider": true,

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<typeof ConfigSchema>
@@ -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)

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<TextDocument> = 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,20 +48,23 @@ export default class BashServer {
     connection,
     executables,
     linter,
+    formatter,
     workspaceFolder,
   }: {
     analyzer: Analyzer
     capabilities: LSP.ClientCapabilities
     connection: LSP.Connection
     executables: Executables
     linter?: Linter
+    formatter?: Formatter
     workspaceFolder: string | null
   }) {
     this.analyzer = analyzer
     this.clientCapabilities = capabilities
     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<LSP.TextEdit[] | null> {
+    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
+  }
 }
 
 /**