From 1ac2c31728dd3109aea3c120d8fa021d358319af Mon Sep 17 00:00:00 2001
From: Vidar Holen <spam@vidarholen.net>
Date: Mon, 27 Jul 2020 21:28:48 -0700
Subject: [PATCH] Warn when shell functions blatantly recurse (fixes #1994)

---
 CHANGELOG.md                  |  1 +
 src/ShellCheck/Analytics.hs   | 37 +++++++++++++++++++++++++++++++++++
 src/ShellCheck/AnalyzerLib.hs |  2 ++
 3 files changed, 40 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fde8b0f..a9ea5f4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 - SC2259/SC2260: Warn when redirections override pipes
 - SC2261: Warn about multiple competing redirections
 - SC2262/SC2263: Warn about aliases declared and used in the same parsing unit
+- SC2264: Warn about wrapper functions that blatantly recurse
 
 ### Fixed
 - SC1072/SC1073 now respond to disable annotations, though ignoring parse errors
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index bba3caf..f8904be 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -191,6 +191,7 @@ nodeChecks = [
     ,checkUselessBang
     ,checkTranslatedStringVariable
     ,checkModifiedArithmeticInRedirection
+    ,checkBlatantRecursion
     ]
 
 optionalChecks = map fst optionalTreeChecks
@@ -3824,5 +3825,41 @@ groupByLink f list =
         else (reverse $ current:span) : g next [] rest
     g current span [] = [reverse (current:span)]
 
+
+prop_checkBlatantRecursion1 = verify checkBlatantRecursion ":(){ :|:& };:"
+prop_checkBlatantRecursion2 = verify checkBlatantRecursion "f() { f; }"
+prop_checkBlatantRecursion3 = verifyNot checkBlatantRecursion "f() { command f; }"
+prop_checkBlatantRecursion4 = verify checkBlatantRecursion "cd() { cd \"$lol/$1\" || exit; }"
+prop_checkBlatantRecursion5 = verifyNot checkBlatantRecursion "cd() { [ -z \"$1\" ] || cd \"$1\"; }"
+prop_checkBlatantRecursion6 = verifyNot checkBlatantRecursion "cd() { something; cd $1; }"
+prop_checkBlatantRecursion7 = verifyNot checkBlatantRecursion "cd() { builtin cd $1; }"
+checkBlatantRecursion :: Parameters -> Token -> Writer [TokenComment] ()
+checkBlatantRecursion params t =
+    case t of
+        T_Function _ _ _ name body ->
+            case getCommandSequences body of
+                    [first : _] -> checkList name first
+                    _ -> return ()
+        _ -> return ()
+  where
+    checkList :: String -> Token -> Writer [TokenComment] ()
+    checkList name t =
+        case t of
+            T_Backgrounded _ t -> checkList name t
+            T_AndIf _ lhs _ -> checkList name lhs
+            T_OrIf _ lhs _ -> checkList name lhs
+            T_Pipeline _ _ cmds -> mapM_ (checkCommand name) cmds
+            _ -> return ()
+
+    checkCommand :: String -> Token -> Writer [TokenComment] ()
+    checkCommand name cmd = sequence_ $ do
+        let (invokedM, t) = getCommandNameAndToken True cmd
+        invoked <- invokedM
+        guard $ name == invoked
+        return $
+            errWithFix (getId t) 2264
+                ("This function unconditionally re-invokes itself. Missing 'command'?")
+                (fixWith [replaceStart (getId t) params 0 $ "command "])
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])
diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs
index ab1c415..1ff50b2 100644
--- a/src/ShellCheck/AnalyzerLib.hs
+++ b/src/ShellCheck/AnalyzerLib.hs
@@ -163,6 +163,8 @@ err   id code str = addComment $ makeComment ErrorC id code str
 info  id code str = addComment $ makeComment InfoC id code str
 style id code str = addComment $ makeComment StyleC id code str
 
+errWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m ()
+errWithFix  = addCommentWithFix ErrorC
 warnWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m ()
 warnWithFix  = addCommentWithFix WarningC
 styleWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m ()