From 8d5d0c24aa0d9dd087d8b7d6c1e4cd8ec786c6c5 Mon Sep 17 00:00:00 2001
From: Fabian Boehm <FHomborg@gmail.com>
Date: Fri, 13 Oct 2023 16:27:35 +0200
Subject: [PATCH] expand_cmdsubst: Make more errors known

These printed "Unknown error while evaluating command substitution".

Now they print something like

```
fish: for: status: cannot overwrite read-only variable
for status in foo; end
    ^~~~~^
in command substitution
fish: Invalid arguments
echo (for status in foo; end)
     ^~~~~~~~~~~~~~~~~~~~~~~^
```

for `echo (for status in foo; end)`

This is, of course, still not *great*. Mostly the `fish: Invalid
arguments` is basically entirely redundant.

An alternative is to simply skip the error message, but that requires some
more scaffolding (describe_with_prefix adds some error messages on its
own, so we can't simply say "don't add the prefix if we don't have a
message")

(cherry picked from commit 1b5eec2af6e71998c2af31830be5236aeb15ffd6)
(cherry picked from commit 67faa107b0993564ebbb5c0dd0e94d0c35b47975)
---
 src/expand.cpp                          | 17 ++++++++++
 tests/checks/syntax-error-location.fish | 45 +++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/src/expand.cpp b/src/expand.cpp
index e04077d2b..76e3ae8e7 100644
--- a/src/expand.cpp
+++ b/src/expand.cpp
@@ -665,6 +665,23 @@ static expand_result_t expand_cmdsubst(wcstring input, const operation_context_t
             case STATUS_NOT_EXECUTABLE:
                 err = L"Command not executable";
                 break;
+            case STATUS_INVALID_ARGS:
+                // TODO: Also overused
+                // This is sent for:
+                // invalid redirections or pipes (like `<&foo`),
+                // invalid variables (invalid name or read-only) for for-loops,
+                // switch $foo if $foo expands to more than one argument
+                // time in a background job.
+                err = L"Invalid arguments";
+                break;
+            case STATUS_EXPAND_ERROR:
+                // Sent in `for $foo in ...` if $foo expands to more than one word
+                err = L"Expansion error";
+                break;
+            case STATUS_UNMATCHED_WILDCARD:
+                // Sent in `for $foo in ...` if $foo expands to more than one word
+                err = L"Unmatched wildcard";
+                break;
             default:
                 err = L"Unknown error while evaluating command substitution";
                 break;
diff --git a/tests/checks/syntax-error-location.fish b/tests/checks/syntax-error-location.fish
index fb6fe03a8..e391bc13b 100644
--- a/tests/checks/syntax-error-location.fish
+++ b/tests/checks/syntax-error-location.fish
@@ -73,3 +73,48 @@ echo "bind -M" | $fish
 # CHECKERR: ^
 # CHECKERR: (Type 'help bind' for related documentation)
 
+$fish -c 'echo (for status in foo; end)'
+# CHECKERR: fish: for: status: cannot overwrite read-only variable
+# CHECKERR: for status in foo; end
+# CHECKERR: ^~~~~^
+# CHECKERR: in command substitution
+# CHECKERR: fish: Invalid arguments
+# CHECKERR: echo (for status in foo; end)
+# CHECKERR: ^~~~~~~~~~~~~~~~~~~~~~~^
+
+$fish -c 'echo (echo <&foo)'
+# CHECKERR: fish: Requested redirection to 'foo', which is not a valid file descriptor
+# CHECKERR: echo <&foo
+# CHECKERR: ^~~~^
+# CHECKERR: in command substitution
+# CHECKERR: fish: Invalid arguments
+# CHECKERR: echo (echo <&foo)
+# CHECKERR: ^~~~~~~~~~~^
+
+
+$fish -c 'echo (time echo foo &)'
+# CHECKERR: fish: 'time' is not supported for background jobs. Consider using 'command time'.
+# CHECKERR: time echo foo &
+# CHECKERR: ^~~~~~~~~~~~~~^
+# CHECKERR: in command substitution
+# CHECKERR: fish: Invalid arguments
+# CHECKERR: echo (time echo foo &)
+# CHECKERR: ^~~~~~~~~~~~~~~~^
+
+$fish -c 'echo (set -l foo 1 2 3; for $foo in foo; end)'
+# CHECKERR: fish: Unable to expand variable name ''
+# CHECKERR: set -l foo 1 2 3; for $foo in foo; end
+# CHECKERR: ^~~^
+# CHECKERR: in command substitution
+# CHECKERR: fish: Expansion error
+# CHECKERR: echo (set -l foo 1 2 3; for $foo in foo; end)
+# CHECKERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
+
+$fish -c 'echo (echo *nosuchname*)'
+# CHECKERR: fish: No matches for wildcard '*nosuchname*'. See `help wildcards-globbing`.
+# CHECKERR: echo *nosuchname*
+# CHECKERR: ^~~~~~~~~~~^
+# CHECKERR: in command substitution
+# CHECKERR: fish: Unmatched wildcard
+# CHECKERR: echo (echo *nosuchname*)
+# CHECKERR: ^~~~~~~~~~~~~~~~~~^