Skip to content
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

The "shell" directive has no effect in a shell script file #929

Closed
sprat opened this issue Aug 4, 2023 · 2 comments · Fixed by #1081
Closed

The "shell" directive has no effect in a shell script file #929

sprat opened this issue Aug 4, 2023 · 2 comments · Fixed by #1081

Comments

@sprat
Copy link

sprat commented Aug 4, 2023

Code editor

Sublime Text

Platform

Windows, Linux

Version

5.0.0

What steps will reproduce the bug?

The following script raises a shellcheck warning on the "set" line:

#!/bin/sh
# shellcheck shell=ash
set -euxo pipefail

However it should not because I explicitly defined the shell to eliminate the warning.

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

I would expect the shell directive to be taken into account, so no warning should appear on the "set" line

What do you see instead?

The shell directive is ignored

Additional information

I suspect that the problem is caused by the LSP language server passing the --shell option to the command-line which have the priority over the shell directive defined in my script file. But, AFAIK, there's no way to change/remove the value of the --shell option, right?

@sprat
Copy link
Author

sprat commented Aug 17, 2023

After diving a bit into the code, i can see that the shell directive is not used in the guessShellDialect method which is used to derive the shell that will be passed to the --shell option in the command-line. The method use either the file extension or parses the shebang to derive the shell, that's all.

I must admit that I don't understand why the language server need to detect the shell dialect instead of letting shellcheck do its job. There is a comment that attempt to give a rationale:

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

However I don't understand the argument here: if the user want his shell script to be interpreted as a bash script, he can either add a shebang or use a shellcheck directive, right?

@skovhus
Copy link
Collaborator

skovhus commented Aug 17, 2023

PR are more than welcome here. The current behaviour is not perfect, but does enable people to use ShellCheck for bash-like without adding directives. We can either include parsing directives when guessing the dialect OR simply remove the feature to enable parsing files with a bash syntax.

Also the support for enabling parsing files with a bash syntax was added before the directive parser (PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants