The Reflect.callMethod situation (#7442)

* [tests] add test for Reflect.callMethod on statics/closures

see #7106

* flip closure test

* update spec

* accuracy

* [php] ignore the first argument in Reflect.callMethod()

std/Reflect.hx

@@ -89,7 +89,17 @@ extern class Reflect {
 	public static function setProperty( o : Dynamic, field : String, value : Dynamic ) : Void;
 
 	/**
-		Call a method with the given object and arguments.
+		Call a method `func` with the given arguments `args`.
+
+		The object `o` is ignored in most cases. It serves as the `this`-context in the following
+		situations:
+
+		* (neko) Allows switching the context to `o` in all cases.
+		* (macro) Same as neko for Haxe 3. No context switching in Haxe 4.
+		* (js, lua) Require the `o` argument if `func` does not, but should have a context.
+		    This can occur by accessing a function field natively, e.g. through `Reflect.field`
+			or by using `(object : Dynamic).field`. However, if `func` has a context, `o` is
+			ignored like on other targets.
 	**/
 	public static function callMethod( o : Dynamic, func : haxe.Constraints.Function, args : Array<Dynamic> ) : Dynamic;
 
@@ -135,7 +145,7 @@ extern class Reflect {
 
 	/**
 		Compares the functions `f1` and `f2`.
-		
+
 		If `f1` or `f2` are null, the result is false.
 		If `f1` or `f2` are not functions, the result is unspecified.
 

std/php/_std/Reflect.hx

@@ -100,15 +100,7 @@ using php.Global;
 	}
 
 	public static function callMethod( o : Dynamic, func : Function, args : Array<Dynamic> ) : Dynamic {
-		var callback:Any = func;
-		if(o != null && !Boot.isClass(o)) {
-			if (Std.is(func, Closure)) {
-				callback = (cast func:Closure).bindTo(o);
-			} else {
-				callback = Boot.castClosure(func).getCallback(o);
-			}
-		}
-		return Global.call_user_func_array(callback, @:privateAccess args.arr);
+		return Global.call_user_func_array(func, @:privateAccess args.arr);
 	}
 
 	public static function fields( o : Dynamic ) : Array<String> {

tests/unit/src/unit/issues/Issue7106.hx

@@ -0,0 +1,40 @@
+package unit.issues;
+
+private class C {
+	var i:Int;
+
+	public function new(i:Int) {
+		this.i = i;
+	}
+
+	public function getI() {
+		return i;
+	}
+}
+
+class Issue7106 extends unit.Test {
+	function testClosure() {
+		var c1 = new C(1);
+		var c2 = new C(2);
+		var c2Dyn:Dynamic = c2;
+		eq(2, Reflect.callMethod(c1, c2.getI, []));
+		eq(2, Reflect.callMethod(c2, Reflect.field(c2, "getI"), []));
+		eq(2, Reflect.callMethod(c2, c2Dyn.getI, []));
+
+		// On targets that don't have native closures, the first argument
+		// replaces the context. On other targets it is ignored.
+		var cCheck = #if (js || neko || lua) c2 #else c1 #end;
+		eq(2, Reflect.callMethod(cCheck, Reflect.field(c2, "getI"), []));
+		eq(2, Reflect.callMethod(cCheck, c2Dyn.getI, []));
+	}
+
+	function testStatic() {
+        eq(49, Reflect.callMethod(null,foo,[]));
+        eq(49, Reflect.callMethod(Issue7106,foo,[]));
+        eq(49, Reflect.callMethod(Type,foo,[]));
+	}
+
+	static function foo() {
+		return 49;
+	}
+}