From 264d8270a73d2aae69285c64e0c8270f029621a6 Mon Sep 17 00:00:00 2001
From: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Mon, 1 Oct 2018 20:54:55 -0500
Subject: [PATCH] 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 |  2 +-
 src/expand.cpp               | 22 +++++++++++++++++++++-
 tests/zero_based_array.err   |  9 +++++++++
 tests/zero_based_array.in    |  3 +++
 tests/zero_based_array.out   |  0
 5 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 tests/zero_based_array.err
 create mode 100644 tests/zero_based_array.in
 create mode 100644 tests/zero_based_array.out

diff --git a/share/completions/rsync.fish b/share/completions/rsync.fish
index 441db0e26..df6bc8b57 100644
--- a/share/completions/rsync.fish
+++ b/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)"
diff --git a/src/expand.cpp b/src/expand.cpp
index a85f8d05f..ae7410c25 100644
--- a/src/expand.cpp
+++ b/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);
diff --git a/tests/zero_based_array.err b/tests/zero_based_array.err
new file mode 100644
index 000000000..08ed84e77
--- /dev/null
+++ b/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 ]
+           ^
diff --git a/tests/zero_based_array.in b/tests/zero_based_array.in
new file mode 100644
index 000000000..5eb57a66b
--- /dev/null
+++ b/tests/zero_based_array.in
@@ -0,0 +1,3 @@
+echo $foo[0]
+echo $foo[ 0 ]
+echo $foo[ 00 ]
diff --git a/tests/zero_based_array.out b/tests/zero_based_array.out
new file mode 100644
index 000000000..e69de29bb