Merge pull request #694 from bash-lsp/skip-shellcheck-for-zsh

Skip linting zsh files

Author: skovhus

Diff for: server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 4.5.4
+
+- Skip running ShellCheck for unsupported zsh files. We will still try for files without a shebang and without a known file extension. https://github.com/bash-lsp/bash-language-server/pull/694
+
 ## 4.5.3
 
 - Fix issue where some features would work as expected in case of a syntax issue https://github.com/bash-lsp/bash-language-server/pull/691

Diff for: server/package.json

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

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

@@ -71,37 +71,49 @@ export class Linter {
     sourcePaths: string[],
     additionalShellCheckArguments: string[] = [],
   ): Promise<LintingResult> {
+    const documentText = document.getText()
+
+    const shellDialect = guessShellDialect({
+      documentText,
+      uri: document.uri,
+    })
+
+    if (shellDialect && !SUPPORTED_BASH_DIALECTS.includes(shellDialect)) {
+      // We found a dialect that isn't supported by ShellCheck.
+      return { diagnostics: [], codeActions: [] }
+    }
+
+    // NOTE: that ShellCheck actually does shebang parsing, but we manually
+    // do it here in order to fallback to bash for files without a shebang.
+    // This enables parsing files with a bash syntax, but could yield false positives.
+    const shellName =
+      shellDialect && SUPPORTED_BASH_DIALECTS.includes(shellDialect)
+        ? shellDialect
+        : 'bash'
+
     const result = await this.runShellCheck(
-      document,
+      documentText,
+      shellName,
       [...sourcePaths, dirname(fileURLToPath(document.uri))],
       additionalShellCheckArguments,
     )
+
     if (!this._canLint) {
       return { diagnostics: [], codeActions: [] }
     }
 
     // Clean up the debounced function
     delete this.uriToDebouncedExecuteLint[document.uri]
 
-    return mapShellCheckResult({ document, result })
+    return mapShellCheckResult({ uri: document.uri, result })
   }
 
   private async runShellCheck(
-    document: TextDocument,
+    documentText: string,
+    shellName: string,
     sourcePaths: string[],
     additionalArgs: string[] = [],
   ): Promise<ShellCheckResult> {
-    const documentText = document.getText()
-
-    const { shellDialect } = analyzeShebang(documentText)
-    // NOTE: that ShellCheck actually does shebang parsing, but we manually
-    // do it here in order to fallback to bash. This enables parsing files
-    // with a bash syntax.
-    const shellName =
-      shellDialect && SUPPORTED_BASH_DIALECTS.includes(shellDialect)
-        ? shellDialect
-        : 'bash'
-
     const sourcePathsArgs = sourcePaths
       .map((folder) => folder.trim())
       .filter((folderName) => folderName)
@@ -169,10 +181,10 @@ export class Linter {
 }
 
 function mapShellCheckResult({
-  document,
+  uri,
   result,
 }: {
-  document: TextDocument
+  uri: string
   result: ShellCheckResult
 }): {
   diagnostics: LSP.Diagnostic[]
@@ -208,8 +220,8 @@ function mapShellCheckResult({
 
     const codeAction = CodeActionProvider.getCodeAction({
       comment,
-      document,
       diagnostics: [diagnostic],
+      uri,
     })
 
     if (codeAction) {
@@ -230,12 +242,12 @@ function mapShellCheckResult({
 class CodeActionProvider {
   public static getCodeAction({
     comment,
-    document,
     diagnostics,
+    uri,
   }: {
     comment: ShellCheckComment
-    document: TextDocument
     diagnostics: LSP.Diagnostic[]
+    uri: string
   }): LSP.CodeAction | null {
     const { code, fix } = comment
     if (!fix || fix.replacements.length === 0) {
@@ -257,7 +269,7 @@ class CodeActionProvider {
       diagnostics,
       edit: {
         changes: {
-          [document.uri]: edits,
+          [uri]: edits,
         },
       },
       kind: LSP.CodeActionKind.QuickFix,
@@ -283,3 +295,7 @@ class CodeActionProvider {
     }
   }
 }
+
+function guessShellDialect({ documentText, uri }: { documentText: string; uri: string }) {
+  return uri.endsWith('.zsh') ? 'zsh' : analyzeShebang(documentText).shellDialect
+}

Diff for: server/src/util/__tests__/shebang.test.ts

@@ -12,10 +12,10 @@ describe('analyzeShebang', () => {
     })
   })
 
-  it('returns no shell dialect for unsupported shell "#!/usr/bin/zsh"', () => {
-    expect(analyzeShebang('#!/usr/bin/zsh')).toEqual({
+  it('returns no shell dialect for unsupported shell "#!/usr/bin/fish"', () => {
+    expect(analyzeShebang('#!/usr/bin/fish')).toEqual({
       shellDialect: null,
-      shebang: '/usr/bin/zsh',
+      shebang: '/usr/bin/fish',
     })
   })
 
@@ -30,6 +30,7 @@ describe('analyzeShebang', () => {
     ['#! /bin/bash', 'bash'],
     ['#! /bin/dash', 'dash'],
     ['#!/usr/bin/bash', 'bash'],
+    ['#!/usr/bin/zsh', 'zsh'],
   ])('returns a bash dialect for %p', (command, expectedDialect) => {
     expect(analyzeShebang(command).shellDialect).toBe(expectedDialect)
     expect(analyzeShebang(`${command} `).shellDialect).toBe(expectedDialect)

Diff for: server/src/util/shebang.ts

@@ -1,8 +1,9 @@
 const SHEBANG_REGEXP = /^#!(.*)/
 const SHELL_REGEXP = /bin[/](?:env )?(\w+)/
 
-const BASH_DIALECTS = ['sh', 'bash', 'dash', 'ksh'] as const
-type SupportedBashDialect = (typeof BASH_DIALECTS)[number]
+// Non exhaustive list of bash dialects that we potentially could support and try to analyze.
+const BASH_DIALECTS = ['sh', 'bash', 'dash', 'ksh', 'zsh', 'csh', 'ash'] as const
+type BashDialect = (typeof BASH_DIALECTS)[number]
 
 export function getShebang(fileContent: string): string | null {
   const match = SHEBANG_REGEXP.exec(fileContent)
@@ -13,7 +14,7 @@ export function getShebang(fileContent: string): string | null {
   return match[1].trim()
 }
 
-export function getShellDialect(shebang: string): SupportedBashDialect | null {
+export function getShellDialect(shebang: string): BashDialect | null {
   const match = SHELL_REGEXP.exec(shebang)
   if (match && match[1]) {
     const bashDialect = match[1].trim() as any
@@ -26,7 +27,7 @@ export function getShellDialect(shebang: string): SupportedBashDialect | null {
 }
 
 export function analyzeShebang(fileContent: string): {
-  shellDialect: SupportedBashDialect | null
+  shellDialect: BashDialect | null
   shebang: string | null
 } {
   const shebang = getShebang(fileContent)

Diff for: testing/fixtures/basic-zsh.zsh

@@ -0,0 +1,14 @@
+#!/usr/bin/env zsh
+# Path to your oh-my-zsh installation.
+export ZSH=~/.oh-my-zsh
+
+# Uncomment the following line if you want to disable marking untracked files
+# under VCS as dirty. This makes repository status check for large repositories
+# much, much faster.
+DISABLE_UNTRACKED_FILES_DIRTY="true"
+
+fpath=(/usr/local/share/zsh-completions $fpath)
+
+export CLICOLOR=1
+
+echo $DISABLE_UNTRACKED_FILES_DIRTY