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

False positive for 1072 when there's inline disable directives #1036

Closed
1 task done
mechalynx opened this issue Oct 28, 2017 · 9 comments
Closed
1 task done

False positive for 1072 when there's inline disable directives #1036

mechalynx opened this issue Oct 28, 2017 · 9 comments

Comments

@mechalynx
Copy link

mechalynx commented Oct 28, 2017

For bugs

  • Rule Id: 1072
  • Version: local 0.4.6 / also occurs online
  • I tried on shellcheck.net and verified that this is still a problem on the latest commit

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

#! /usr/bin/env bash

if [[ "$1" =~ [0-9]+ ]]
then
  echo "$1"
# shellcheck disable=1001
elif [[ "$1" =~ [^0-9] ]]
then
  echo "$2"
fi

Here's what shellcheck currently says:

Line 3:
if [[ "$1" =~ [0-9]+ ]]
^-- SC1009: The mentioned parser error was in this if expression.
 
Line 4:
then
^-- SC1073: Couldn't parse this then clause.
 
Line 7:
elif [[ "$1" =~ [^0-9] ]]
     ^-- SC1072: Unexpected keyword/token. Fix any mentioned problems and try again.

Here's what I wanted or expected to see:

No error exists in that code. The false positive does not occur if the disable directive is missing or at the block level (outside the if or a function block).

@Nightfirecat
Copy link
Contributor

It's because the disable directive is in the wrong place. (source)

The following raises no errors:

#! /usr/bin/env bash

# shellcheck disable=1001
if [[ "$1" =~ [0-9]+ ]]
then
  echo "$1"
elif [[ "$1" =~ [^0-9] ]]
then
  echo "$2"
fi

@mechalynx
Copy link
Author

@Nightfirecat That's my point. It should be treated as referring to the line immediately following it or, if it's not supposed to ever be used there, throw an error indicating that. Instead it falsely reports a different error.

@koalaman
Copy link
Owner

I agree, you should at least be getting a warning. Scoping directives to statements wasn't as clever as I thought it would be

@mechalynx
Copy link
Author

mechalynx commented Oct 28, 2017

@koalaman I assume this might not be that easy to fix. Do you mind if I add a note to the relevant page on the wiki (the one Nightfirecat linked to) that mentions this bug and recommends avoiding directives within if clauses? I would have done so already but not sure if it's desirable on your part.

Just so people have an obvious place to look for an answer until this can be resolved.

edit - It seems it consistently throws 1072 no matter where the directive is placed within the then clause. If adding info on the Directive page is ok, perhaps it's a good idea to add it to the wiki page for 1072 as well, since that's likely the place people will be looking for an answer at.

@koalaman
Copy link
Owner

Feel free to update the wiki. I suspect it's actually relatively easy to fix (in the sense of adding a warning), but I'll have to look more into that when I get a chance.

@mechalynx
Copy link
Author

Wiki updated on the Directive, SC1072 and Parser Error pages. I hopefully made the note clear but discreet. I'll try to keep an eye out to revert the changes when you get around to fixing this.

Thanks!

@koalaman
Copy link
Owner

Now you should at least get an explicit warning rather than a weird parsing failure, both for elif/else/fi/done and for branches in statements.

@mechalynx
Copy link
Author

@koalaman I'll modify the wiki to mention that this bug will be fixed in the next version and is already fixed on shellcheck.net, until the next release. Thanks!

@LucasLarson
Copy link

I’m still able to regenerate this error on shellcheck.net with only the following code

#!/bin/sh
[ ! ]

sajith added a commit to atlanticwave-sdx/sdx-controller that referenced this issue Mar 28, 2023
There's an unhelpful hadolint error that simply says:

> Dockerfile:3 SC1072 error: Fix any mentioned problems and try again.

It is unclear what we're supposed to do here, and it could be a false
positive because of koalaman/shellcheck#1036
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