From b26a7bb22aca67c8162c8e19ebb70c87ceaca40b Mon Sep 17 00:00:00 2001
From: Calvin Rose <calsrose@gmail.com>
Date: Tue, 24 Sep 2019 13:23:18 -0500
Subject: [PATCH] Disallow the empty string for some string fns.

This will prevent these functions from being run
with empty strings, which usually produces useless
output, as the internal string search algorithm will
never "find" empty strings. This is by design, as it is
not always obvious which empty strings should be found in
the search text.
---
 CHANGELOG.md      |  2 ++
 src/core/string.c | 17 ++++++++++-------
 test/suite2.janet |  9 +++++++--
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 891782c6..1a29ff03 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 All notable changes to this project will be documented in this file.
 
 ## Unreleased
+- `string/` functions that take a pattern to search for will throw an error
+  when receiving the empty string.
 - Replace (start:end) style stacktrace source position information with
   line, column. This should be more readable for humans. Also, range information
   can be recovered by re-parsing source.
diff --git a/src/core/string.c b/src/core/string.c
index c02b2bc7..44f943e2 100644
--- a/src/core/string.c
+++ b/src/core/string.c
@@ -108,6 +108,9 @@ static void kmp_init(
     if (!lookup) {
         JANET_OUT_OF_MEMORY;
     }
+    if (patlen == 0) {
+        janet_panic("expected non-empty pattern");
+    }
     s->lookup = lookup;
     s->i = 0;
     s->j = 0;
@@ -378,15 +381,13 @@ static Janet cfun_string_split(int32_t argc, Janet *argv) {
     }
     findsetup(argc, argv, &state, 1);
     array = janet_array(0);
-    while ((result = kmp_next(&state)) >= 0 && limit--) {
+    while ((result = kmp_next(&state)) >= 0 && --limit) {
         const uint8_t *slice = janet_string(state.text + lastindex, result - lastindex);
         janet_array_push(array, janet_wrap_string(slice));
         lastindex = result + state.patlen;
     }
-    {
-        const uint8_t *slice = janet_string(state.text + lastindex, state.textlen - lastindex);
-        janet_array_push(array, janet_wrap_string(slice));
-    }
+    const uint8_t *slice = janet_string(state.text + lastindex, state.textlen - lastindex);
+    janet_array_push(array, janet_wrap_string(slice));
     kmp_deinit(&state);
     return janet_wrap_array(array);
 }
@@ -600,10 +601,12 @@ static const JanetReg string_cfuns[] = {
     },
     {
         "string/split", cfun_string_split,
-        JDOC("(string/split delim str)\n\n"
+        JDOC("(string/split delim str &opt start limit)\n\n"
              "Splits a string str with delimiter delim and returns an array of "
              "substrings. The substrings will not contain the delimiter delim. If delim "
-             "is not found, the returned array will have one element.")
+             "is not found, the returned array will have one element. Will start searching "
+             "for delim at the index start (if provided), and return up to a maximum "
+             "of limit results (if provided).")
     },
     {
         "string/check-set", cfun_string_checkset,
diff --git a/test/suite2.janet b/test/suite2.janet
index ed747348..c0428019 100644
--- a/test/suite2.janet
+++ b/test/suite2.janet
@@ -62,8 +62,7 @@
 
 # String functions
 (assert (= 3 (string/find "abc" "   abcdefghijklmnop")) "string/find 1")
-(assert (= nil (string/find "" "")) "string/find 2")
-(assert (= 0 (string/find "A" "A")) "string/find 3")
+(assert (= 0 (string/find "A" "A")) "string/find 2")
 (assert (string/has-prefix? "" "foo") "string/has-prefix? 1")
 (assert (string/has-prefix? "fo" "foo") "string/has-prefix? 2")
 (assert (not (string/has-prefix? "o" "foo")) "string/has-prefix? 3")
@@ -98,6 +97,12 @@
 (assert (deep= (string/find-all "e" "onetwothree") @[2 9 10]) "string/find-all 1")
 (assert (deep= (string/find-all "," "onetwothree") @[]) "string/find-all 2")
 
+(assert-error "string/find error 1" (string/find "" "abcd"))
+(assert-error "string/split error 1" (string/split "" "abcd"))
+(assert-error "string/replace error 1" (string/replace "" "." "abcd"))
+(assert-error "string/replace-all error 1" (string/replace-all "" "." "abcdabcd"))
+(assert-error "string/find-all error 1" (string/find-all "" "abcd"))
+
 # Check if abstract test works
 (assert (abstract? stdout) "abstract? stdout")
 (assert (abstract? stdin) "abstract? stdin")