Fix out of range error for non-negative string indexing offsets (#1052)

* Fix out of range error for non-negative offsets * Update docs and failing conformance test
google · Oct 28, 2024 · ba36ff8 · ba36ff8
1 parent 3338c3f
commit ba36ff8
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 12 deletions.
diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel
@@ -47,6 +47,9 @@ _TESTS_TO_SKIP = [
     "macros/map/map_extract_keys",
     "timestamps/duration_converters/get_milliseconds",
 
+    # Temporarily failing tests, need a spec update
+    "string_ext/value_errors/indexof_out_of_range,lastindexof_out_of_range",
+
     # Future enhancments.
     "enums/strong_proto2",
     "enums/strong_proto3",

diff --git a/ext/README.md b/ext/README.md
@@ -593,7 +593,8 @@ Examples:
     'hello mellow'.indexOf('jello')    // returns -1
     'hello mellow'.indexOf('', 2)      // returns 2
     'hello mellow'.indexOf('ello', 2)  // returns 7
-    'hello mellow'.indexOf('ello', 20) // error
+    'hello mellow'.indexOf('ello', 20) // returns -1
+    'hello mellow'.indexOf('ello', -1) // error
 
 ### Join
 
@@ -631,6 +632,7 @@ Examples:
     'hello mellow'.lastIndexOf('ello')     // returns 7
     'hello mellow'.lastIndexOf('jello')    // returns -1
     'hello mellow'.lastIndexOf('ello', 6)  // returns 1
+    'hello mellow'.lastIndexOf('ello', 20) // returns -1
     'hello mellow'.lastIndexOf('ello', -1) // error
 
 ### LowerAscii

diff --git a/ext/strings.go b/ext/strings.go
@@ -119,7 +119,8 @@ const (
 //	'hello mellow'.indexOf('jello')    // returns -1
 //	'hello mellow'.indexOf('', 2)      // returns 2
 //	'hello mellow'.indexOf('ello', 2)  // returns 7
-//	'hello mellow'.indexOf('ello', 20) // error
+//	'hello mellow'.indexOf('ello', 20) // returns -1
+//	'hello mellow'.indexOf('ello', -1) // error
 //
 // # Join
 //
@@ -155,6 +156,7 @@ const (
 //	'hello mellow'.lastIndexOf('ello')     // returns 7
 //	'hello mellow'.lastIndexOf('jello')    // returns -1
 //	'hello mellow'.lastIndexOf('ello', 6)  // returns 1
+//	'hello mellow'.lastIndexOf('ello', 20) // returns -1
 //	'hello mellow'.lastIndexOf('ello', -1) // error
 //
 // # LowerAscii
@@ -561,9 +563,13 @@ func indexOfOffset(str, substr string, offset int64) (int64, error) {
 	off := int(offset)
 	runes := []rune(str)
 	subrunes := []rune(substr)
-	if off < 0 || off >= len(runes) {
+	if off < 0 {
 		return -1, fmt.Errorf("index out of range: %d", off)
 	}
+	// If the offset exceeds the length, return -1 rather than error.
+	if off >= len(runes) {
+		return -1, nil
+	}
 	for i := off; i < len(runes)-(len(subrunes)-1); i++ {
 		found := true
 		for j := 0; j < len(subrunes); j++ {
@@ -594,9 +600,13 @@ func lastIndexOfOffset(str, substr string, offset int64) (int64, error) {
 	off := int(offset)
 	runes := []rune(str)
 	subrunes := []rune(substr)
-	if off < 0 || off >= len(runes) {
+	if off < 0 {
 		return -1, fmt.Errorf("index out of range: %d", off)
 	}
+	// If the offset is far greater than the length return -1
+	if off >= len(runes) {
+		return -1, nil
+	}
 	if off > len(runes)-len(subrunes) {
 		off = len(runes) - len(subrunes)
 	}

diff --git a/ext/strings_test.go b/ext/strings_test.go
@@ -150,18 +150,12 @@ var stringTests = []struct {
 		expr: `'tacocat'.charAt(30) == ''`,
 		err:  "index out of range: 30",
 	},
-	{
-		expr: `'tacocat'.indexOf('a', 30) == -1`,
-		err:  "index out of range: 30",
-	},
+	{expr: `'tacocat'.indexOf('a', 30) == -1`},
 	{
 		expr: `'tacocat'.lastIndexOf('a', -1) == -1`,
 		err:  "index out of range: -1",
 	},
-	{
-		expr: `'tacocat'.lastIndexOf('a', 30) == -1`,
-		err:  "index out of range: 30",
-	},
+	{expr: `'tacocat'.lastIndexOf('a', 30) == -1`},
 	{
 		expr: `"tacocat".substring(40) == "cat"`,
 		err:  "index out of range: 40",