From 2c5155e43d030e1325c3c2765d8f492024b02fd9 Mon Sep 17 00:00:00 2001
From: Vidar Holen <vidar@vidarholen.net>
Date: Sun, 14 Apr 2024 18:47:19 -0700
Subject: [PATCH] Warn about capturing the output of redirected commands.

---
 CHANGELOG.md                |  1 +
 src/ShellCheck/Analytics.hs | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b40245a51..25fc68887 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Git
 ### Added
+- SC2327/SC2328: Warn about capturing the output of redirected commands.
 ### Fixed
 
 
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index f37ac1d73..8b74ac669 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -204,6 +204,7 @@ nodeChecks = [
     ,checkUnnecessaryArithmeticExpansionIndex
     ,checkUnnecessaryParens
     ,checkPlusEqualsNumber
+    ,checkExpansionWithRedirection
     ]
 
 optionalChecks = map fst optionalTreeChecks
@@ -5040,5 +5041,46 @@ checkPlusEqualsNumber params t =
             isUnquotedNumber || isNumericalVariableName || isNumericalVariableExpansion
 
 
+
+prop_checkExpansionWithRedirection1 = verify checkExpansionWithRedirection "var=$(foo > bar)"
+prop_checkExpansionWithRedirection2 = verify checkExpansionWithRedirection "var=`foo 1> bar`"
+prop_checkExpansionWithRedirection3 = verify checkExpansionWithRedirection "var=${ foo >> bar; }"
+prop_checkExpansionWithRedirection4 = verify checkExpansionWithRedirection "var=$(foo | bar > baz)"
+prop_checkExpansionWithRedirection5 = verifyNot checkExpansionWithRedirection "stderr=$(foo 2>&1 > /dev/null)"
+prop_checkExpansionWithRedirection6 = verifyNot checkExpansionWithRedirection "var=$(foo; bar > baz)"
+prop_checkExpansionWithRedirection7 = verifyNot checkExpansionWithRedirection "var=$(foo > bar; baz)"
+prop_checkExpansionWithRedirection8 = verifyNot checkExpansionWithRedirection "var=$(cat <&3)"
+checkExpansionWithRedirection params t =
+    case t of
+        T_DollarExpansion id [cmd] -> check id cmd
+        T_Backticked id [cmd] -> check id cmd
+        T_DollarBraceCommandExpansion id [cmd] -> check id cmd
+        _ -> return ()
+  where
+    check id pipe =
+        case pipe of
+            (T_Pipeline _ _ t@(_:_)) -> checkCmd id (last t)
+            _ -> return ()
+
+    checkCmd captureId (T_Redirecting _ redirs _) = walk captureId redirs
+
+    walk captureId [] = return ()
+    walk captureId (t:rest) =
+        case t of
+            T_FdRedirect _ _ (T_IoDuplicate _ _ "1") -> return ()
+            T_FdRedirect id "1" (T_IoDuplicate _ _ _) -> return ()
+            T_FdRedirect id "" (T_IoDuplicate _ op _) | op `elem` [T_GREATAND (Id 0), T_Greater (Id 0)] -> emit id captureId True
+            T_FdRedirect id str (T_IoFile _ op file) | str `elem` ["", "1"] && op `elem` [ T_DGREAT (Id 0), T_Greater (Id 0) ]  ->
+                if getLiteralString file == Just "/dev/null"
+                then emit id captureId False
+                else emit id captureId True
+            _ -> walk captureId rest
+
+    emit redirectId captureId suggestTee = do
+        warn captureId 2327 "This command substitution will be empty because the command's output gets redirected away."
+        err redirectId 2328 $ "This redirection takes output away from the command substitution" ++ if suggestTee then " (use tee to duplicate)." else "."
+
+
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])