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

SC2032 and SC2033 false positives when conditionally defining function #1200

Open
1 of 2 tasks
tdmalone opened this issue May 3, 2018 · 3 comments
Open
1 of 2 tasks

Comments

@tdmalone
Copy link
Contributor

tdmalone commented May 3, 2018

For bugs

  • Rule Id (if any, e.g. SC1000): SC2032/SC2033
  • My shellcheck version (shellcheck --version or "online"): 0.4.7
  • I tried on shellcheck.net and verified that this is still a problem on the latest commit
  • It's not reproducible on shellcheck.net, but I think that's because it's an OS, configuration or encoding issue

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

#!/usr/bin/env bash

if [ "$( sudo -v )" ]; then
  sudo mv terraform /usr/bin/terraform
else
  terraform() { "${TRAVIS_BUILD_DIR}/terraform" "$@"; }
  export -f terraform
fi

Here's what shellcheck currently says:

Line 4:
  sudo mv terraform /usr/bin/terraform
          ^-- SC2033: Shell functions can't be passed to external commands.
 
Line 6:
  terraform() { "${TRAVIS_BUILD_DIR}/terraform" "$@"; }
  ^-- SC2032: Use own script or sh -c '..' to run this from sudo.

Here's what I wanted or expected to see:

No issues detected!
@tdmalone
Copy link
Contributor Author

tdmalone commented May 3, 2018

I'm open to the fact that I might be doing something wrong, but I think this is a false positive. I'm conditionally creating the function, so it shouldn't even exist at the point I'm trying to run sudo instead.

I also realise this might be very hard to detect, and I will use a directive for now.

@Dmole
Copy link

Dmole commented May 13, 2018

I think if you or some calling script wrap your whole code in brackets it will create the function and export it then check the condition so I think it's a valid warning. You should probably just re-name your function.

@ngzhian
Copy link
Contributor

ngzhian commented Jun 15, 2018

You're right, false positive. I think this is a result of the simple variable analysis shellcheck has. It figures that terraform was defined as a function, and used in sudo, even though they are in separate branches. I think you'ld have to ignore this unfortunately. I'm thinking of making the data flow analysis better, but that will take some time.

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