From cfd68ee0c2ebfd0ab08a1d4bf628162b454dc207 Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Tue, 1 Sep 2020 16:48:14 -0700 Subject: [PATCH] Give each sh/dash compatibility warning its own SC3xxx error code --- CHANGELOG.md | 1 + nextnumber | 2 +- src/ShellCheck/Checker.hs | 2 +- src/ShellCheck/Checks/ShellSupport.hs | 115 +++++++++++++------------- 4 files changed, 62 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f63da..d4b9145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Changed - Assignments are now parsed to spec, without leniency for leading $ or spaces +- POSIX/dash unsupported feature warnings now have individual SC3xxx codes - SC1090: A leading `$x/` or `$(x)/` is now treated as `./` when locating files - SC2154: Variables appearing in -z/-n tests are no longer considered unassigned diff --git a/nextnumber b/nextnumber index 0c9c7b1..3bdf10a 100755 --- a/nextnumber +++ b/nextnumber @@ -6,7 +6,7 @@ then exit 1 fi -for i in 1 2 +for i in 1 2 3 do last=$(grep -hv "^prop" ./**/*.hs | grep -Ewo "${i}[0-9]{3}" | sort -n | tail -n 1) echo "Next ${i}xxx: $((last+1))" diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs index 673a116..d81d664 100644 --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -276,7 +276,7 @@ prop_filewideAnnotation8 = null $ check "# Disable $? warning\n#shellcheck disable=SC2181\n# Disable quoting warning\n#shellcheck disable=2086\ntrue\n[ $? == 0 ] && echo $1" prop_sourcePartOfOriginalScript = -- #1181: -x disabled posix warning for 'source' - 2039 `elem` checkWithIncludes [("./saywhat.sh", "echo foo")] "#!/bin/sh\nsource ./saywhat.sh" + 3046 `elem` checkWithIncludes [("./saywhat.sh", "echo foo")] "#!/bin/sh\nsource ./saywhat.sh" prop_spinBug1413 = null $ check "fun() {\n# shellcheck disable=SC2188\n> /dev/null\n}\n" diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs index 788323d..8b8e095 100644 --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -188,102 +188,102 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do kludge params = bashism where isDash = shellType params == Dash - warnMsg id s = + warnMsg id code s = if isDash - then err id 2169 $ "In dash, " ++ s ++ " not supported." - else warn id 2039 $ "In POSIX sh, " ++ s ++ " undefined." + then err id code $ "In dash, " ++ s ++ " not supported." + else warn id code $ "In POSIX sh, " ++ s ++ " undefined." - bashism (T_ProcSub id _ _) = warnMsg id "process substitution is" - bashism (T_Extglob id _ _) = warnMsg id "extglob is" - bashism (T_DollarSingleQuoted id _) = warnMsg id "$'..' is" - bashism (T_DollarDoubleQuoted id _) = warnMsg id "$\"..\" is" - bashism (T_ForArithmetic id _ _ _ _) = warnMsg id "arithmetic for loops are" - bashism (T_Arithmetic id _) = warnMsg id "standalone ((..)) is" - bashism (T_DollarBracket id _) = warnMsg id "$[..] in place of $((..)) is" - bashism (T_SelectIn id _ _ _) = warnMsg id "select loops are" - bashism (T_BraceExpansion id _) = warnMsg id "brace expansion is" - bashism (T_Condition id DoubleBracket _) = warnMsg id "[[ ]] is" - bashism (T_HereString id _) = warnMsg id "here-strings are" + bashism (T_ProcSub id _ _) = warnMsg id 3001 "process substitution is" + bashism (T_Extglob id _ _) = warnMsg id 3002 "extglob is" + bashism (T_DollarSingleQuoted id _) = warnMsg id 3003 "$'..' is" + bashism (T_DollarDoubleQuoted id _) = warnMsg id 3004 "$\"..\" is" + bashism (T_ForArithmetic id _ _ _ _) = warnMsg id 3005 "arithmetic for loops are" + bashism (T_Arithmetic id _) = warnMsg id 3006 "standalone ((..)) is" + bashism (T_DollarBracket id _) = warnMsg id 3007 "$[..] in place of $((..)) is" + bashism (T_SelectIn id _ _ _) = warnMsg id 3008 "select loops are" + bashism (T_BraceExpansion id _) = warnMsg id 3009 "brace expansion is" + bashism (T_Condition id DoubleBracket _) = 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 $ "lexicographical " ++ op ++ " is" + unless isDash $ warnMsg id 3012 $ "lexicographical " ++ op ++ " is" bashism (TC_Binary id SingleBracket op _ _) | op `elem` [ "-ot", "-nt", "-ef" ] = - unless isDash $ warnMsg id $ op ++ " is" + unless isDash $ warnMsg id 3013 $ op ++ " is" bashism (TC_Binary id SingleBracket "==" _ _) = - warnMsg id "== in place of = is" + warnMsg id 3014 "== in place of = is" bashism (TC_Binary id SingleBracket "=~" _ _) = - warnMsg id "=~ regex matching is" + warnMsg id 3015 "=~ regex matching is" bashism (TC_Unary id SingleBracket "-v" _) = - warnMsg id "unary -v (in place of [ -n \"${var+x}\" ]) is" + warnMsg id 3016 "unary -v (in place of [ -n \"${var+x}\" ]) is" bashism (TC_Unary id _ "-a" _) = - warnMsg id "unary -a in place of -e is" + warnMsg id 3017 "unary -a in place of -e is" bashism (TA_Unary id op _) | op `elem` [ "|++", "|--", "++|", "--|"] = - warnMsg id $ filter (/= '|') op ++ " is" - bashism (TA_Binary id "**" _ _) = warnMsg id "exponentials are" - bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id "&> is" - bashism (T_FdRedirect id "" (T_IoFile _ (T_GREATAND _) _)) = warnMsg id ">& is" - bashism (T_FdRedirect id ('{':_) _) = warnMsg id "named file descriptors are" + warnMsg id 3018 $ filter (/= '|') op ++ " is" + bashism (TA_Binary id "**" _ _) = warnMsg id 3019 "exponentials are" + bashism (T_FdRedirect id "&" (T_IoFile _ (T_Greater _) _)) = warnMsg id 3020 "&> is" + bashism (T_FdRedirect id "" (T_IoFile _ (T_GREATAND _) _)) = warnMsg id 3021 ">& is" + bashism (T_FdRedirect id ('{':_) _) = warnMsg id 3022 "named file descriptors are" bashism (T_FdRedirect id num _) - | all isDigit num && length num > 1 = warnMsg id "FDs outside 0-9 are" + | all isDigit num && length num > 1 = warnMsg id 3023 "FDs outside 0-9 are" bashism (T_Assignment id Append _ _ _) = - warnMsg id "+= is" + warnMsg id 3024 "+= is" bashism (T_IoFile id _ word) | isNetworked = - warnMsg id "/dev/{tcp,udp} is" + warnMsg id 3025 "/dev/{tcp,udp} is" where file = onlyLiteralString word isNetworked = any (`isPrefixOf` file) ["/dev/tcp", "/dev/udp"] bashism (T_Glob id str) | "[^" `isInfixOf` str = - warnMsg id "^ in place of ! in glob bracket expressions is" + warnMsg id 3026 "^ in place of ! in glob bracket expressions is" bashism t@(TA_Variable id str _) | isBashVariable str = - warnMsg id $ str ++ " is" + warnMsg id 3027 $ str ++ " is" bashism t@(T_DollarBraced id _ token) = do mapM_ check expansion when (isBashVariable var) $ - warnMsg id $ var ++ " is" + warnMsg id 3028 $ var ++ " is" where str = concat $ oversimplify token var = getBracedReference str check (regex, feature) = - when (isJust $ matchRegex regex str) $ warnMsg id feature + when (isJust $ matchRegex regex str) $ warnMsg id 3053 feature bashism t@(T_Pipe id "|&") = - warnMsg id "|& in place of 2>&1 | is" + warnMsg id 3029 "|& in place of 2>&1 | is" bashism (T_Array id _) = - warnMsg id "arrays are" + warnMsg id 3030 "arrays are" bashism (T_IoFile id _ t) | isGlob t = - warnMsg id "redirecting to/from globs is" + warnMsg id 3031 "redirecting to/from globs is" bashism (T_CoProc id _ _) = - warnMsg id "coproc is" + warnMsg id 3032 "coproc is" bashism (T_Function id _ _ str _) | not (isVariableName str) = - warnMsg id "naming functions outside [a-zA-Z_][a-zA-Z0-9_]* is" + warnMsg id 3033 "naming functions outside [a-zA-Z_][a-zA-Z0-9_]* is" bashism (T_DollarExpansion id [x]) | isOnlyRedirection x = - warnMsg id "$( do checkOptions (flag@(fid,flag') : opt@(oid,opt') : rest) | flag' `matches` oFlagRegex = do when (opt' `notElem` longOptions) $ - warnMsg oid $ "set option " <> opt' <> " is" + warnMsg oid 3040 $ "set option " <> opt' <> " is" checkFlags (flag:rest) | otherwise = checkFlags (flag:opt:rest) checkOptions (flag:rest) = checkFlags (flag:rest) @@ -314,10 +314,10 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do unless (flag' `matches` validFlagsRegex) $ forM_ (tail flag') $ \letter -> when (letter `notElem` optionsSet) $ - warnMsg fid $ "set flag " <> ('-':letter:" is") + warnMsg fid 3041 $ "set flag " <> ('-':letter:" is") checkOptions rest | beginsWithDoubleDash flag' = do - warnMsg fid $ "set flag " <> flag' <> " is" + warnMsg fid 3042 $ "set flag " <> flag' <> " is" checkOptions rest -- Either a word that doesn't start with a dash, or simply '--', -- so stop checking. @@ -339,16 +339,19 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do let name = fromMaybe "" $ getCommandName t flags = getLeadingFlags t in do + when (name == "local" && not isDash) $ + -- This is so commonly accepted that we'll make it a special case + warnMsg id 3043 $ "'local' is" when (name `elem` unsupportedCommands) $ - warnMsg id $ "'" ++ name ++ "' is" + warnMsg id 3044 $ "'" ++ name ++ "' is" sequence_ $ do allowed' <- Map.lookup name allowedFlags allowed <- allowed' (word, flag) <- find (\x -> (not . null . snd $ x) && snd x `notElem` allowed) flags - return . warnMsg (getId word) $ name ++ " -" ++ flag ++ " is" + return . warnMsg (getId word) 3045 $ name ++ " -" ++ flag ++ " is" - when (name == "source") $ warnMsg id "'source' in place of '.' is" + when (name == "source") $ warnMsg id 3046 "'source' in place of '.' is" when (name == "trap") $ let check token = sequence_ $ do @@ -356,12 +359,12 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do let upper = map toUpper str return $ do when (upper `elem` ["ERR", "DEBUG", "RETURN"]) $ - warnMsg (getId token) $ "trapping " ++ str ++ " is" + warnMsg (getId token) 3047 $ "trapping " ++ str ++ " is" when ("SIG" `isPrefixOf` upper) $ - warnMsg (getId token) + warnMsg (getId token) 3048 "prefixing signal names with 'SIG' is" when (not isDash && upper /= str) $ - warnMsg (getId token) + warnMsg (getId token) 3049 "using lower/mixed case for signal names is" in mapM_ check (drop 1 rest) @@ -370,13 +373,13 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do format <- rest !!! 0 -- flags are covered by allowedFlags let literal = onlyLiteralString format guard $ "%q" `isInfixOf` literal - return $ warnMsg (getId format) "printf %q is" + return $ warnMsg (getId format) 3050 "printf %q is" where unsupportedCommands = [ "let", "caller", "builtin", "complete", "compgen", "declare", "dirs", "disown", "enable", "mapfile", "readarray", "pushd", "popd", "shopt", "suspend", "typeset" - ] ++ if not isDash then ["local"] else [] + ] allowedFlags = Map.fromList [ ("cd", Just ["L", "P"]), ("exec", Just []), @@ -394,9 +397,9 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do ("wait", Just []) ] bashism t@(T_SourceCommand id src _) - | getCommandName src == Just "source" = warnMsg id "'source' in place of '.' is" + | getCommandName src == Just "source" = warnMsg id 3051 "'source' in place of '.' is" bashism (TA_Expansion _ (T_Literal id str : _)) - | str `matches` radix = warnMsg id "arithmetic base conversion is" + | str `matches` radix = warnMsg id 3052 "arithmetic base conversion is" where radix = mkRegex "^[0-9]+#" bashism _ = return ()