From d58ce336e645cf5fc0f3c84b5847fdd88dae2fca Mon Sep 17 00:00:00 2001
From: Ryan Hendrickson <ryan.hendrickson@alum.mit.edu>
Date: Fri, 25 Jul 2025 15:32:02 -0400
Subject: [PATCH] Suggest removing null checks in parameter expansions

${parameter-} expands to the empty string if parameter is unset.
${parameter:-} is a more verbose way of saying the same thing, because
it expands to the empty string if parameter is unset or empty, and if
parameter is empty, it merely replaces one empty with another.

Similarly, ${parameter=} assigns the empty string to parameter if
parameter is unset, which is equivalent to what ${parameter:=} does,
since assigning the empty string to a parameter already holding the
empty string is a no-op.

(Of course, when the replacement to the right of the - or = is
non-empty, there is a meaningful difference between the versions with
and without the null-check colon.)
---
 CHANGELOG.md                |  1 +
 src/ShellCheck/Analytics.hs | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3f834aa..30451af 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - SC2330: Warn about unsupported glob matches with [[ .. ]] in BusyBox.
 - SC2331: Suggest using standard -e instead of unary -a in tests.
 - SC2332: Warn about `[ ! -o opt ]` being unconditionally true in Bash.
+- SC2335: In parameter expansions, suggest replacing `:-`/`:=` with `-`/`=` when the substitution is empty.
 - SC3062: Warn about bashism `[ -o opt ]`.
 - Precompiled binaries for Linux riscv64 (linux.riscv64)
 ### Changed
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 6d23344..81a7f05 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -1808,13 +1808,17 @@ prop_checkBadParameterSubstitution8 = verify checkBadParameterSubstitution "${$(
 prop_checkBadParameterSubstitution9 = verifyNot checkBadParameterSubstitution "$# ${#} $! ${!} ${!#} ${#!}"
 prop_checkBadParameterSubstitution10 = verify checkBadParameterSubstitution "${'foo'}"
 prop_checkBadParameterSubstitution11 = verify checkBadParameterSubstitution "${${x%.*}##*/}"
+prop_checkBadParameterSubstitution12 = verify checkBadParameterSubstitution "${var:-}"
+prop_checkBadParameterSubstitution13 = verifyNot checkBadParameterSubstitution "${var:-x} ${var-} ${var:-$(cmd)}"
+prop_checkBadParameterSubstitution14 = verify checkBadParameterSubstitution "${var:=}"
+prop_checkBadParameterSubstitution15 = verifyNot checkBadParameterSubstitution "${var:=x} ${var=} ${var:=$(cmd)}"
 
-checkBadParameterSubstitution _ t =
+checkBadParameterSubstitution params t =
     case t of
-        (T_DollarBraced i _ (T_NormalWord _ contents@(first:_))) ->
+        (T_DollarBraced i _ (T_NormalWord _ contents@(first:more))) ->
             if isIndirection contents
             then err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval."
-            else checkFirst first
+            else checkFirst first more
         _ -> return ()
 
   where
@@ -1833,11 +1837,15 @@ checkBadParameterSubstitution _ t =
                                     else Just False
                   _ -> Just False
 
-    checkFirst t =
+    checkFirst t more =
         case t of
-            T_Literal id (c:_) ->
+            T_Literal id s@(c:_) ->
                 if isVariableChar c || isSpecialVariableChar c
-                then return ()
+                then case reverse s of
+                    x : ':' : _ | [] <- more, x `elem` "-=" -> -- i.e., s ends with ":-" or ":="
+                        styleWithFix id 2335 ("Checking for null is unnecessary when replacing with null. Remove the colon.") $
+                            fixWith [ replaceEnd id params 2 [x] ]
+                    _ -> return ()
                 else err id 2296 $ "Parameter expansions can't start with " ++ e4m [c] ++ ". Double check syntax."
 
             T_ParamSubSpecialChar {} -> return ()