From aaffe381987cd253ae8dac4b77af08fca2d7982d Mon Sep 17 00:00:00 2001
From: "Joseph C. Sible" <josephcsible@users.noreply.github.com>
Date: Sat, 8 Feb 2020 23:06:57 -0500
Subject: [PATCH 1/3] Use the Identity monad to avoid unnecessary uses of
 fromJust

---
 src/ShellCheck/ASTLib.hs              | 5 +++--
 src/ShellCheck/AnalyzerLib.hs         | 2 +-
 src/ShellCheck/Checks/Commands.hs     | 7 ++++---
 src/ShellCheck/Checks/ShellSupport.hs | 3 ++-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs
index 03a2f9a..658ed64 100644
--- a/src/ShellCheck/ASTLib.hs
+++ b/src/ShellCheck/ASTLib.hs
@@ -25,6 +25,7 @@ import Control.Monad.Writer
 import Control.Monad
 import Data.Char
 import Data.Functor
+import Data.Functor.Identity
 import Data.List
 import Data.Maybe
 
@@ -177,7 +178,7 @@ getLiteralString = getLiteralStringExt (const Nothing)
 
 -- Definitely get a literal string, skipping over all non-literals
 onlyLiteralString :: Token -> String
-onlyLiteralString = fromJust . getLiteralStringExt (const $ return "")
+onlyLiteralString = runIdentity . getLiteralStringExt (const $ return "")
 
 -- Maybe get a literal string, but only if it's an unquoted argument.
 getUnquotedLiteral (T_NormalWord _ list) =
@@ -216,7 +217,7 @@ getGlobOrLiteralString = getLiteralStringExt f
 
 -- Maybe get the literal value of a token, using a custom function
 -- to map unrecognized Tokens into strings.
-getLiteralStringExt :: (Token -> Maybe String) -> Token -> Maybe String
+getLiteralStringExt :: Monad m => (Token -> m String) -> Token -> m String
 getLiteralStringExt more = g
   where
     allInList = fmap concat . mapM g
diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs
index e4640e7..0da80e3 100644
--- a/src/ShellCheck/AnalyzerLib.hs
+++ b/src/ShellCheck/AnalyzerLib.hs
@@ -806,7 +806,7 @@ isVariableName (x:r) = isVariableStartChar x && all isVariableChar r
 isVariableName _     = False
 
 getVariablesFromLiteralToken token =
-    getVariablesFromLiteral (fromJust $ getLiteralStringExt (const $ return " ") token)
+    getVariablesFromLiteral (runIdentity $ getLiteralStringExt (const $ return " ") token)
 
 -- Try to get referenced variables from a literal string like "$foo"
 -- Ignores tons of cases like arithmetic evaluation and array indices.
diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs
index 299a335..869b389 100644
--- a/src/ShellCheck/Checks/Commands.hs
+++ b/src/ShellCheck/Checks/Commands.hs
@@ -34,6 +34,7 @@ import ShellCheck.Regex
 import Control.Monad
 import Control.Monad.RWS
 import Data.Char
+import Data.Functor.Identity
 import Data.List
 import Data.Maybe
 import qualified Data.Map.Strict as Map
@@ -348,7 +349,7 @@ returnOrExit multi invalid = (f . arguments)
     isInvalid s = null s || any (not . isDigit) s || length s > 5
         || let value = (read s :: Integer) in value > 255
 
-    literal token = fromJust $ getLiteralStringExt lit token
+    literal token = runIdentity $ getLiteralStringExt lit token
     lit (T_DollarBraced {}) = return "0"
     lit (T_DollarArithmetic {}) = return "0"
     lit (T_DollarExpansion {}) = return "0"
@@ -735,7 +736,7 @@ checkAliasesUsesArgs = CommandCheck (Exactly "alias") (f . arguments)
     re = mkRegex "\\$\\{?[0-9*@]"
     f = mapM_ checkArg
     checkArg arg =
-        let string = fromJust $ getLiteralStringExt (const $ return "_") arg in
+        let string = runIdentity $ getLiteralStringExt (const $ return "_") arg in
             when ('=' `elem` string && string `matches` re) $
                 err (getId arg) 2142
                     "Aliases can't use positional parameters. Use a function."
@@ -781,7 +782,7 @@ checkFindWithoutPath = CommandCheck (Basename "find") f
     -- path. We assume that all the pre-path flags are single characters from a
     -- list of GNU and macOS flags.
     hasPath (first:rest) =
-        let flag = fromJust $ getLiteralStringExt (const $ return "___") first in
+        let flag = runIdentity $ getLiteralStringExt (const $ return "___") first in
             not ("-" `isPrefixOf` flag) || isLeadingFlag flag && hasPath rest
     hasPath [] = False
     isLeadingFlag flag = length flag <= 2 || all (`elem` leadingFlagChars) flag
diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs
index b643525..d9ae1cf 100644
--- a/src/ShellCheck/Checks/ShellSupport.hs
+++ b/src/ShellCheck/Checks/ShellSupport.hs
@@ -30,6 +30,7 @@ import ShellCheck.Regex
 import Control.Monad
 import Control.Monad.RWS
 import Data.Char
+import Data.Functor.Identity
 import Data.List
 import Data.Maybe
 import qualified Data.Map as Map
@@ -488,7 +489,7 @@ checkBraceExpansionVars = ForShell [Bash] f
             T_DollarExpansion {} -> return "$"
             T_DollarArithmetic {} -> return "$"
             _ -> return "-"
-    toString t = fromJust $ getLiteralStringExt literalExt t
+    toString t = runIdentity $ getLiteralStringExt literalExt t
     isEvaled t = do
         cmd <- getClosestCommandM t
         return $ maybe False (`isUnqualifiedCommand` "eval") cmd

From f8648e546595a0e44d8f8e3c46a68ec774534bde Mon Sep 17 00:00:00 2001
From: "Joseph C. Sible" <josephcsible@users.noreply.github.com>
Date: Sun, 9 Feb 2020 21:26:42 -0500
Subject: [PATCH 2/3] Switch getLiteralStringExt to Identity where it can never
 be Nothing

---
 src/ShellCheck/Analytics.hs       | 6 ++----
 src/ShellCheck/Checks/Commands.hs | 9 ++++-----
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 8a2902c..71743b4 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -1066,9 +1066,7 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
       checkStrings =
         mapM_ stringError . take 1 . filter isNonNum
 
-      isNonNum t = fromMaybe False $ do
-        s <- getLiteralStringExt (const $ return "") t
-        return . not . all numChar $ s
+      isNonNum t = not . all numChar . runIdentity $ getLiteralStringExt (const $ return "") t
       numChar x = isDigit x || x `elem` "+-. "
 
       stringError t = err (getId t) 2170 $
@@ -2595,7 +2593,7 @@ checkTildeInPath _ (T_SimpleCommand _ vars _) =
             warn id 2147 "Literal tilde in PATH works poorly across programs."
     checkVar _ = return ()
 
-    hasTilde t = fromMaybe False (liftM2 elem (return '~') (getLiteralStringExt (const $ return "") t))
+    hasTilde t = '~' `elem` runIdentity (getLiteralStringExt (const $ return "") t)
     isQuoted T_DoubleQuoted {} = True
     isQuoted T_SingleQuoted {} = True
     isQuoted _ = False
diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs
index 869b389..a6b8d9e 100644
--- a/src/ShellCheck/Checks/Commands.hs
+++ b/src/ShellCheck/Checks/Commands.hs
@@ -249,13 +249,12 @@ prop_checkGrepRe23= verifyNot checkGrepRe "grep '.*' file"
 checkGrepRe = CommandCheck (Basename "grep") check where
     check cmd = f cmd (arguments cmd)
     -- --regex=*(extglob) doesn't work. Fixme?
-    skippable (Just s) = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s
-    skippable _ = False
+    skippable s = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s
     f _ [] = return ()
     f cmd (x:r) =
-        let str = getLiteralStringExt (const $ return "_") x
+        let str = runIdentity $ getLiteralStringExt (const $ return "_") x
         in
-            if str `elem` [Just "--", Just "-e", Just "--regex"]
+            if str `elem` ["--", "-e", "--regex"]
             then checkRE cmd r -- Regex is *after* this
             else
                 if skippable str
@@ -366,7 +365,7 @@ checkFindExecWithSingleArgument = CommandCheck (Basename "find") (f . arguments)
     check (exec:arg:term:_) = do
         execS <- getLiteralString exec
         termS <- getLiteralString term
-        cmdS <- getLiteralStringExt (const $ return " ") arg
+        let cmdS = runIdentity $ getLiteralStringExt (const $ return " ") arg
 
         guard $ execS `elem` ["-exec", "-execdir"] && termS `elem` [";", "+"]
         guard $ cmdS `matches` commandRegex

From 4d92a2e15c36c9a3c029b66144e41db42a83cfc3 Mon Sep 17 00:00:00 2001
From: "Joseph C. Sible" <josephcsible@users.noreply.github.com>
Date: Sun, 9 Feb 2020 21:36:38 -0500
Subject: [PATCH 3/3] Add getLiteralStringDef and simplify with it

---
 src/ShellCheck/ASTLib.hs          | 6 +++++-
 src/ShellCheck/Analytics.hs       | 4 ++--
 src/ShellCheck/AnalyzerLib.hs     | 2 +-
 src/ShellCheck/Checks/Commands.hs | 8 ++++----
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs
index 658ed64..17a475d 100644
--- a/src/ShellCheck/ASTLib.hs
+++ b/src/ShellCheck/ASTLib.hs
@@ -176,9 +176,13 @@ willConcatInAssignment token =
 getLiteralString :: Token -> Maybe String
 getLiteralString = getLiteralStringExt (const Nothing)
 
