koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.45k stars 1.78k forks source link

SC2001 should not be recommended when shebang is /bin/ksh #3024

Open mbbh opened 4 months ago

mbbh commented 4 months ago

For bugs

Here's a snippet or screenshot that shows the problem:


#!/bin/ksh
x="foo"
foo=$(echo "$x" | sed -e 's,foo,bar,')
echo "$foo"

Here's what shellcheck currently says:

^-- SC2001 (style): See if you can use ${variable//search/replace} instead.

Here's what I wanted or expected to see:

Shellcheck should not be suggesting this as this is a bash/zsh feature and not present in ksh

in ksh using ${a//b/c} or ${a//b/c} will produce 'bad substitution' when used in the way suggested by shellcheck.

dumitrache-adrian92 commented 3 months ago

Hey, this version of the script outputs bar for me on ksh:

#!/bin/ksh
x="foo"

foo=${x//foo/bar}
echo "$foo"

I looked a bit into this, it seems that ksh93 does have this kind of string replacement according to this documentation (described in the "Parameter expansions" section), but ksh88 does not. Can you check your version with ksh --version?

Not sure how this situation should be handled since distros like Ubuntu install ksh93 by default under /bin/ksh when you ask for ksh. I can't even find instructions for how to install ksh88.

mbbh commented 3 months ago

Hey,

thanks for looking into this. As it turns out the OpenBSD ksh is not a direct fork of the upstream ksh, but apparently some sort of rewrite, if I am understanding https://github.com/openbsd/src/blob/master/bin/ksh/README correctly. As such, the version offered by this ksh is @(#)PD KSH v5.2.14 99/07/13.2, which likely doesn't map to the version of ksh shellcheck is supporting.

the conversion isn't mentioned in OpenBSDs ksh manpage https://man.openbsd.org/ksh.1.

I should've expected this, apologies for not checking that it may be OpenBSD related in the beginning, I did not think of it.

brother commented 3 months ago

Maybe these subtle but important differnce should be mentioned somewhere more prominent. Wiki or README or man or such. I noticed that ksh88 is mentioned in the source at some points at least.

Jamie-Landeg-Jones commented 2 months ago

Sounds like "pdksh", which has a long history, and currently seems to be maintained here: https://github.com/multishell/pdksh

The readme.md goes into history/implementation details.

Jamie-Landeg-Jones commented 2 months ago

More on the whys and differences between mksh, pdksh, and ksh here:

https://www.reddit.com/r/openbsd/comments/7k7ce1/ksh_vs_pdksh_vs_mksh_vs_ksh93_vs_others/