Track whether braces were present in T_DollarBraced

References of the form $var and ${var} both map to the same structure in
the AST, which prevents any later analysis functions from distinguishing
them.  In preparation for adding checks that need this info, add a Bool
to T_DollarBraced that tracks whether the braces were seen at parsing
time and update all references so that this change is a no-op.
This commit is contained in:
Benjamin Gordon 2019-04-29 14:26:39 -06:00 committed by Benjamin Gordon
parent 0358090b3c
commit aa3b709b5d
7 changed files with 33 additions and 34 deletions

View File

@ -76,7 +76,7 @@ data Token =
| T_DSEMI Id | T_DSEMI Id
| T_Do Id | T_Do Id
| T_DollarArithmetic Id Token | T_DollarArithmetic Id Token
| T_DollarBraced Id Token | T_DollarBraced Id Bool Token
| T_DollarBracket Id Token | T_DollarBracket Id Token
| T_DollarDoubleQuoted Id [Token] | T_DollarDoubleQuoted Id [Token]
| T_DollarExpansion Id [Token] | T_DollarExpansion Id [Token]
@ -253,7 +253,7 @@ analyze f g i =
delve (T_Function id a b name body) = d1 body $ T_Function id a b name delve (T_Function id a b name body) = d1 body $ T_Function id a b name
delve (T_Condition id typ token) = d1 token $ T_Condition id typ delve (T_Condition id typ token) = d1 token $ T_Condition id typ
delve (T_Extglob id str l) = dl l $ T_Extglob id str delve (T_Extglob id str l) = dl l $ T_Extglob id str
delve (T_DollarBraced id op) = d1 op $ T_DollarBraced id delve (T_DollarBraced id braced op) = d1 op $ T_DollarBraced id braced
delve (T_HereDoc id d q str l) = dl l $ T_HereDoc id d q str delve (T_HereDoc id d q str l) = dl l $ T_HereDoc id d q str
delve (TC_And id typ str t1 t2) = d2 t1 t2 $ TC_And id typ str delve (TC_And id typ str t1 t2) = d2 t1 t2 $ TC_And id typ str
@ -323,7 +323,7 @@ getId t = case t of
T_NormalWord id _ -> id T_NormalWord id _ -> id
T_DoubleQuoted id _ -> id T_DoubleQuoted id _ -> id
T_DollarExpansion id _ -> id T_DollarExpansion id _ -> id
T_DollarBraced id _ -> id T_DollarBraced id _ _ -> id
T_DollarArithmetic id _ -> id T_DollarArithmetic id _ -> id
T_BraceExpansion id _ -> id T_BraceExpansion id _ -> id
T_ParamSubSpecialChar id _ -> id T_ParamSubSpecialChar id _ -> id

View File

@ -81,7 +81,7 @@ oversimplify token =
(T_NormalWord _ l) -> [concat (concatMap oversimplify l)] (T_NormalWord _ l) -> [concat (concatMap oversimplify l)]
(T_DoubleQuoted _ l) -> [concat (concatMap oversimplify l)] (T_DoubleQuoted _ l) -> [concat (concatMap oversimplify l)]
(T_SingleQuoted _ s) -> [s] (T_SingleQuoted _ s) -> [s]
(T_DollarBraced _ _) -> ["${VAR}"] (T_DollarBraced _ _ _) -> ["${VAR}"]
(T_DollarArithmetic _ _) -> ["${VAR}"] (T_DollarArithmetic _ _) -> ["${VAR}"]
(T_DollarExpansion _ _) -> ["${VAR}"] (T_DollarExpansion _ _) -> ["${VAR}"]
(T_Backticked _ _) -> ["${VAR}"] (T_Backticked _ _) -> ["${VAR}"]
@ -133,11 +133,11 @@ isUnquotedFlag token = fromMaybe False $ do
return $ "-" `isPrefixOf` str return $ "-" `isPrefixOf` str
-- Given a T_DollarBraced, return a simplified version of the string contents. -- Given a T_DollarBraced, return a simplified version of the string contents.
bracedString (T_DollarBraced _ l) = concat $ oversimplify l bracedString (T_DollarBraced _ _ l) = concat $ oversimplify l
bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)" bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)"
-- Is this an expansion of multiple items of an array? -- Is this an expansion of multiple items of an array?
isArrayExpansion t@(T_DollarBraced _ _) = isArrayExpansion t@(T_DollarBraced _ _ _) =
let string = bracedString t in let string = bracedString t in
"@" `isPrefixOf` string || "@" `isPrefixOf` string ||
not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string
@ -146,7 +146,7 @@ isArrayExpansion _ = False
-- Is it possible that this arg becomes multiple args? -- Is it possible that this arg becomes multiple args?
mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t
where where
f t@(T_DollarBraced _ _) = f t@(T_DollarBraced _ _ _) =
let string = bracedString t in let string = bracedString t in
"!" `isPrefixOf` string "!" `isPrefixOf` string
f (T_DoubleQuoted _ parts) = any f parts f (T_DoubleQuoted _ parts) = any f parts

