From dc41f0cc5bdcf1c814b184892b20ee0d2822e95a Mon Sep 17 00:00:00 2001
From: Vidar Holen <vidar@vidarholen.net>
Date: Fri, 11 Apr 2025 14:14:09 -0700
Subject: [PATCH] Refactor checks for POSIX test flags

---
 src/ShellCheck/Checks/ShellSupport.hs | 98 +++++++++++++++------------
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs
index 2039483..c828555 100644
--- a/src/ShellCheck/Checks/ShellSupport.hs
+++ b/src/ShellCheck/Checks/ShellSupport.hs
@@ -251,48 +251,16 @@ checkBashisms = ForShell [Sh, Dash, BusyboxSh] $ \t -> do
     bashism (T_Condition id DoubleBracket _) =
         unless isBusyboxSh $ warnMsg id 3010 "[[ ]] is"
     bashism (T_HereString id _) = warnMsg id 3011 "here-strings are"
-    bashism (TC_Binary id SingleBracket op _ _)
-        | op `elem` [ "<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="] =
-            unless isDash $ warnMsg id 3012 $ "lexicographical " ++ op ++ " is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs])
-        | op `elem` [ "<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="] =
-            unless isDash $ warnMsg id 3012 $ "lexicographical " ++ op ++ " is"
-    bashism (TC_Unary id _ op _)
-        | op `elem` [ "-k", "-G", "-O" ] =
-            unless isDash $ warnMsg id 3013 $ op ++ " is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just op, _])
-        | op `elem` [ "-k", "-G", "-O" ] =
-            unless isDash $ warnMsg id 3013 $ op ++ " is"
-    bashism (TC_Unary id _ op _)
-        | op `elem` [ "-N", "-o", "-R" ] =
-            warnMsg id 3013 $ op ++ " is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just op, _])
-        | op `elem` [ "-N", "-o", "-R" ] =
-            warnMsg id 3013 $ op ++ " is"
-    bashism (TC_Binary id SingleBracket op _ _)
-        | op `elem` [ "-ot", "-nt", "-ef" ] =
-            unless isDash $ warnMsg id 3013 $ op ++ " is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs])
-        | op `elem` [ "-ot", "-nt", "-ef" ] =
-            unless isDash $ warnMsg id 3013 $ op ++ " is"
-    bashism (TC_Binary id SingleBracket "==" _ _) =
-        unless isBusyboxSh $ warnMsg id 3014 "== in place of = is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just "==", rhs]) =
-        unless isBusyboxSh $ warnMsg id 3014 "== in place of = is"
-    bashism (TC_Binary id SingleBracket "=~" _ _) =
-            warnMsg id 3015 "=~ regex matching is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just "=~", rhs]) =
-            warnMsg id 3015 "=~ regex matching is"
-    bashism (TC_Unary id SingleBracket "-v" _) =
-            warnMsg id 3016 "unary -v (in place of [ -n \"${var+x}\" ]) is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just "-v", _]) =
-            warnMsg id 3016 "unary -v (in place of [ -n \"${var+x}\" ]) is"
-    bashism (TC_Unary id _ "-a" _) =
-            warnMsg id 3017 "unary -a in place of -e is"
-    bashism (TC_Unary id _ "-o" _) =
-            warnMsg id 3062 "unary -o to check options is"
-    bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just "-a", _]) =
-            warnMsg id 3017 "unary -a in place of -e is"
+
+    bashism (TC_Binary id _ op _ _) =
+        checkTestOp bashismBinaryTestFlags op id
+    bashism (T_SimpleCommand id _ [asStr -> Just "test", lhs, asStr -> Just op, rhs]) =
+        checkTestOp bashismBinaryTestFlags op id
+    bashism (TC_Unary id _ op _) =
+        checkTestOp bashismUnaryTestFlags op id
+    bashism (T_SimpleCommand id _ [asStr -> Just "test", asStr -> Just op, _]) =
+        checkTestOp bashismUnaryTestFlags op id
+
     bashism (TA_Unary id op _)
         | op `elem` [ "|++", "|--", "++|", "--|"] =
             warnMsg id 3018 $ filter (/= '|') op ++ " is"
@@ -529,6 +497,52 @@ checkBashisms = ForShell [Sh, Dash, BusyboxSh] $ \t -> do
                 Assignment (_, _, name, _) -> name == var
                 _ -> False
 
+    checkTestOp table op id = sequence_ $ do
+        (code, shells, msg) <- Map.lookup op table
+        guard . not $ shellType params `elem` shells
+        return $ warnMsg id code (msg op)
+
+
+buildTestFlagMap list = Map.fromList $ concatMap (\(x,y) -> map (\c -> (c,y)) x) list
+bashismBinaryTestFlags = buildTestFlagMap [
+    -- ([list of applicable flags],
+    --     (error code, exempt shells, message builder :: String -> String)),
+    --
+    -- Distinct error codes allow the wiki to give more helpful, targeted
+    -- information.
+    (["<", ">", "\\<", "\\>", "<=", ">=", "\\<=", "\\>="],
+        (3012, [Dash, BusyboxSh], \op -> "lexicographical " ++ op ++ " is")),
+    (["-nt", "-ot", "-ef"],
+        (3013, [Dash, BusyboxSh], \op -> op ++ " is")),
+    (["=="],
+        (3014, [BusyboxSh], \op -> op ++ " in place of = is")),
+    (["=~"],
+        (3015, [], \op -> op ++ " regex matching is")),
+
+    ([], (0,[],const ""))
+  ]
+bashismUnaryTestFlags = buildTestFlagMap [
+    (["-v"],
+        (3016, [], \op -> "test " ++ op ++ " (in place of [ -n \"${var+x}\" ]) is")),
+    (["-a"],
+        (3017, [], \op -> "unary " ++ op ++ " in place of -e is")),
+    (["-o"],
+        (3062, [], \op -> "test " ++ op ++ " to check options is")),
+    (["-R"],
+        (3063, [], \op -> "test " ++ op ++ " and namerefs in general are")),
+    (["-N"],
+        (3064, [], \op -> "test " ++ op ++ " is")),
+    (["-k"],
+        (3065, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")),
+    (["-G"],
+        (3066, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")),
+    (["-O"],
+        (3067, [Dash, BusyboxSh], \op -> "test " ++ op ++ " is")),
+
+    ([], (0,[],const ""))
+  ]
+
+
 prop_checkEchoSed1 = verify checkEchoSed "FOO=$(echo \"$cow\" | sed 's/foo/bar/g')"
 prop_checkEchoSed1b = verify checkEchoSed "FOO=$(sed 's/foo/bar/g' <<< \"$cow\")"
 prop_checkEchoSed2 = verify checkEchoSed "rm $(echo $cow | sed -e 's,foo,bar,')"