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

Feature Request: Provide an option to specify the fallback shell type #2971

Open
2 tasks done
moonfruit opened this issue Apr 24, 2024 · 9 comments
Open
2 tasks done

Comments

@moonfruit
Copy link

For new checks and feature suggestions

Description

Some tools like bash-language-server detect the shell type themselves and add the option --shell=SHELLNAME to shellcheck.
But this option will overwrite shellcheck directive for type in file or .shellcheckrc.

I think a new option should be provided to specify a fallback shell type.

@brother
Copy link
Collaborator

brother commented Apr 24, 2024

Not sure this is a shellcheck problem really.

Would it be possible to add the shell directive at the top of the file?

brother ~$ cat /tmp/test.sh 
#!/bin/sh

myvar=(1 2 3)

echo "${myvar[1]}"
brother ~$ shellcheck --norc /tmp/test.sh 

In /tmp/test.sh line 3:
myvar=(1 2 3)
      ^-----^ SC3030 (warning): In POSIX sh, arrays are undefined.


In /tmp/test.sh line 5:
echo "${myvar[1]}"
      ^---------^ SC3054 (warning): In POSIX sh, array references are undefined.

For more information:
  https://www.shellcheck.net/wiki/SC3030 -- In POSIX sh, arrays are undefined.
  https://www.shellcheck.net/wiki/SC3054 -- In POSIX sh, array references are...
brother ~$ shellcheck --norc --shell=bash /tmp/test.sh 
brother ~$ cat /tmp/test.sh 
#!/bin/sh
# shellcheck shell=bash

myvar=(1 2 3)

echo "${myvar[1]}"
brother ~$ shellcheck --norc --shell=bash /tmp/test.sh 
brother ~$ shellcheck --norc /tmp/test.sh 

@moonfruit
Copy link
Author

moonfruit commented Apr 24, 2024

For example:

test.sh:

#!/bin/sh
# shellcheck shell=bash
echo -e "abc"

And check with shellcheck

$ shellcheck test.sh
$ shellcheck --shell=sh test.sh

In test.sh line 3:
echo -e "abc"
     ^-- SC3037 (warning): In POSIX sh, echo flags are undefined.

For more information:
  https://www.shellcheck.net/wiki/SC3037 -- In POSIX sh, echo flags are undef...

You can find that the command line option override the shell directive defined in the file.

@moonfruit
Copy link
Author

moonfruit commented Apr 24, 2024

bash-language-server always sets --shell= to what it thinks the shell is. I think it should set it to a fallback shell type to allow # shellcheck shell=bash to work. But shellcheck does not have this function.

@brother
Copy link
Collaborator

brother commented Apr 24, 2024

maybe that should stop doing such things then?

@moonfruit
Copy link
Author

Well, now that you've said that, all I can do now is post this question to bash-language-server.

@moonfruit
Copy link
Author

It seems to me that a small change on both sides would be the feature that still works. Otherwise, an external tool may be needed to parse the shellcheck directive to solve the problem more perfectly.

@brother
Copy link
Collaborator

brother commented Apr 25, 2024

I don't think this need to get all crazy but it looks like bash-lang-server is (lack of words) abusing the shell directive. We developers and and end consumers can set those options ourselves if needed.

@ale5000-git
Copy link

ale5000-git commented Apr 26, 2024

@moonfruit
My opinion: shellcheck already detect the shell by itself in multiple ways so there isn't any need for the server to suggest it (even more so if the suggestions are wrong).

@moonfruit
Copy link
Author

I'm not the maintainer of bash-language-server, I can't decide what it will do.
I just made a suggestion.

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

No branches or pull requests

3 participants