Fix regression expanding \$()

When expanding command substitutions, we use a naïve way of detecting whether
the cmdsub has the optional leading dollar. We check if the last character was
a dollar, which breaks if it's an escaped dollar.  We wrongly expand
\$(echo "") to the empty string. Fix this by checking if the dollar was escaped.

The parse_util_* functions have a bunch of output parameters. We should
return a parameter bag instead (I think I tried once and failed).

CHANGELOG.rst

@@ -20,6 +20,7 @@ Deprecations and removed features
 Scripting improvements
 ----------------------
 - Quoted command substitution that directly follow a variable expansion (like ``echo "$var$(echo x)"``) no longer affect the variable expansion (:issue:`8849`).
+- Fish now correctly expands command substitutions that are preceded by an escaped dollar (like ``echo \$(echo)``). This regressed in version 3.4.0.
 - ``math`` can now handle underscores (``_``) as visual separators in numbers (:issue:`8611`, :issue:`8496`)::
 
     math 5 + 2_123_252

src/expand.cpp

@@ -626,8 +626,9 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t
     wcstring subcmd;
 
     bool is_quoted = false;
+    bool has_dollar = false;
     switch (parse_util_locate_cmdsubst_range(input, &cursor, &subcmd, &paren_begin, &paren_end,
-                                             false, &is_quoted)) {
+                                             false, &is_quoted, &has_dollar)) {
         case -1: {
             append_syntax_error(errors, SOURCE_LOCATION_UNKNOWN, L"Mismatched parenthesis");
             return expand_result_t::make_error(STATUS_EXPAND_ERROR);
@@ -646,8 +647,6 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t
         }
     }
 
