Skip to content

Commit

Permalink
Supress SC2015 about A && B || C when B is a test.
Browse files Browse the repository at this point in the history
  • Loading branch information
koalaman committed May 4, 2024
1 parent 4f81dbe commit 76ff702
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
### Added
- SC2327/SC2328: Warn about capturing the output of redirected commands.
- SC2329: Warn when (non-escaping) functions are never invoked.
### Changed
- SC2015 about `A && B || C` no longer triggers when B is a test command.

This comment has been minimized.

Copy link
@rozcietrzewiacz

rozcietrzewiacz May 13, 2024

Thank you! ❤️

### Fixed
- SC2317 about unreachable commands is now less spammy for nested ones.

Expand Down
16 changes: 5 additions & 11 deletions src/ShellCheck/Analytics.hs
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,9 @@ prop_checkShorthandIf5 = verifyNot checkShorthandIf "foo && rm || printf b"
prop_checkShorthandIf6 = verifyNot checkShorthandIf "if foo && bar || baz; then true; fi"
prop_checkShorthandIf7 = verifyNot checkShorthandIf "while foo && bar || baz; do true; done"
prop_checkShorthandIf8 = verify checkShorthandIf "if true; then foo && bar || baz; fi"
checkShorthandIf params x@(T_OrIf _ (T_AndIf id _ _) (T_Pipeline _ _ t))
| not (isOk t || inCondition) =
prop_checkShorthandIf9 = verifyNot checkShorthandIf "foo && [ -x /file ] || bar"
checkShorthandIf params x@(T_OrIf _ (T_AndIf id _ b) (T_Pipeline _ _ t))
| not (isOk t || inCondition) && not (isTestCommand b) =
info id 2015 "Note that A && B || C is not if-then-else. C may run when A is true."
where
isOk [t] = isAssignment t || fromMaybe False (do
Expand Down Expand Up @@ -4197,7 +4198,7 @@ checkBadTestAndOr params t =
in
mapM_ checkTest commandWithSeps
checkTest (before, cmd, after) =
when (isTest cmd) $ do
when (isTestCommand cmd) $ do
checkPipe before
checkPipe after

Expand All @@ -4213,17 +4214,10 @@ checkBadTestAndOr params t =
T_AndIf _ _ rhs -> checkAnds id rhs
T_OrIf _ _ rhs -> checkAnds id rhs
T_Pipeline _ _ list | not (null list) -> checkAnds id (last list)
cmd -> when (isTest cmd) $
cmd -> when (isTestCommand cmd) $
errWithFix id 2265 "Use && for logical AND. Single & will background and return true." $
(fixWith [replaceEnd id params 0 "&"])

isTest t =
case t of
T_Condition {} -> True
T_SimpleCommand {} -> t `isCommand` "test"
T_Redirecting _ _ t -> isTest t
T_Annotation _ _ t -> isTest t
_ -> False

prop_checkComparisonWithLeadingX1 = verify checkComparisonWithLeadingX "[ x$foo = xlol ]"
prop_checkComparisonWithLeadingX2 = verify checkComparisonWithLeadingX "test x$foo = xlol"
Expand Down
8 changes: 8 additions & 0 deletions src/ShellCheck/AnalyzerLib.hs
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,14 @@ modifiesVariable params token name =
Assignment (_, _, n, source) -> isTrueAssignmentSource source && n == name
_ -> False

isTestCommand t =
case t of
T_Condition {} -> True
T_SimpleCommand {} -> t `isCommand` "test"
T_Redirecting _ _ t -> isTestCommand t
T_Annotation _ _ t -> isTestCommand t
T_Pipeline _ _ [t] -> isTestCommand t
_ -> False

return []
runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])

0 comments on commit 76ff702

Please sign in to comment.