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

Command substitution in an if condition masking return values #1510

Open
2 tasks done
yuya-takeyama opened this issue Mar 7, 2019 · 3 comments
Open
2 tasks done

Command substitution in an if condition masking return values #1510

yuya-takeyama opened this issue Mar 7, 2019 · 3 comments

Comments

@yuya-takeyama
Copy link

For new checks and feature suggestions

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

This is a script hiding return value:

#!/bin/bash

set -eu
set -o pipefail

if [ -z "$(ls ./tmp)" ]; then
  echo "./tmp is empty"
else
  echo "./tmp is NOT empty"
fi

echo "script completed"

It doesn't stop the execution even if ./tmp doesn't exist. (unexpected)

$ bash before.sh
ls: ./tmp: No such file or directory
./tmp is empty
script completed

Here's a fixed version:

--- before.sh	2019-03-08 00:28:48.000000000 +0900
+++ after.sh	2019-03-08 00:28:04.000000000 +0900
@@ -3,7 +3,8 @@
 set -eu
 set -o pipefail
 
-if [ -z "$(ls ./tmp)" ]; then
+ls_result=$(ls ./tmp)
+if [ -z "$ls_result" ]; then
   echo "./tmp is empty"
 else
   echo "./tmp is NOT empty"
#!/bin/bash

set -eu
set -o pipefail

ls_result=$(ls ./tmp)
if [ -z "$ls_result" ]; then
  echo "./tmp is empty"
else
  echo "./tmp is NOT empty"
fi

echo "script completed"

It stops at the failure of ls. (expected)

$ bash after.sh
ls: ./tmp: No such file or directory

Here's what shellcheck currently says:

No issues detected!

Here's what I wanted or expected to see:

To be honest, I don't understand the exact reasons that the former script completes running the whole script.
But I think it's helpful if it could detect such a problem.

@yuya-takeyama yuya-takeyama changed the title Command substitution in an if statement masking return values Command substitution in an if condition masking return values Mar 7, 2019
@dseynhae
Copy link

dseynhae commented Sep 8, 2019

I stumble with the same problem on my Windows 10 PC/WSL:
The minimal code to reproduce it:

#!/bin/bash
if ! _exe=$(command -v dummy); then
  echo "_exe ($_exe) does not exist"
fi
  • I have installed shellcheck 0.6.0 from chocolatey. In my local environment (no remote session to WSL), there are no issues reported.

  • However, if I remote into my WSL (and I made sure that there is no shellchecker installed in my Ubuntu18.04LTS WSL console), the Windows shellchecker complains:

/!\ Syntax error: expected "word" somewhere in the file [xx,yy]
  • I see the same problem with the shellchecker on WSL:

sudo apt install shellchecker from the Bionic repository installs an older version (with this error).
sudo apt install ./shellcheck_0.5.0-3_amd64.deb also shows the error.

It looks like the shellchecker parser is expecting a simple command with local variable assignments (and complains that the actual word corresponding to the command is missing), and doesn't understand that an assignment with a command substitution is an expression in itself, returning an exit code.

Except, it works online, and on Windows 10 with 6.0.0 (when NOT using a remote session with WSL).

@junkblocker
Copy link

Just to make it not sounds like it's a WSL specific problem (which it couldn't be), this throws a warning on *nix (not just on WSL)

if ! target="$(/usr/bin/readlink "$HOME/.cache")" || [[ -z "$target" ]]; then
    :
fi

with

Syntax error: expected "word" somewhere in the file

@DoxasticFox
Copy link
Contributor

@yuya-takeyama

An optional check was added in #2303 to detect this.

To be honest, I don't understand the exact reasons that the former script completes running the whole script.

This wiki explains set -e's quirks.

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

4 participants