Skip to content

Skip linting zsh files #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions server/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
56 changes: 36 additions & 20 deletions server/src/shellcheck/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -169,10 +181,10 @@ export class Linter {
}

function mapShellCheckResult({
document,
uri,
result,
}: {
document: TextDocument
uri: string
result: ShellCheckResult
}): {
diagnostics: LSP.Diagnostic[]
Expand Down Expand Up @@ -208,8 +220,8 @@ function mapShellCheckResult({

const codeAction = CodeActionProvider.getCodeAction({
comment,
document,
diagnostics: [diagnostic],
uri,
})

if (codeAction) {
Expand All @@ -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) {
Expand All @@ -257,7 +269,7 @@ class CodeActionProvider {
diagnostics,
edit: {
changes: {
[document.uri]: edits,
[uri]: edits,
},
},
kind: LSP.CodeActionKind.QuickFix,
Expand All @@ -283,3 +295,7 @@ class CodeActionProvider {
}
}
}

function guessShellDialect({ documentText, uri }: { documentText: string; uri: string }) {
return uri.endsWith('.zsh') ? 'zsh' : analyzeShebang(documentText).shellDialect
}
7 changes: 4 additions & 3 deletions server/src/util/__tests__/shebang.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
})
})

Expand All @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions server/src/util/shebang.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions testing/fixtures/basic-zsh.zsh
Original file line number Diff line number Diff line change
@@ -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