Incompatible get method in PHP 7.2

snapshotpl commented 6 years ago

I use zendframework/zend-servicemanager 2.7.8 and zendframework/zend-mvc 2.7.10. After upgrade to php 7.2 I receive an fatal error:

PHP Fatal error:  Declaration of Zend\Mvc\Controller\PluginManager::get($name, ?array $options = NULL) must be compatible with Zend\ServiceManager\AbstractPluginManager::get($name, $options = Array, $usePeeringServiceManagers = true) in /usr/src/app/vendor/zendframework/zend-mvc/src/Controller/PluginManager.php on line 211

In mvc (PluginManager) method definition looks like:

public function get($name, array $options = null)

In servicemanager (AbstractPluginManager):

public function get($name, $options = [], $usePeeringServiceManagers = true)

ciaranmcnulty commented 6 years ago

Odd, it should just be a warning https://3v4l.org/4sdoZ

michalbundyra commented 6 years ago

@ciaranmcnulty It's a bit more complicated unfortunately: https://3v4l.org/Z7q5P

interface ServiceLocatorInterface
{
    public function get($name);
}

class ServiceManager implements ServiceLocatorInterface {
    public function get($name, $usePeeringServiceManagers = true) {}
}

class AbstractPluginManager extends ServiceManager
{
    public function get($name, $options = [], $usePeeringServiceManagers = true){}
}

class PluginManager extends AbstractPluginManager
{
    public function get($name, array $options = null){}
}

And please notice that zend-mvc support zend-servicemanager ^2.7.5 || ^3.0.3. Probably there is a solution, but it will be quite complicated, and I'm not sure if version 2 of ZF components are going to get full PHP 7.2 support.

snapshotpl commented 6 years ago

And please notice that zend-mvc support zend-servicemanager ^2.7.5 || ^3.0.3.

Yes, but only in v2. v3 support zend-servicemanager ^3.3.

michalbundyra commented 6 years ago

Basically to solve the issue we have to do the same what I did here: https://github.com/zendframework/zend-cache/pull/141/commits/d19e3ac77f2e6f0124b4d72bc51afb903cdaffef

I can't do it now, I can work on it tomorrow.

michalbundyra commented 6 years ago

@snapshotpl

Yes, but only in v2. v3 support zend-servicemanager ^3.3.

And as I understand you reporting issue in mvc v2, correct? I think we don't have the issue in mvc v3.

snapshotpl commented 6 years ago

Yes. In v3 looks fine

michalbundyra commented 6 years ago

Ok, so I'm not going to work on it, because I'm not sure if this fix is going to be applied in v2 at all. I need to get confirmation that we are actually going to support PHP 7.2 in v2 components. I had a look, and I see more issues:

PHPUnit 4.8.36 (latest version from 4.x channel) is not compatible with PHP 7.2 so we got deprecation on running:

Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /zend-mvc/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
On running tests I got another failure:

PHP Fatal error: Declaration of Zend\Session\AbstractContainer::offsetGet($key) must be compatible with & Zend\Stdlib\ArrayObject::offsetGet($key) in /zend-mvc/vendor/zendframework/zend-session/src/AbstractContainer.php on line 614

So I think there will be more issues like that also in other v2 components....

weierophinney commented 6 years ago

I was having trouble following, because the error message that @snapshotpl provided showed a nullable type (?array), and, of course, neither the zend-mvc nor the zend-servicemanager versions used have nullable types present.

But what it comes down to is that AbstractPluginManager::get() defines the $options property both without a typehint as well as with a default value of an empty array, while Zend\Mvc\Controller\PluginManager defines it using an array typehint and a null default value.

In this particular case, I think we should update the zend-mvc 2.X version, as it's a quick and easy fix, and 2.7 is essentially an LTS release for aiding in v2 -> v3 migrations.

In most cases, however, if a v3 release already exists and is supported by the component, we should likely indicate "won't fix".

weierophinney commented 6 years ago

Interestingly, I cannot verify the tests, because zend-mvc 2.7.* pins to the PHPUnit 4.8 releases, which do not run under PHP 7.2, as @webimpress already pointed out. (I didn't get much sleep last night, so missed that comment earlier when reviewing.)

As such, I think we need to mark "won't fix" here, particularly as the signature already works with the zend-servicemanager v3 AbstractPluginManager.

FabianRahm commented 6 years ago

Hi Matthew,

"In this particular case, I think we should update the zend-mvc 2.X version, as it's a quick and easy fix, and 2.7 is essentially an LTS release for aiding in v2 -> v3 migrations."

What is the scope of the LTS then? I am running against the same issue on early 7.2 tests. I would love to see a fix since we're still not on v3, at all.

Regards

weierophinney commented 6 years ago

What is the scope of the LTS then?

Critical bugfixes and security fixes only.

We never tested zend-mvc v2 versions against PHP 7.2 because it did not exist at the time of the last stable minor release.

Adding support for PHP 7.2 constitutes a new feature, and that falls outside the scope of the LTS. While we indicate a ^7.0 constraint, that was done on the assumption that PHP would not be releasing backwards incompatible changes in the v7 life-cycle. For us to test against 7.2 at this time, we would need to:

Update the unit test suite from PHPUnit 4.8 to 5.7 || 6.0. This is non-trivial, as it involves:
- Changing references to PHPUnit classes to use the new namespaced versions.
- Changing how we test for expected exceptions, as that functionality has changed in the v5 series.
- Changing code where we use PHPUnit's built-in mocking library, as mock object creation changed.
Once the test suite is updated, fix any issues that now appear.

This is work we have already done in the v3 series, and it took quite some time. Doing this work again for code that will not be receiving new features diverts efforts away from maintaining stable and evolving code. As such, we simply cannot prioritize it.

BrunoSpy commented 6 years ago

IMHO that's definitively a bug and need to be fixed.

your composer.json states ^7.0
there are still some modules that are not v3 compatible
https://framework.zend.com/long-term-support indicates an end of support in march 2018

FabianRahm commented 6 years ago

@BrunoSpy I think @weierophinney released 2.7.13 which fixes this issue.

https://github.com/zendframework/zend-mvc/releases/tag/release-2.7.13

Many thanks!

ghost commented 6 years ago

só botar & na frente da função offsetGet linha 425 do AbstractContainer.php

weierophinney commented 6 years ago

@marcosrdesouza English only in the issue tracker, please.

Are you seeing more problems? If so, please provide full details: exception/error messages, what you did that resulted in the problem, etc.

Additionally, since this issue has been marked fixed, and the original reporter has indicated it was resolved to their satisfaction, when you do provide the above information, do so in a new issue.

Thanks!

BigStarMyLife commented 4 years ago

Thank you very much. I knew it at much..

BigStarMyLife commented 4 years ago

Is this error is version error? so Can i use what version in zend 2? What is the solution?

michalbundyra commented 4 years ago

@BigStarMyLife Yes, it has been fixed in 2.7.13.

zendframework / zend-mvc

Incompatible get method in PHP 7.2 #266