Merge pull request #1163 from chris-reeves/fix-1162/formatting-wipes-out-unparsable-files

Handle non-zero exit status when formatting using shfmt

Author: skovhus

Diff for: server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 5.3.2
+
+- Handle non-zero exit status when formatting using shfmt https://github.com/bash-lsp/bash-language-server/pull/1163
+
 ## 5.3.1
 
 - Clear diagnostics when closing document https://github.com/bash-lsp/bash-language-server/pull/1135

Diff for: server/package.json

@@ -3,7 +3,7 @@
   "description": "A language server for Bash",
   "author": "Mads Hartmann",
   "license": "MIT",
-  "version": "5.3.1",
+  "version": "5.3.2",
   "main": "./out/server.js",
   "typings": "./out/server.d.ts",
   "bin": {

Diff for: server/src/shfmt/__tests__/index.test.ts

@@ -38,7 +38,7 @@ describe('formatter', () => {
     expect(new Formatter({ executablePath: 'foo' }).canFormat).toBe(true)
   })
 
-  it('should set canFormat to false when formatting fails', async () => {
+  it('should set canFormat to false when the executable cannot be found', async () => {
     const [result, formatter] = await getFormattingResult({
       document: textToDoc(''),
       executablePath: 'foo',
@@ -54,6 +54,14 @@ describe('formatter', () => {
     )
   })
 
+  it('should throw when formatting fails', async () => {
+    expect(async () => {
+      await getFormattingResult({ document: FIXTURE_DOCUMENT.PARSE_PROBLEMS })
+    }).rejects.toThrow(
+      'Shfmt: exited with status 1: <standard input>:10:1: > must be followed by a word',
+    )
+  })
+
   it('should format when shfmt is present', async () => {
     const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT })
     expect(result).toMatchInlineSnapshot(`

Diff for: server/src/shfmt/index.ts

@@ -91,14 +91,10 @@ export class Formatter {
       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(
@@ -107,7 +103,11 @@ export class Formatter {
         this._canFormat = false
         return ''
       }
-      throw new Error(`Shfmt: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`)
+      throw new Error(`Shfmt: child process error: ${e}`)
+    }
+
+    if (exit != 0) {
+      throw new Error(`Shfmt: exited with status ${exit}: ${err}`)
     }
 
     return out