-    bool have_dollar = paren_begin > 0 && input.at(paren_begin - 1) == L'$';
-
     wcstring_list_t sub_res;
     int subshell_status = exec_subshell_for_expand(subcmd, *ctx.parser, ctx.job_group, sub_res);
     if (subshell_status != 0) {
@@ -757,7 +756,7 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t
             wcstring whole_item;
             whole_item.reserve(paren_begin + 1 + sub_res_joined.size() + 1 +
                                tail_item.completion.size());
-            whole_item.append(input, 0, paren_begin - have_dollar);
+            whole_item.append(input, 0, paren_begin - has_dollar);
             whole_item.push_back(INTERNAL_SEPARATOR);
             whole_item.append(sub_res_joined);
             whole_item.push_back(INTERNAL_SEPARATOR);
@@ -776,7 +775,7 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t
             wcstring whole_item;
             whole_item.reserve(paren_begin + 1 + sub_item2.size() + 1 +
                                tail_item.completion.size());
-            whole_item.append(input, 0, paren_begin - have_dollar);
+            whole_item.append(input, 0, paren_begin - has_dollar);
             whole_item.push_back(INTERNAL_SEPARATOR);
             whole_item.append(sub_item2);
             whole_item.push_back(INTERNAL_SEPARATOR);

src/parse_util.cpp

@@ -91,7 +91,8 @@ size_t parse_util_get_offset(const wcstring &str, int line, long line_offset) {
 }
 
 static int parse_util_locate_cmdsub(const wchar_t *in, const wchar_t **begin, const wchar_t **end,
-                                    bool allow_incomplete, bool *inout_is_quoted) {
+                                    bool allow_incomplete, bool *inout_is_quoted,
+                                    bool *out_has_dollar) {
     bool escaped = false;
     bool is_first = true;
     bool is_token_begin = true;
@@ -104,10 +105,12 @@ static int parse_util_locate_cmdsub(const wchar_t *in, const wchar_t **begin, co
     assert(in && "null parameter");
 
     const wchar_t *pos = in;
+    const wchar_t *last_dollar = nullptr;
     auto process_opening_quote = [&](wchar_t quote) -> bool /* ok */ {
         const wchar_t *q_end = quote_end(pos, quote);
         if (!q_end) return false;
         if (*q_end == L'$') {
+            last_dollar = q_end;
             quoted_cmdsubs.push_back(paran_count);
         }
         // We want to report whether the outermost comand substitution between
@@ -131,10 +134,15 @@ static int parse_util_locate_cmdsub(const wchar_t *in, const wchar_t **begin, co
                 escaped = true;
             } else if (*pos == L'#' && is_token_begin) {
                 pos = comment_end(pos) - 1;
+            } else if (*pos == L'$') {
+                last_dollar = pos;
             } else {
                 if (*pos == L'(') {
                     if ((paran_count == 0) && (paran_begin == nullptr)) {
                         paran_begin = pos;
+                        if (out_has_dollar) {
+                            *out_has_dollar = last_dollar == pos - 1;
+                        }
                     }
 
                     paran_count++;
@@ -151,7 +159,7 @@ static int parse_util_locate_cmdsub(const wchar_t *in, const wchar_t **begin, co
                         break;
                     }
 
-                    // Check if the ) did complete a quoted command substituion.
+                    // Check if the ) did complete a quoted command substitution.
                     if (!quoted_cmdsubs.empty() && quoted_cmdsubs.back() == paran_count) {
                         quoted_cmdsubs.pop_back();
                         // Quoted command substitutions temporarily close double quotes.
@@ -244,7 +252,8 @@ long parse_util_slice_length(const wchar_t *in) {
 
 int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_offset,
                                      wcstring *out_contents, size_t *out_start, size_t *out_end,
-                                     bool accept_incomplete, bool *inout_is_quoted) {
+                                     bool accept_incomplete, bool *inout_is_quoted,
+                                     bool *out_has_dollar) {
     // Clear the return values.
     if (out_contents != nullptr) out_contents->clear();
     *out_start = 0;
@@ -261,7 +270,7 @@ int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_o
     const wchar_t *bracket_range_end = nullptr;
 
     int ret = parse_util_locate_cmdsub(valid_range_start, &bracket_range_begin, &bracket_range_end,
-                                       accept_incomplete, inout_is_quoted);
+                                       accept_incomplete, inout_is_quoted, out_has_dollar);
     if (ret <= 0) {
         return ret;
     }
@@ -303,7 +312,7 @@ void parse_util_cmdsubst_extent(const wchar_t *buff, size_t cursor_pos, const wc
     const wchar_t *pos = buff;
     for (;;) {
         const wchar_t *begin = nullptr, *end = nullptr;
-        if (parse_util_locate_cmdsub(pos, &begin, &end, true, nullptr) <= 0) {
+        if (parse_util_locate_cmdsub(pos, &begin, &end, true, nullptr, nullptr) <= 0) {
             // No subshell found, all done.
             break;
         }

src/parse_util.h

@@ -30,10 +30,12 @@ long parse_util_slice_length(const wchar_t *in);
 /// the end of the string if it was incomplete
 /// \param accept_incomplete whether to permit missing closing parenthesis
 /// \param inout_is_quoted whether the cursor is in a double-quoted context.
+/// \param out_has_dollar whether the command substitution has the optional leading $.
 /// \return -1 on syntax error, 0 if no subshells exist and 1 on success
 int parse_util_locate_cmdsubst_range(const wcstring &str, size_t *inout_cursor_offset,
                                      wcstring *out_contents, size_t *out_start, size_t *out_end,
-                                     bool accept_incomplete, bool *inout_is_quoted = nullptr);
+                                     bool accept_incomplete, bool *inout_is_quoted = nullptr,
+                                     bool *out_has_dollar = nullptr);
 
 /// Find the beginning and end of the command substitution under the cursor. If no subshell is
 /// found, the entire string is returned. If the current command substitution is not ended, i.e. the

tests/checks/cmdsub.fish

@@ -66,3 +66,11 @@ echo "quoted1""quoted2"(echo unquoted3)"$(echo quoted4)_$(echo quoted5)"
 
 var=a echo "$var$(echo b)"
 # CHECK: ab
+
+# Make sure we don't swallow an escaped dollar.
+echo \$(echo 1)
+# CHECK: $1
+echo "\$(echo 1)"
+# CHECK: $(echo 1)
+echo "\$$(echo 1)"
+# CHECK: $1