Emit an error message on literal zero indices

Mostly resolves #4862, though there remains the lingering question of
whether or not to emit a warning to /dev/tty or stderr when a
non-literal-zero index evaluates to zero.

share/completions/rsync.fish

@@ -47,7 +47,7 @@ complete -c rsync -l rsync-path -x -d "Specify the rsync to run on remote machin
 complete -c rsync -l existing -d "Ignore non-existing files on receiving side"
 complete -c rsync -l ignore-existing -d "Ignore files that already exist on receiver"
 complete -c rsync -l remove-sent-files -d "Sent files/symlinks are removed from sender"
-complete -c rsync -l remove-source-files -d "Remove all files from source/sender after sync"
+complete -c rsync -l remove-source-files -d "Remove all synced files from source/sender"
 complete -c rsync -l del -d "An alias for --delete-during"
 complete -c rsync -l delete -d "Delete files that don’t exist on sender"
 complete -c rsync -l delete-before -d "Receiver deletes before transfer (default)"

src/expand.cpp

@@ -185,6 +185,9 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long
     const long size = (long)array_size;
     size_t pos = 1;  // skip past the opening square brace
 
+    int zero_index = -1;
+    bool literal_zero_index = true;
+
     while (1) {
         while (iswspace(in[pos]) || (in[pos] == INTERNAL_SEPARATOR)) pos++;
         if (in[pos] == L']') {
@@ -192,6 +195,14 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long
             break;
         }
 
+        // Explicitly refuse $foo[0] as valid syntax, regardless of whether or not we're going
+        // to show an error if the index ultimately evaluates to zero. This will help newcomers
+        // to fish avoid a common off-by-one error. See #4862.
+        if (literal_zero_index && in[pos] == L'0') {
+            zero_index = pos;
+            literal_zero_index = true;
+        }
+
         const size_t i1_src_pos = pos;
         const wchar_t *end;
         long tmp = fish_wcstol(&in[pos], &end);
@@ -250,6 +261,10 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long
         source_positions.push_back(i1_src_pos);
     }
 
+    if (literal_zero_index && zero_index != -1) {
+        return zero_index;
+    }
+
     if (end_ptr) {
         *end_ptr = (wchar_t *)(in + pos);
     }
@@ -363,7 +378,12 @@ static bool expand_variables(const wcstring &instr, std::vector<completion_t> *o
         size_t bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list,
                                      effective_val_count);
         if (bad_pos != 0) {
-            append_syntax_error(errors, slice_start + bad_pos, L"Invalid index value");
+            if (in[slice_start + bad_pos] == L'0') {
+                append_syntax_error(errors, slice_start + bad_pos,
+                        L"array indices start at 1, not 0.");
+            } else {
+                append_syntax_error(errors, slice_start + bad_pos, L"Invalid index value");
+            }
             return false;
         }
         var_name_and_slice_stop = (slice_end - in);

tests/zero_based_array.err

@@ -0,0 +1,9 @@
+fish: array indices start at 1, not 0.
+echo $foo[0]
+          ^
+fish: array indices start at 1, not 0.
+echo $foo[ 0 ]
+           ^
+fish: array indices start at 1, not 0.
+echo $foo[ 00 ]
+           ^

tests/zero_based_array.in

@@ -0,0 +1,3 @@
+echo $foo[0]
+echo $foo[ 0 ]
+echo $foo[ 00 ]