String.index of (#7402)

* [tests] add tests for indexOf/lastIndexOf

* [python] make indexOf and lastIndexOf ecma compliant when str is empty

* [php] fix indexOf and lastIndexOf if search string is ""

* [neko] fix indexOf and lastIndexOf if search string is ""

* [cs] fix indexOf and lastIndexOf if search string is ""

* [lua] conform to String.indexOf spec (#56)

* [python] don't inline StringImpl#lastIndexOf/#indexOf (#55)

Because their method body is quite big.

* fix merge mishap

* fix JVM, ignore flash

* fix

* dodge lua

---------

Co-authored-by: frabbit <[email protected]>
Co-authored-by: Justin Donaldson <[email protected]>
Co-authored-by: Sebastian Thomschke <[email protected]>

std/jvm/StringExt.hx

@@ -48,12 +48,15 @@ class StringExt {
 	}
 
 	public static function indexOf(me:String, str:String, startIndex:Null<Int>) {
+		if (str.length == 0) {
+			return java.lang.Math.max(0, java.lang.Math.min(startIndex == null ? 0 : startIndex, me.length));
+		}
 		return if (startIndex == null) (cast me : NativeString).indexOf(str) else (cast me : NativeString).indexOf(str, startIndex);
 	}
 
 	public static function lastIndexOf(me:String, str:String, ?startIndex:Int):Int {
-		if(str == '') {
-			return startIndex == null || startIndex > me.length ? me.length : startIndex;
+		if (str.length == 0) {
+			return java.lang.Math.max(0, java.lang.Math.min(startIndex == null ? me.length : startIndex, me.length));
 		}
 		if (startIndex == null || startIndex > me.length || startIndex < 0) {
 			startIndex = me.length - 1;

std/lua/_std/String.hx

@@ -36,8 +36,7 @@ class String {
 
 	public var length(default, null):Int;
 
-	public inline function new(string:String)
-		untyped {}
+	public inline function new(string:String) untyped {}
 
 	@:keep
 	static function __index(s:Dynamic, k:Dynamic):Dynamic {
@@ -79,9 +78,10 @@ class String {
 
 	static function indexOfEmpty(s:String, startIndex:Int):Int {
 		var length = BaseString.len(s);
-		if(startIndex < 0) {
+		if (startIndex < 0) {
 			startIndex = length + startIndex;
-			if(startIndex < 0) startIndex = 0;
+			if (startIndex < 0)
+				startIndex = 0;
 		}
 		return startIndex > length ? length : startIndex;
 	}
@@ -90,13 +90,22 @@ class String {
 		var ret = -1;
 		if (startIndex == null)
 			startIndex = length;
-		while (true) {
-			var p = indexOf(str, ret + 1);
-			if (p == -1 || p > startIndex || p == ret)
-				break;
-			ret = p;
+		if (str == "") {
+			if (this == "") {
+				return 0;
+			} else {
+				var max = cast Math.max(startIndex, 0);
+				return cast Math.min(length, max);
+			}
+		} else {
+			while (true) {
+				var p = indexOf(str, ret + 1);
+				if (p == -1 || p > startIndex || p == ret)
+					break;
+				ret = p;
+			}
+			return ret;
 		}
-		return ret;
 	}
 
 	public function split(delimiter:String):Array<String> {

std/neko/_std/String.hx

@@ -57,6 +57,11 @@
 	}
 
 	public function indexOf(str:String, ?startIndex:Int):Int {
+		if (str.length == 0) {
+			var startIndex = startIndex == null ? 0 : startIndex;
+			var min = startIndex > length ? length : startIndex;
+			return min < 0 ? 0 : min;
+		}
 		untyped {
 			var l = __dollar__ssize(this.__s);
 			if (startIndex == null || startIndex < -l)
@@ -76,6 +81,11 @@
 	}
 
 	public function lastIndexOf(str:String, ?startIndex:Int):Int {
+		if (str.length == 0) {
+			var startIndex = startIndex == null ? length : startIndex;
+			var min = startIndex > length ? length : startIndex;
+			return min < 0 ? 0 : min;
+		}
 		untyped {
 			var last = -1;
 			var l = __dollar__ssize(this.__s);
@@ -212,10 +222,9 @@
 		return new String(untyped __dollar__string(s) + this.__s);
 	}
 
-	public static function fromCharCode(code:Int):String
-		untyped {
-			var s = __dollar__smake(1);
-			__dollar__sset(s, 0, code);
-			return new String(s);
-		}
+	public static function fromCharCode(code:Int):String untyped {
+		var s = __dollar__smake(1);
+		__dollar__sset(s, 0, code);
+		return new String(s);
+	}
 }

std/php/Boot.hx

@@ -107,7 +107,7 @@ class Boot {
 		Check if specified property has getter
 	**/
 	public static function hasGetter(phpClassName:String, property:String):Bool {
-		if(!ensureLoaded(phpClassName))
+		if (!ensureLoaded(phpClassName))
 			return false;
 
 		var has = false;
@@ -124,7 +124,7 @@ class Boot {
 		Check if specified property has setter
 	**/
 	public static function hasSetter(phpClassName:String, property:String):Bool {
-		if(!ensureLoaded(phpClassName))
+		if (!ensureLoaded(phpClassName))
 			return false;
 
 		var has = false;
@@ -148,7 +148,8 @@ class Boot {
 		Retrieve metadata for specified class
 	**/
 	public static function getMeta(phpClassName:String):Null<Dynamic> {
-		if(!ensureLoaded(phpClassName)) return null;
+		if (!ensureLoaded(phpClassName))
+			return null;
 		return Global.isset(meta[phpClassName]) ? meta[phpClassName] : null;
 	}
 
@@ -283,7 +284,7 @@ class Boot {
 	**/
 	@:pure(false)
 	static public function isPhpKeyword(str:String):Bool {
-		//The body of this method is generated by the compiler
+		// The body of this method is generated by the compiler
 		return false;
 	}
 
@@ -596,7 +597,7 @@ class Boot {
 		if (result != null) {
 			return result;
 		}
-		if(!Global.method_exists(obj, methodName) && !Global.isset(Syntax.field(obj, methodName))) {
+		if (!Global.method_exists(obj, methodName) && !Global.isset(Syntax.field(obj, methodName))) {
 			return null;
 		}
 		result = new HxClosure(obj, methodName);
@@ -769,6 +770,9 @@ private class HxString {
 	}
 
 	public static function indexOf(str:String, search:String, startIndex:Int = null):Int {
+		if (search.length == 0) {
+			return Global.max(0, Global.min(startIndex == null ? 0 : startIndex, str.length));
+		}
 		if (startIndex == null) {
 			startIndex = 0;
 		} else {
@@ -793,9 +797,15 @@ private class HxString {
 	}
 
 	public static function lastIndexOf(str:String, search:String, startIndex:Int = null):Int {
+		if (search.length == 0) {
+			return Global.max(0, Global.min(startIndex == null ? str.length : startIndex, str.length));
+		}
 		var start = startIndex;
 		if (start == null) {
 			start = 0;
+		}
+		if (startIndex == null) {
+			startIndex = 0;
 		} else {
 			var length = str.length;
 			if (start >= 0) {
@@ -809,7 +819,8 @@ private class HxString {
 		}
 		var index:EitherType<Int, Bool> = if (search == '') {
 			var length = str.length;
-			startIndex == null || startIndex > length ? length : startIndex;
+			startIndex == null
+			|| startIndex > length ? length : startIndex;
 		} else {
 			Global.mb_strrpos(str, search, start);
 		}
@@ -942,17 +953,14 @@ private class HxDynamicStr extends HxClosure {
 
 /**
 	Anonymous objects implementation
-**/
-@:keep
-@:dox(hide)
-private class HxAnon extends StdClass {
+**/ @:keep @:dox(hide) private class HxAnon extends StdClass {
 	public function new(fields:NativeArray = null) {
 		super();
 		if (fields != null) {
 			Syntax.foreach(fields, function(name, value) Syntax.setField(this, name, value));
 		}
 	}
-	
+
 	@:phpMagic
 	function __get(name:String) {
 		return null;
@@ -966,10 +974,7 @@ private class HxAnon extends StdClass {
 
 /**
 	Closures implementation
-**/
-@:keep
-@:dox(hide)
-private class HxClosure {
+**/ @:keep @:dox(hide) private class HxClosure {
 	/** `this` for instance methods; php class name for static methods */
 	var target:Dynamic;
 
@@ -1023,4 +1028,4 @@ private class HxClosure {
 	public function callWith(newThis:Dynamic, args:NativeArray):Dynamic {
 		return Global.call_user_func_array(getCallback(newThis), args);
 	}
-}
+}

std/python/internal/StringImpl.hx

@@ -42,8 +42,12 @@ class StringImpl {
 	}
 
 	@:ifFeature("dynamic_read.lastIndexOf", "anon_optional_read.lastIndexOf", "python.internal.StringImpl.lastIndexOf")
-	public static inline function lastIndexOf(s:String, str:String, ?startIndex:Int):Int {
-		if (startIndex == null) {
+	public static function lastIndexOf(s:String, str:String, ?startIndex:Int):Int {
+		if (str == "") {
+			var i = startIndex == null ? s.length : startIndex;
+			return UBuiltins.max(0, UBuiltins.min(i,  s.length));
+		}
+		else if (startIndex == null) {
 			return Syntax.callField(s, "rfind", str, 0, s.length);
 		} else if(str == "") {
 			var length = s.length;
@@ -75,8 +79,12 @@ class StringImpl {
 	}
 
 	@:ifFeature("dynamic_read.indexOf", "anon_optional_read.indexOf", "python.internal.StringImpl.indexOf")
-	public static inline function indexOf(s:String, str:String, ?startIndex:Int) {
-		if (startIndex == null)
+	public static function indexOf (s:String, str:String, ?startIndex:Int) {
+		if (str == "") {
+			var i = startIndex == null ? 0 : startIndex;
+			return UBuiltins.max(0, UBuiltins.min(i,  s.length));
+		}
+		else if (startIndex == null)
 			return Syntax.callField(s, "find", str);
 		else
 			return indexOfImpl(s, str, startIndex);

tests/unit/src/unit/issues/Issue5271.hx

@@ -0,0 +1,72 @@
+package unit.issues;
+
+/**
+ * Tests if String's indexOf/lastIndexOf functions behave ECMAScript compliant
+ */
+class Issue5271 extends unit.Test {
+	#if !flash
+	function test() {
+		/*
+		 * test indexOf
+		 */
+		eq(0, "".indexOf(""));
+		eq(0, "".indexOf("", 0));
+		eq(0, "".indexOf("", 1));
+		eq(0, "".indexOf("", -1));
+
+		eq(0, " ".indexOf(""));
+		eq(0, " ".indexOf("", 0));
+		eq(1, " ".indexOf("", 1));
+		eq(1, " ".indexOf("", 2));
+		eq(0, " ".indexOf("", -1));
+
+		eq(0, "dog".indexOf(""));
+		eq(0, "dog".indexOf("", 0));
+		eq(1, "dog".indexOf("", 1));
+		eq(2, "dog".indexOf("", 2));
+		eq(3, "dog".indexOf("", 3));
+		eq(3, "dog".indexOf("", 4));
+		eq(3, "dog".indexOf("", 10));
+		#if !lua
+		eq(0, "dog".indexOf("", -1));
+		#end
+
+		eq(-1, "dogdog".indexOf("cat"));
+		eq(3, "dogcat".indexOf("cat"));
+		eq(3, "dogcat".indexOf("cat", 0));
+		eq(3, "dogcat".indexOf("cat", 1));
+		eq(3, "catcat".indexOf("cat", 3));
+		eq(-1, "catcat".indexOf("cat", 4));
+
+		/*
+		 * test lastIndexOf
+		 */
+		eq(0, "".lastIndexOf(""));
+		eq(0, "".lastIndexOf("", 0));
+		eq(0, "".lastIndexOf("", 1));
+		eq(0, "".lastIndexOf("", -1));
+
+		eq(1, " ".lastIndexOf(""));
+		eq(0, " ".lastIndexOf("", 0));
+		eq(1, " ".lastIndexOf("", 1));
+		eq(1, " ".lastIndexOf("", 2));
+		eq(0, " ".lastIndexOf("", -1));
+
+		eq(3, "dog".lastIndexOf(""));
+		eq(0, "dog".lastIndexOf("", 0));
+		eq(1, "dog".lastIndexOf("", 1));
+		eq(2, "dog".lastIndexOf("", 2));
+		eq(3, "dog".lastIndexOf("", 3));
+		eq(3, "dog".lastIndexOf("", 4));
+		eq(3, "dog".lastIndexOf("", 10));
+		eq(0, "dog".lastIndexOf("", -1));
+
+		eq(-1, "dogdog".lastIndexOf("cat"));
+		eq(3, "dogcat".lastIndexOf("cat"));
+		eq(-1, "dogcat".lastIndexOf("cat", 0));
+		eq(-1, "dogcat".lastIndexOf("cat", 1));
+		eq(3, "catcat".lastIndexOf("cat", 3));
+		eq(3, "catcat".lastIndexOf("cat", 4));
+	}
+	#end
+}