From 2341a4c6836c0eec0a622ed9d3cf83f36ab4c1ab Mon Sep 17 00:00:00 2001 From: Benjamin Gordon Date: Wed, 13 Nov 2019 15:50:21 -0700 Subject: [PATCH 1/2] SC2256: Check for translated strings matching known variables SC2247 already warns about translated strings that look like $"(foo)" or $"{foo}". Since typical use of translated strings is to translate whole messages, a string like $"foo" is likely to be a similar mistake if foo is the name of an existing variable. Conversely, a string like $"foo bar" is potentially meant to be a message id even if foo is a known variable. Add a warning for the $"foo" case, but make it separate from the existing warning so that projects that reuse variable names as their message ids can separately disable the new warning. --- CHANGELOG.md | 1 + src/ShellCheck/Analytics.hs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6689dac..d7e4181 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Added - SC2254: Suggest quoting expansions in case statements - SC2255: Suggest using `$((..))` in `[ 2*3 -eq 6 ]` +- SC2256: Warn about translated strings that are known variables ## v0.7.0 - 2019-07-28 ### Added diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index 97054c1..b0928ce 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -192,6 +192,7 @@ nodeChecks = [ ,checkRedirectionToCommand ,checkDollarQuoteParen ,checkUselessBang + ,checkTranslatedString ] optionalChecks = map fst optionalTreeChecks @@ -3451,6 +3452,24 @@ checkDollarQuoteParen params t = where fix id = fixWith [replaceStart id params 2 "\"$"] +prop_checkTranslatedString1 = verify checkTranslatedString "foo_bar2=val; $\"foo_bar2\"" +prop_checkTranslatedString2 = verifyNot checkTranslatedString "$\"foo_bar2\"" +prop_checkTranslatedString3 = verifyNot checkTranslatedString "$\"..\"" +prop_checkTranslatedString4 = verifyNot checkTranslatedString "var=val; $\"$var\"" +prop_checkTranslatedString5 = verifyNot checkTranslatedString "foo=var; bar=val2; $\"foo bar\"" +checkTranslatedString params (T_DollarDoubleQuoted id ((T_Literal _ s):_)) = + fromMaybe (return ()) $ do + Map.lookup s assignments + return $ + warnWithFix id 2256 "This translated string is the name of a variable. Flip leading $ and \" if this should be a quoted substitution." (fix id) + where + assignments = foldl (flip ($)) Map.empty (map insertAssignment $ variableFlow params) + insertAssignment (Assignment (_, token, name, _)) | isVariableName name = + Map.insert name token + insertAssignment _ = Prelude.id + fix id = fixWith [replaceStart id params 2 "\"$"] +checkTranslatedString _ _ = return () + prop_checkDefaultCase1 = verify checkDefaultCase "case $1 in a) true ;; esac" prop_checkDefaultCase2 = verify checkDefaultCase "case $1 in ?*?) true ;; *? ) true ;; esac" prop_checkDefaultCase3 = verifyNot checkDefaultCase "case $1 in x|*) true ;; esac" From 4a63a3a8bdd7ed7525f44226c094570da3786e94 Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Wed, 13 Nov 2019 19:53:55 -0800 Subject: [PATCH 2/2] For SC2256, make sure the complete string is a variable name --- src/ShellCheck/Analytics.hs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs index b0928ce..a29d76d 100644 --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -192,7 +192,7 @@ nodeChecks = [ ,checkRedirectionToCommand ,checkDollarQuoteParen ,checkUselessBang - ,checkTranslatedString + ,checkTranslatedStringVariable ] optionalChecks = map fst optionalTreeChecks @@ -3452,13 +3452,14 @@ checkDollarQuoteParen params t = where fix id = fixWith [replaceStart id params 2 "\"$"] -prop_checkTranslatedString1 = verify checkTranslatedString "foo_bar2=val; $\"foo_bar2\"" -prop_checkTranslatedString2 = verifyNot checkTranslatedString "$\"foo_bar2\"" -prop_checkTranslatedString3 = verifyNot checkTranslatedString "$\"..\"" -prop_checkTranslatedString4 = verifyNot checkTranslatedString "var=val; $\"$var\"" -prop_checkTranslatedString5 = verifyNot checkTranslatedString "foo=var; bar=val2; $\"foo bar\"" -checkTranslatedString params (T_DollarDoubleQuoted id ((T_Literal _ s):_)) = +prop_checkTranslatedStringVariable1 = verify checkTranslatedStringVariable "foo_bar2=val; $\"foo_bar2\"" +prop_checkTranslatedStringVariable2 = verifyNot checkTranslatedStringVariable "$\"foo_bar2\"" +prop_checkTranslatedStringVariable3 = verifyNot checkTranslatedStringVariable "$\"..\"" +prop_checkTranslatedStringVariable4 = verifyNot checkTranslatedStringVariable "var=val; $\"$var\"" +prop_checkTranslatedStringVariable5 = verifyNot checkTranslatedStringVariable "foo=var; bar=val2; $\"foo bar\"" +checkTranslatedStringVariable params (T_DollarDoubleQuoted id [T_Literal _ s]) = fromMaybe (return ()) $ do + guard $ all isVariableChar s Map.lookup s assignments return $ warnWithFix id 2256 "This translated string is the name of a variable. Flip leading $ and \" if this should be a quoted substitution." (fix id) @@ -3468,7 +3469,7 @@ checkTranslatedString params (T_DollarDoubleQuoted id ((T_Literal _ s):_)) = Map.insert name token insertAssignment _ = Prelude.id fix id = fixWith [replaceStart id params 2 "\"$"] -checkTranslatedString _ _ = return () +checkTranslatedStringVariable _ _ = return () prop_checkDefaultCase1 = verify checkDefaultCase "case $1 in a) true ;; esac" prop_checkDefaultCase2 = verify checkDefaultCase "case $1 in ?*?) true ;; *? ) true ;; esac"