SC2325/SC2326: Warn about ! ! foo and foo | ! bar (fixes #2810)

3 files changed, 43 insertions, 7 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6c9513..0338f7d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,8 @@
 ## Git
 ### Added
 - SC2324: Warn when x+=1 appends instead of increments
+- SC2325: Warn about multiple `!`s in dash/sh.
+- SC2326: Warn about `foo | ! bar` in bash/dash/sh.
 
 ### Fixed
 - source statements with here docs now work correctly
diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs
index cf8acc9..c7ece1a 100644
--- a/src/ShellCheck/Checks/ShellSupport.hs
+++ b/src/ShellCheck/Checks/ShellSupport.hs
@@ -60,6 +60,8 @@ checks = [
     ,checkBraceExpansionVars
     ,checkMultiDimensionalArrays
     ,checkPS1Assignments
+    ,checkMultipleBangs
+    ,checkBangAfterPipe
     ]
 
 testChecker (ForShell _ t) =
@@ -566,5 +568,29 @@ checkPS1Assignments = ForShell [Bash] f
     escapeRegex = mkRegex "\\\\x1[Bb]|\\\\e|\x1B|\\\\033"
 
 
+prop_checkMultipleBangs1 = verify checkMultipleBangs "! ! true"
+prop_checkMultipleBangs2 = verifyNot checkMultipleBangs "! true"
+checkMultipleBangs = ForShell [Dash, Sh] f
+  where
+    f token = case token of
+        T_Banged id (T_Banged _ _) ->
+            err id 2325 "Multiple ! in front of pipelines are a bash/ksh extension. Use only 0 or 1."
+        _ -> return ()
+
+
+prop_checkBangAfterPipe1 = verify checkBangAfterPipe "true | ! true"
+prop_checkBangAfterPipe2 = verifyNot checkBangAfterPipe "true | ( ! true )"
+prop_checkBangAfterPipe3 = verifyNot checkBangAfterPipe "! ! true | true"
+checkBangAfterPipe = ForShell [Dash, Sh, Bash] f
+  where
+    f token = case token of
+        T_Pipeline _ _ cmds -> mapM_ check cmds
+        _ -> return ()
+
+    check token = case token of
+        T_Banged id _ ->
+            err id 2326 "! is not allowed in the middle of pipelines. Use command group as in cmd | { ! cmd; } if necessary."
+        _ -> return ()
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])
diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs
index 341a435..ffc58e2 100644
--- a/src/ShellCheck/Parser.hs
+++ b/src/ShellCheck/Parser.hs
@@ -2296,14 +2296,18 @@ readSource t = return t
 prop_readPipeline = isOk readPipeline "! cat /etc/issue | grep -i ubuntu"
 prop_readPipeline2 = isWarning readPipeline "!cat /etc/issue | grep -i ubuntu"
 prop_readPipeline3 = isOk readPipeline "for f; do :; done|cat"
+prop_readPipeline4 = isOk readPipeline "! ! true"
+prop_readPipeline5 = isOk readPipeline "true | ! true"
 readPipeline = do
     unexpecting "keyword/token" readKeyword
-    do
-        (T_Bang id) <- g_Bang
-        pipe <- readPipeSequence
-        return $ T_Banged id pipe
-      <|>
-        readPipeSequence
+    readBanged readPipeSequence
+
+readBanged parser = do
+    pos <- getPosition
+    (T_Bang id) <- g_Bang
+    next <- readBanged parser
+    return $ T_Banged id next
+ <|> parser
 
 prop_readAndOr = isOk readAndOr "grep -i lol foo || exit 1"
 prop_readAndOr1 = isOk readAndOr "# shellcheck disable=1\nfoo"
@@ -2359,7 +2363,7 @@ readTerm = do
 
 readPipeSequence = do
     start <- startSpan
-    (cmds, pipes) <- sepBy1WithSeparators readCommand
+    (cmds, pipes) <- sepBy1WithSeparators (readBanged readCommand)
                         (readPipe `thenSkip` (spacing >> readLineBreak))
     id <- endSpan start
     spacing
@@ -2389,6 +2393,10 @@ readCommand = choice [
     ]
 
 readCmdName = do
+    -- If the command name is `!` then
+    optional . lookAhead . try $ do
+        char '!'
+        whitespace
     -- Ignore alias suppression
     optional . try $ do
         char '\\'