+-- Definitely get a literal string, with a given default for all non-literals
+getLiteralStringDef :: String -> Token -> String
+getLiteralStringDef x = runIdentity . getLiteralStringExt (const $ return x)
+
 -- Definitely get a literal string, skipping over all non-literals
 onlyLiteralString :: Token -> String
-onlyLiteralString = runIdentity . getLiteralStringExt (const $ return "")
+onlyLiteralString = getLiteralStringDef ""
 
 -- Maybe get a literal string, but only if it's an unquoted argument.
 getUnquotedLiteral (T_NormalWord _ list) =
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 71743b4..f390b53 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -1066,7 +1066,7 @@ checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do
       checkStrings =
         mapM_ stringError . take 1 . filter isNonNum
 
-      isNonNum t = not . all numChar . runIdentity $ getLiteralStringExt (const $ return "") t
+      isNonNum t = not . all numChar $ onlyLiteralString t
       numChar x = isDigit x || x `elem` "+-. "
 
       stringError t = err (getId t) 2170 $
@@ -2593,7 +2593,7 @@ checkTildeInPath _ (T_SimpleCommand _ vars _) =
             warn id 2147 "Literal tilde in PATH works poorly across programs."
     checkVar _ = return ()
 
-    hasTilde t = '~' `elem` runIdentity (getLiteralStringExt (const $ return "") t)
+    hasTilde t = '~' `elem` onlyLiteralString t
     isQuoted T_DoubleQuoted {} = True
     isQuoted T_SingleQuoted {} = True
     isQuoted _ = False
diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs
index 0da80e3..7c046f3 100644
--- a/src/ShellCheck/AnalyzerLib.hs
+++ b/src/ShellCheck/AnalyzerLib.hs
@@ -806,7 +806,7 @@ isVariableName (x:r) = isVariableStartChar x && all isVariableChar r
 isVariableName _     = False
 
 getVariablesFromLiteralToken token =
-    getVariablesFromLiteral (runIdentity $ getLiteralStringExt (const $ return " ") token)
+    getVariablesFromLiteral (getLiteralStringDef " " token)
 
 -- Try to get referenced variables from a literal string like "$foo"
 -- Ignores tons of cases like arithmetic evaluation and array indices.
diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs
index a6b8d9e..88b5967 100644
--- a/src/ShellCheck/Checks/Commands.hs
+++ b/src/ShellCheck/Checks/Commands.hs
@@ -252,7 +252,7 @@ checkGrepRe = CommandCheck (Basename "grep") check where
     skippable s = not ("--regex=" `isPrefixOf` s) && "-" `isPrefixOf` s
     f _ [] = return ()
     f cmd (x:r) =
-        let str = runIdentity $ getLiteralStringExt (const $ return "_") x
+        let str = getLiteralStringDef "_" x
         in
             if str `elem` ["--", "-e", "--regex"]
             then checkRE cmd r -- Regex is *after* this
@@ -365,7 +365,7 @@ checkFindExecWithSingleArgument = CommandCheck (Basename "find") (f . arguments)
     check (exec:arg:term:_) = do
         execS <- getLiteralString exec
         termS <- getLiteralString term
-        let cmdS = runIdentity $ getLiteralStringExt (const $ return " ") arg
+        let cmdS = getLiteralStringDef " " arg
 
         guard $ execS `elem` ["-exec", "-execdir"] && termS `elem` [";", "+"]
         guard $ cmdS `matches` commandRegex
@@ -735,7 +735,7 @@ checkAliasesUsesArgs = CommandCheck (Exactly "alias") (f . arguments)
     re = mkRegex "\\$\\{?[0-9*@]"
     f = mapM_ checkArg
     checkArg arg =
-        let string = runIdentity $ getLiteralStringExt (const $ return "_") arg in
+        let string = getLiteralStringDef "_" arg in
             when ('=' `elem` string && string `matches` re) $
                 err (getId arg) 2142
                     "Aliases can't use positional parameters. Use a function."
@@ -781,7 +781,7 @@ checkFindWithoutPath = CommandCheck (Basename "find") f
     -- path. We assume that all the pre-path flags are single characters from a
     -- list of GNU and macOS flags.
     hasPath (first:rest) =
-        let flag = runIdentity $ getLiteralStringExt (const $ return "___") first in
+        let flag = getLiteralStringDef "___" first in
             not ("-" `isPrefixOf` flag) || isLeadingFlag flag && hasPath rest
     hasPath [] = False
     isLeadingFlag flag = length flag <= 2 || all (`elem` leadingFlagChars) flag