# BC break: ReflectionMethod::invoke() expects parameter 1 to be object, string given - Thread: https://externals.io/message/95370 - Original archive: https://news-web.php.net/php.internals/95370 - Started by Nicolas Grekas on 2016-08-22T11:17:57+00:00 - Messages: 11 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #95370 — Nicolas Grekas — 2016-08-22T11:17:57+00:00 URL: https://externals.io/message/95370#95370 Hello, now that the BC break on ReflectionType has been reverted, another one remains in ReflectionMethod::invoke(): the method doesn't accept a string as first argument anymore, see e.g.: https://3v4l.org/pImmv As you can see, this worked since 5.0 and even in HHVM. It would be great to fix this BC break please. Regards, Nicolas --- ## #95373 — Levi Morrison — 2016-08-22T14:00:36+00:00 URL: https://externals.io/message/95370#95373 · In reply to: #95370 On Mon, Aug 22, 2016 at 5:17 AM, Nicolas Grekas < nicolas.grekas+php@gmail.com> wrote: > Hello, > > now that the BC break on ReflectionType has been reverted, another one > remains in ReflectionMethod::invoke(): > > the method doesn't accept a string as first argument anymore, see e.g.: > > https://3v4l.org/pImmv > > As you can see, this worked since 5.0 and even in HHVM. > > It would be great to fix this BC break please. > > Regards, > Nicolas > According to the [documentation][1] it requires an object. If the documentation has not been altered recently to make it this way then I'm inclined to keep the backward compatibility break. Your example uses a static method - you should be passing null and not the name of the class (this is also in the documentation). [1]: http://php.net/manual/en/reflectionmethod.invoke.php --- ## #95374 — Pierre Joye — 2016-08-22T14:16:40+00:00 URL: https://externals.io/message/95370#95374 · In reply to: #95373 > > On Mon, Aug 22, 2016 at 5:17 AM, Nicolas Grekas < > nicolas.grekas+php@gmail.com> wrote: > > > […] > > According to the [documentation][1] it requires an object. If the > documentation has not been altered recently to make it this way then I'm > inclined to keep the backward compatibility break. Your example uses a > static method - you should be passing null and not the name of the class > (this is also in the documentation). > > [1]: http://php.net/manual/en/reflectionmethod.invoke.php I have to disagree here. Many codes out there uses string. What is the appealing reason to break these codes in 7.1? I think it should restore the precious behavior and if the docs need a fix, let fix it, not the other way. Cheers Pierre --- ## #95375 — Ferenc Kovacs — 2016-08-22T14:21:47+00:00 URL: https://externals.io/message/95370#95375 · In reply to: #95374 > > > […] > > I have to disagree here. > > Many codes out there uses string. What is the appealing reason to break > these codes in 7.1? > > I think it should restore the precious behavior and if the docs need a > fix, let fix it, not the other way. > > Cheers > Pierre > Hi, If it was explicitly documented to be expecting an object, then altering the code to match the documented behavior can be considered a bugfix, but I agree that if it is moderately/widely used we should consider keeping the old behavior instead of removing it in a minor version. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #95377 — Alexander Lisachenko — 2016-08-22T14:29:54+00:00 URL: https://externals.io/message/95370#95377 · In reply to: #95373 Hello, there is one more issue with ReflectionMethod::invoke() for static methods: it's impossible to call an overridden parent static method and preserve a scope information: // https://3v4l.org/ASntq class foo { static function bar() { echo self::class, ' vs ', static::class, PHP_EOL; } static function bar1() { echo self::class, ' vs ', static::class, PHP_EOL; } } class baz extends foo { static function bar() { echo self::class, ' vs ', static::class, PHP_EOL; } } $r = new ReflectionClass('baz'); $m = $r->getMethod('bar1'); // without overriden method - it's ok, our static scope is preserved $m->invoke(null); // foo vs baz $r = new ReflectionClass('baz'); $m = $r->getMethod('bar'); // with overriden method it's impossible to call parent static method, no way to specify a scope. $m->invoke(null); // baz vs baz // how to statically call the parent static method bar() with scope == baz?? 2016-08-22 17:00 GMT+03:00 Levi Morrison : > On Mon, Aug 22, 2016 at 5:17 AM, Nicolas Grekas < > nicolas.grekas+php@gmail.com> wrote: > > > […] > > According to the [documentation][1] it requires an object. If the > documentation has not been altered recently to make it this way then I'm > inclined to keep the backward compatibility break. Your example uses a > static method - you should be passing null and not the name of the class > (this is also in the documentation). > > [1]: http://php.net/manual/en/reflectionmethod.invoke.php --- ## #95376 — Alexander Lisachenko — 2016-08-22T14:32:02+00:00 URL: https://externals.io/message/95370#95376 · In reply to: #95377 If the first argument for invoke() and invokeArgs() for static methods will be a string with compatible class name for the scope, then it can be used as a scope modifier (like second argument for Closure::bind($instance, $scopeClassName)) 2016-08-22 17:29 GMT+03:00 Alexander Lisachenko : > Hello, there is one more issue with ReflectionMethod::invoke() for static > methods: it's impossible to call an overridden parent static method and > preserve a scope information: > > // https://3v4l.org/ASntq > > class foo > { > static function bar() > { > echo self::class, ' vs ', static::class, PHP_EOL; > } > > static function bar1() > { > echo self::class, ' vs ', static::class, PHP_EOL; > } > } > > class baz extends foo > { > static function bar() > { > echo self::class, ' vs ', static::class, PHP_EOL; > } > } > > $r = new ReflectionClass('baz'); > $m = $r->getMethod('bar1'); // without overriden method - it's ok, our > static scope is preserved > $m->invoke(null); // foo vs baz > > $r = new ReflectionClass('baz'); > $m = $r->getMethod('bar'); // with overriden method it's impossible to > call parent static method, no way to specify a scope. > $m->invoke(null); // baz vs baz > > // how to statically call the parent static method bar() with scope == > baz?? > > 2016-08-22 17:00 GMT+03:00 Levi Morrison : > > > […] --- ## #95378 — Rowan Collins — 2016-08-22T14:38:36+00:00 URL: https://externals.io/message/95370#95378 · In reply to: #95370 > in ReflectionMethod::invoke(): > > the method doesn't accept a string as first argument anymore, see e.g.: > > https://3v4l.org/pImmv To be precise, in prior versions, the first argument was completely ignored for static methods: https://3v4l.org/8o4Hm $m->invoke('some_other_class_name', 'baz'); // Warns in 7.1.0beta3, ignores everywhere else $m->invoke(42, 'baz'); // Warns in 7.1.0beta3, ignores everywhere else $m->invoke(new DateTime, 'baz'); // OK in all versions $m->invoke($m, 'baz'); // OK in all versions It is still ignored if it is a valid object, so the current beta's behaviour doesn't actually make a lot of sense. The most logical would be for it to fail on any value other than null, since for a non-static method any other object will give "Given object is not an instance of the class this method was declared in" (except in HHVM, which just invokes on whatever object you give it): https://3v4l.org/sCFcA However, simply documenting that "the first argument is ignored for static methods", and removing the parameter validation, would probably be sensible for BC if there is likely to be real-world code passing something other than null. Regards, -- Rowan Collins [IMSoP] --- ## #95381 — Rowan Collins — 2016-08-22T15:17:00+00:00 URL: https://externals.io/message/95370#95381 · In reply to: #95376 > If the first argument for invoke() and invokeArgs() for static methods will > be a string with compatible class name for the scope, then it can be used > as a scope modifier (like second argument for Closure::bind($instance, > $scopeClassName)) The problem with this is that you will only be able to use this with a version check - in prior versions, the argument will be silently accepted but ignored. That would make it extremely easy for any code relying on this to be subtly broken on older builds. Regards, -- Rowan Collins [IMSoP] --- ## #95379 — Christoph M. Becker — 2016-08-22T15:32:48+00:00 URL: https://externals.io/message/95370#95379 · In reply to: #95370 > now that the BC break on ReflectionType has been reverted, another one > remains in ReflectionMethod::invoke(): > > the method doesn't accept a string as first argument anymore, see e.g.: > > https://3v4l.org/pImmv > > As you can see, this worked since 5.0 and even in HHVM. > > It would be great to fix this BC break please. The BC break is caused by implementing feature requrest #38992 (), which has been posted nearly 10 years ago. My analysis as posted in the bug report: | Actually, the first parameter to ReflectionMethod::invoke() is | completely ignored, if the method is a static method, see | . A comment in the sources[1] also | explains the reasoning. | | This doesn't make any sense, however, because for static methods | to be invoked, `NULL` is supposed to be passed as first parameter to | ::invoke() as well as ::invokeArgs(). Allowing an arbitrary | argument is confusing at best. | | Changing the behavior would introduce a BC break, though, so it | appears to be reasonable to do that for PHP 7.1 only. | | [1] If so desired, I can revert that commit, but I wouldn't be happy with sticking with a completely unused parameter, which obviously has been and still is misunderstood. -- Christoph M. Becker --- ## #95380 — Christoph M. Becker — 2016-08-22T15:40:41+00:00 URL: https://externals.io/message/95370#95380 · In reply to: #95378 > > > […] > > To be precise, in prior versions, the first argument was completely > ignored for static methods: https://3v4l.org/8o4Hm > > $m->invoke('some_other_class_name', 'baz'); // Warns in 7.1.0beta3, > ignores everywhere else > $m->invoke(42, 'baz'); // Warns in 7.1.0beta3, ignores everywhere else > $m->invoke(new DateTime, 'baz'); // OK in all versions > $m->invoke($m, 'baz'); // OK in all versions > > It is still ignored if it is a valid object, so the current beta's > behaviour doesn't actually make a lot of sense. Well, see . Only as of 7.1.0beta3 ::invoke() matches the behavior of ::invokeArgs(). -- Christoph M. Becker --- ## #95382 — Rowan Collins — 2016-08-22T15:44:16+00:00 URL: https://externals.io/message/95370#95382 · In reply to: #95380 > > […] > Well, see . Only as of 7.1.0beta3 ::invoke() > matches the behavior of ::invokeArgs(). OK, so that's a further inconsistency. The current version still doesn't actually make sense, though. If you want to validate that the correct argument is being passed, then the only value allowed should be null. > If so desired, I can revert that commit, but I wouldn't be happy with > sticking with a completely unused parameter, which obviously has been > and still is misunderstood. The parameter is still unused, and can be any object. The only sane validation would be to check explicitly for null, which is documented as the correct argument for both invoke() and invokeArgs(). Regards, -- Rowan Collins [IMSoP]