View File

@ -754,7 +754,7 @@ checkShorthandIf _ _ = return ()
prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done"
prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*"
prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]" prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]"
checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _)]) checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _ _)])
| bracedString b == "*" = | bracedString b == "*" =
unless (isStrictlyQuoteFree (parentMap p) t) $ unless (isStrictlyQuoteFree (parentMap p) t) $
warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems." warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems."
@ -825,7 +825,7 @@ checkArrayWithoutIndex params _ =
doVariableFlowAnalysis readF writeF defaultMap (variableFlow params) doVariableFlowAnalysis readF writeF defaultMap (variableFlow params)
where where
defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables
readF _ (T_DollarBraced id token) _ = do readF _ (T_DollarBraced id _ token) _ = do
map <- get map <- get
return . maybeToList $ do return . maybeToList $ do
name <- getLiteralString token name <- getLiteralString token
@ -1267,7 +1267,7 @@ prop_checkArithmeticDeref12= verify checkArithmeticDeref "for ((i=0; $i < 3; i))
prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))" prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))"
prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))" prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))"
prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))" prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))"
checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _)]) = checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _ _)]) =
unless (isException $ bracedString b) getWarning unless (isException $ bracedString b) getWarning
where where
isException [] = True isException [] = True
@ -1302,8 +1302,7 @@ prop_checkComparisonAgainstGlob3 = verify checkComparisonAgainstGlob "[ $cow = *
prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]"
prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]" prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]"
prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]" prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]"
checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _ _]))
checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _]))
| op `elem` ["=", "==", "!="] = | op `elem` ["=", "==", "!="] =
warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching." warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching."
checkComparisonAgainstGlob params (TC_Binary _ SingleBracket op _ word) checkComparisonAgainstGlob params (TC_Binary _ SingleBracket op _ word)
@ -1457,7 +1456,7 @@ prop_checkIndirectExpansion2 = verifyNot checkIndirectExpansion "${foo//$n/lol}"
prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}" prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}"
prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}" prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}"
prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}" prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}"
checkIndirectExpansion _ (T_DollarBraced i (T_NormalWord _ contents)) = checkIndirectExpansion _ (T_DollarBraced i _ (T_NormalWord _ contents)) =
when (isIndirection contents) $ when (isIndirection contents) $
err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval." err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval."
where where
@ -1467,7 +1466,7 @@ checkIndirectExpansion _ (T_DollarBraced i (T_NormalWord _ contents)) =
isIndirectionPart t = isIndirectionPart t =
case t of T_DollarExpansion _ _ -> Just True case t of T_DollarExpansion _ _ -> Just True
T_Backticked _ _ -> Just True T_Backticked _ _ -> Just True
T_DollarBraced _ _ -> Just True T_DollarBraced _ _ _ -> Just True
T_DollarArithmetic _ _ -> Just True T_DollarArithmetic _ _ -> Just True
T_Literal _ s -> if all isVariableChar s T_Literal _ s -> if all isVariableChar s
then Nothing then Nothing
@ -1494,7 +1493,7 @@ checkInexplicablyUnquoted params (T_NormalWord id tokens) = mapM_ check (tails t
check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) = check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) =
case trapped of case trapped of
T_DollarExpansion id _ -> warnAboutExpansion id T_DollarExpansion id _ -> warnAboutExpansion id
T_DollarBraced id _ -> warnAboutExpansion id T_DollarBraced id _ _ -> warnAboutExpansion id
T_Literal id s -> T_Literal id s ->
unless (quotesSingleThing a && quotesSingleThing b || isRegex (getPath (parentMap params) trapped)) $ unless (quotesSingleThing a && quotesSingleThing b || isRegex (getPath (parentMap params) trapped)) $
warnAboutLiteral id warnAboutLiteral id
@ -1515,7 +1514,7 @@ checkInexplicablyUnquoted params (T_NormalWord id tokens) = mapM_ check (tails t
-- the quotes were probably intentional and harmless. -- the quotes were probably intentional and harmless.
quotesSingleThing x = case x of quotesSingleThing x = case x of
[T_DollarExpansion _ _] -> True [T_DollarExpansion _ _] -> True
[T_DollarBraced _ _] -> True [T_DollarBraced _ _ _] -> True
[T_Backticked _ _] -> True [T_Backticked _ _] -> True
_ -> False _ -> False
@ -1822,7 +1821,7 @@ checkSpacefulness' onFind params t =
isExpansion t = isExpansion t =
case t of case t of
(T_DollarBraced _ _ ) -> True (T_DollarBraced _ _ _ ) -> True
_ -> False _ -> False
isSpacefulWord :: (String -> Bool) -> [Token] -> Bool isSpacefulWord :: (String -> Bool) -> [Token] -> Bool
@ -1836,7 +1835,7 @@ checkSpacefulness' onFind params t =
T_Extglob {} -> True T_Extglob {} -> True
T_Literal _ s -> s `containsAny` globspace T_Literal _ s -> s `containsAny` globspace
T_SingleQuoted _ s -> s `containsAny` globspace T_SingleQuoted _ s -> s `containsAny` globspace
T_DollarBraced _ _ -> spacefulF $ getBracedReference $ bracedString x T_DollarBraced _ _ _ -> spacefulF $ getBracedReference $ bracedString x
T_NormalWord _ w -> isSpacefulWord spacefulF w T_NormalWord _ w -> isSpacefulWord spacefulF w
T_DoubleQuoted _ w -> isSpacefulWord spacefulF w T_DoubleQuoted _ w -> isSpacefulWord spacefulF w
_ -> False _ -> False
@ -1874,7 +1873,7 @@ checkQuotesInLiterals params t =
return [] return []
writeF _ _ _ _ = return [] writeF _ _ _ _ = return []
forToken map (T_DollarBraced id t) = forToken map (T_DollarBraced id _ t) =
-- skip getBracedReference here to avoid false positives on PE -- skip getBracedReference here to avoid false positives on PE
Map.lookup (concat . oversimplify $ t) map Map.lookup (concat . oversimplify $ t) map
forToken quoteMap (T_DoubleQuoted id tokens) = forToken quoteMap (T_DoubleQuoted id tokens) =
@ -1888,7 +1887,7 @@ checkQuotesInLiterals params t =
squashesQuotes t = squashesQuotes t =
case t of case t of
T_DollarBraced id _ -> "#" `isPrefixOf` bracedString t T_DollarBraced id _ _ -> "#" `isPrefixOf` bracedString t
_ -> False _ -> False
readF _ expr name = do readF _ expr name = do
@ -2135,10 +2134,10 @@ checkUnassignedReferences params t = warnings
isInArray var t = any isArray $ getPath (parentMap params) t isInArray var t = any isArray $ getPath (parentMap params) t
where where
isArray T_Array {} = True isArray T_Array {} = True
isArray b@(T_DollarBraced _ _) | var /= getBracedReference (bracedString b) = True isArray b@(T_DollarBraced _ _ _) | var /= getBracedReference (bracedString b) = True
isArray _ = False isArray _ = False
isGuarded (T_DollarBraced _ v) = isGuarded (T_DollarBraced _ _ v) =
rest `matches` guardRegex rest `matches` guardRegex
where where
name = concat $ oversimplify v name = concat $ oversimplify v
@ -2224,7 +2223,7 @@ checkWhileReadPitfalls _ _ = return ()
prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var" prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var"
prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd" prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd"
checkPrefixAssignmentReference params t@(T_DollarBraced id value) = checkPrefixAssignmentReference params t@(T_DollarBraced id _ value) =
check path check path
where where
name = getBracedReference $ bracedString t name = getBracedReference $ bracedString t
@ -3026,7 +3025,7 @@ checkSplittingInArrays params t =
T_DollarExpansion id _ -> forCommand id T_DollarExpansion id _ -> forCommand id
T_DollarBraceCommandExpansion id _ -> forCommand id T_DollarBraceCommandExpansion id _ -> forCommand id
T_Backticked id _ -> forCommand id T_Backticked id _ -> forCommand id
T_DollarBraced id str | T_DollarBraced id _ str |
not (isCountingReference part) not (isCountingReference part)
&& not (isQuotedAlternativeReference part) && not (isQuotedAlternativeReference part)
&& not (getBracedReference (bracedString part) `elem` variablesWithoutSpaces) && not (getBracedReference (bracedString part) `elem` variablesWithoutSpaces)

View File

@ -510,7 +510,7 @@ getModifiedVariables t =
guard . not . null $ str guard . not . null $ str
return (t, token, str, DataString SourceChecked) return (t, token, str, DataString SourceChecked)
T_DollarBraced _ l -> maybeToList $ do T_DollarBraced _ _ l -> maybeToList $ do
let string = bracedString t let string = bracedString t
let modifier = getBracedModifier string let modifier = getBracedModifier string
guard $ ":=" `isPrefixOf` modifier guard $ ":=" `isPrefixOf` modifier
@ -702,7 +702,7 @@ getOffsetReferences mods = fromMaybe [] $ do
getReferencedVariables parents t = getReferencedVariables parents t =
case t of case t of
T_DollarBraced id l -> let str = bracedString t in T_DollarBraced id _ l -> let str = bracedString t in
(t, t, getBracedReference str) : (t, t, getBracedReference str) :
map (\x -> (l, l, x)) ( map (\x -> (l, l, x)) (
getIndexReferences str getIndexReferences str
@ -897,7 +897,7 @@ shouldIgnoreCode params code t =
getPath (parentMap params) t getPath (parentMap params) t
-- Is this a ${#anything}, to get string length or array count? -- Is this a ${#anything}, to get string length or array count?
isCountingReference (T_DollarBraced id token) = isCountingReference (T_DollarBraced id _ token) =
case concat $ oversimplify token of case concat $ oversimplify token of
'#':_ -> True '#':_ -> True
_ -> False _ -> False
@ -906,7 +906,7 @@ isCountingReference _ = False
-- FIXME: doesn't handle ${a:+$var} vs ${a:+"$var"} -- FIXME: doesn't handle ${a:+$var} vs ${a:+"$var"}
isQuotedAlternativeReference t = isQuotedAlternativeReference t =
case t of case t of
T_DollarBraced _ _ -> T_DollarBraced _ _ _ ->
getBracedModifier (bracedString t) `matches` re getBracedModifier (bracedString t) `matches` re
_ -> False _ -> False
where where

View File

@ -278,7 +278,7 @@ checkTrapQuotes = CommandCheck (Exactly "trap") (f . arguments) where
warning id = warn id 2064 "Use single quotes, otherwise this expands now rather than when signalled." warning id = warn id 2064 "Use single quotes, otherwise this expands now rather than when signalled."
checkExpansions (T_DollarExpansion id _) = warning id checkExpansions (T_DollarExpansion id _) = warning id
checkExpansions (T_Backticked id _) = warning id checkExpansions (T_Backticked id _) = warning id
checkExpansions (T_DollarBraced id _) = warning id checkExpansions (T_DollarBraced id _ _) = warning id
checkExpansions (T_DollarArithmetic id _) = warning id checkExpansions (T_DollarArithmetic id _) = warning id
checkExpansions _ = return () checkExpansions _ = return ()
@ -896,7 +896,7 @@ checkCatastrophicRm = CommandCheck (Basename "rm") $ \t ->
getPotentialPath = getLiteralStringExt f getPotentialPath = getLiteralStringExt f
where where
f (T_Glob _ str) = return str f (T_Glob _ str) = return str
f (T_DollarBraced _ word) = f (T_DollarBraced _ _ word) =
let var = onlyLiteralString word in let var = onlyLiteralString word in
-- This shouldn't handle non-colon cases. -- This shouldn't handle non-colon cases.
if any (`isInfixOf` var) [":?", ":-", ":="] if any (`isInfixOf` var) [":?", ":-", ":="]

View File

@ -232,7 +232,7 @@ checkBashisms = ForShell [Sh, Dash] $ \t -> do
bashism t@(TA_Variable id str _) | isBashVariable str = bashism t@(TA_Variable id str _) | isBashVariable str =
warnMsg id $ str ++ " is" warnMsg id $ str ++ " is"
bashism t@(T_DollarBraced id token) = do bashism t@(T_DollarBraced id _ token) = do
mapM_ check expansion mapM_ check expansion
when (isBashVariable var) $ when (isBashVariable var) $
warnMsg id $ var ++ " is" warnMsg id $ var ++ " is"

View File

@ -1634,7 +1634,7 @@ readDollarBraced = called "parameter expansion" $ do
word <- readDollarBracedWord word <- readDollarBracedWord
char '}' char '}'
id <- endSpan start id <- endSpan start
return $ T_DollarBraced id word return $ T_DollarBraced id True word
prop_readDollarExpansion1= isOk readDollarExpansion "$(echo foo; ls\n)" prop_readDollarExpansion1= isOk readDollarExpansion "$(echo foo; ls\n)"
prop_readDollarExpansion2= isOk readDollarExpansion "$( )" prop_readDollarExpansion2= isOk readDollarExpansion "$( )"
@ -1661,7 +1661,7 @@ readDollarVariable = do
let singleCharred p = do let singleCharred p = do
value <- wrapString ((:[]) <$> p) value <- wrapString ((:[]) <$> p)
id <- endSpan start id <- endSpan start
return $ (T_DollarBraced id value) return $ (T_DollarBraced id False value)
let positional = do let positional = do
value <- singleCharred digit value <- singleCharred digit
@ -1674,7 +1674,7 @@ readDollarVariable = do
let regular = do let regular = do
value <- wrapString readVariableName value <- wrapString readVariableName
id <- endSpan start id <- endSpan start
return (T_DollarBraced id value) `attempting` do return (T_DollarBraced id False value) `attempting` do
lookAhead $ char '[' lookAhead $ char '['
parseNoteAt pos ErrorC 1087 "Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet)." parseNoteAt pos ErrorC 1087 "Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet)."