Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for PHP 8, fix tests #57

Closed
wants to merge 10 commits into from
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ matrix:
- php: nightly
env:
- DEPS=lowest
- COMPOSER_ARGS="--no-interaction --ignore-platform-reqs"
- php: nightly
env:
- DEPS=latest
allow_failures:
- php: nightly
- COMPOSER_ARGS="--no-interaction --ignore-platform-reqs"

before_install:
- if [[ $TEST_COVERAGE != 'true' ]]; then phpenv config-rm xdebug.ini || return 0 ; fi
Expand Down
5 changes: 3 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"sort-packages": true
},
"require": {
"php": "^7.4",
"php": "^7.4 || ~8.0.0",
"laminas/laminas-stdlib": "^3.2.1",
"laminas/laminas-zendframework-bridge": "^1.0",
"psr/container": "^1.0"
Expand All @@ -35,7 +35,8 @@
"mikey179/vfsstream": "^1.6.8",
"ocramius/proxy-manager": "^2.8",
"phpbench/phpbench": "^0.17",
"phpunit/phpunit": "^7.5.20"
"phpspec/prophecy-phpunit": "^2.0",
"phpunit/phpunit": "^9.3"
},
"provide": {
"psr/container-implementation": "^1.0"
Expand Down
35 changes: 31 additions & 4 deletions src/AbstractFactory/ReflectionBasedAbstractFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ private function resolveParameterWithConfigService(ContainerInterface $container
* resolved to a service in the container.
*/
return function (ReflectionParameter $parameter) use ($container, $requestedName) {
if ($parameter->isArray() && $parameter->getName() === 'config') {
if ($parameter->getType() && $parameter->getType()->getName() === 'array'
&& $parameter->getName() === 'config') {
return $container->get('config');
}
return $this->resolveParameter($parameter, $container, $requestedName);
Expand All @@ -212,11 +213,11 @@ private function resolveParameterWithConfigService(ContainerInterface $container
*/
private function resolveParameter(ReflectionParameter $parameter, ContainerInterface $container, $requestedName)
{
if ($parameter->isArray()) {
if ($this->getParameterClassName($parameter) === 'array') {
return [];
}

if (! $parameter->getClass()) {
if (! $parameter->hasType() || ($parameter->hasType() && $parameter->getType()->isBuiltin())) {
if (! $parameter->isDefaultValueAvailable()) {
throw new ServiceNotFoundException(sprintf(
'Unable to create service "%s"; unable to resolve parameter "%s" '
Expand All @@ -229,7 +230,7 @@ private function resolveParameter(ReflectionParameter $parameter, ContainerInter
return $parameter->getDefaultValue();
}

$type = $parameter->getClass()->getName();
$type = $parameter->getType()->getName();
$type = $this->aliases[$type] ?? $type;

if ($container->has($type)) {
Expand All @@ -249,4 +250,30 @@ private function resolveParameter(ReflectionParameter $parameter, ContainerInter
// default defined.
return $parameter->getDefaultValue();
}

/**
* Gets the parameter's class name.
*
* @param ReflectionParameter $parameter
* The parameter.
*
* @return string|null
* The parameter's class name or NULL if the parameter is not a class.
*/
public static function getParameterClassName(ReflectionParameter $parameter)
ocean marked this conversation as resolved.
Show resolved Hide resolved
{
$name = null;
if ($parameter->hasType() && ! $parameter->getType()->isBuiltin()) {
$name = $parameter->getType()->getName();
$lc_name = strtolower($name);
switch ($lc_name) {
case 'self':
return $parameter->getDeclaringClass()->getName();

case 'parent':
return ($parent = $parameter->getDeclaringClass()->getParentClass()) ? $parent->name : null;
}
}
return $name;
}
}
2 changes: 1 addition & 1 deletion src/Tool/ConfigDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function (ReflectionParameter $argument) {
$classConfig = [];

foreach ($constructorArguments as $constructorArgument) {
$argumentType = $constructorArgument->getClass();
$argumentType = $constructorArgument->getType();
if (is_null($argumentType)) {
if ($ignoreUnresolved) {
// don't throw an exception, just return the previous config
Expand Down
30 changes: 28 additions & 2 deletions src/Tool/FactoryCreator.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,32 @@ public function createFactory($className)
);
}

/**
* Gets the parameter's class name.
*
* @param ReflectionParameter $parameter
* The parameter.
*
* @return string|null
* The parameter's class name or NULL if the parameter is not a class.
*/
public static function getParameterClassName(ReflectionParameter $parameter)
ocean marked this conversation as resolved.
Show resolved Hide resolved
{
$name = null;
if ($parameter->hasType() && ! $parameter->getType()->isBuiltin()) {
$name = $parameter->getType()->getName();
$lc_name = strtolower($name);
switch ($lc_name) {
case 'self':
return $parameter->getDeclaringClass()->getName();

case 'parent':
return ($parent = $parameter->getDeclaringClass()->getParentClass()) ? $parent->name : null;
}
}
return $name;
}

/**
* @param $className
* @return string
Expand Down Expand Up @@ -115,7 +141,7 @@ function (ReflectionParameter $argument) {
return false;
}

if (null === $argument->getClass()) {
if (null === $this->getParameterClassName($argument)) {
throw new InvalidArgumentException(sprintf(
'Cannot identify type for constructor argument "%s"; '
. 'no type hint, or non-class/interface type hint',
Expand All @@ -132,7 +158,7 @@ function (ReflectionParameter $argument) {
}

return array_map(function (ReflectionParameter $parameter) {
return $parameter->getClass()->getName();
return $parameter->getType()->getName();
}, $constructorParameters);
}

Expand Down
7 changes: 5 additions & 2 deletions test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@
use Laminas\ServiceManager\AbstractFactory\ReflectionBasedAbstractFactory;
use Laminas\ServiceManager\Exception\ServiceNotFoundException;
use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
ocean marked this conversation as resolved.
Show resolved Hide resolved
use Psr\Container\ContainerInterface;

use function sprintf;

class ReflectionBasedAbstractFactoryTest extends TestCase
{
use ProphecyTrait;

/** @var ContainerInterface|ObjectProphecy */
private $container;

public function setUp()
public function setUp(): void
{
$this->container = $this->prophesize(ContainerInterface::class);
}
Expand Down Expand Up @@ -171,7 +174,7 @@ public function testFactoryCanSupplyAMixOfParameterTypes()
self::assertInstanceOf(TestAsset\ClassWithMixedConstructorParameters::class, $instance);

self::assertEquals($config, $instance->config);
self::assertEquals([], $instance->options);
self::assertEquals(null, $instance->options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change mean that there was an error before?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there was an error in the tests when I ran them locally. I thought I saw a comment something about $options always being treated as null, but now I can't find it. I'll revert this change and then you can see the error that shows up in Travis.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@froschdesign Yep, in the test the options instance property is set to null, not to an empty array, see:
https://github.com/laminas/laminas-servicemanager/blob/3.5.x/test/AbstractFactory/TestAsset/ClassWithMixedConstructorParameters.php#L24

So either the assertion or the test class constructor needs changing I guess.

self::assertSame($sample, $instance->sample);
self::assertSame($validators, $instance->validators);
}
Expand Down
12 changes: 7 additions & 5 deletions test/AbstractPluginManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use LaminasTest\ServiceManager\TestAsset\SimplePluginManager;
use LaminasTest\ServiceManager\TestAsset\V2v3PluginManager;
use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;
use Psr\Container\ContainerInterface;
use stdClass;

Expand All @@ -35,6 +36,7 @@
class AbstractPluginManagerTest extends TestCase
{
use CommonServiceLocatorBehaviorsTrait;
use ProphecyTrait;

public function createContainer(array $config = [])
{
Expand Down Expand Up @@ -204,7 +206,7 @@ public function testCallingSetServiceLocatorSetsCreationContextWithDeprecationNo
$pluginManager = new TestAsset\LenientPluginManager();
restore_error_handler();

self::assertAttributeSame($pluginManager, 'creationContext', $pluginManager);
self::assertSame($pluginManager, $pluginManager->getCreationContext());
$serviceManager = new ServiceManager();

set_error_handler(function ($errno, $errstr) {
Expand All @@ -213,7 +215,7 @@ public function testCallingSetServiceLocatorSetsCreationContextWithDeprecationNo
$pluginManager->setServiceLocator($serviceManager);
restore_error_handler();

self::assertAttributeSame($serviceManager, 'creationContext', $pluginManager);
self::assertSame($serviceManager, $pluginManager->getCreationContext());
}

/**
Expand All @@ -226,7 +228,7 @@ public function testPassingNoInitialConstructorArgumentSetsPluginManagerAsCreati
}, E_USER_DEPRECATED);
$pluginManager = new TestAsset\LenientPluginManager();
restore_error_handler();
self::assertAttributeSame($pluginManager, 'creationContext', $pluginManager);
self::assertSame($pluginManager, $pluginManager->getCreationContext());
}

/**
Expand All @@ -243,7 +245,7 @@ public function testCanPassConfigInterfaceAsFirstConstructorArgumentWithDeprecat
$pluginManager = new TestAsset\LenientPluginManager($config->reveal());
restore_error_handler();

self::assertAttributeSame($pluginManager, 'creationContext', $pluginManager);
self::assertSame($pluginManager, $pluginManager->getCreationContext());
}

public function invalidConstructorArguments()
Expand Down Expand Up @@ -338,7 +340,7 @@ public function testValidateWillFallBackToValidatePluginWhenDefinedAndEmitDeprec
$errorHandlerCalled = false;
set_error_handler(function ($errno, $errmsg) use (&$errorHandlerCalled) {
self::assertEquals(E_USER_DEPRECATED, $errno);
self::assertContains('3.0', $errmsg);
self::assertStringContainsString('3.0', $errmsg);
$errorHandlerCalled = true;
}, E_USER_DEPRECATED);
$pluginManager->validate($instance);
Expand Down
6 changes: 3 additions & 3 deletions test/CommonServiceLocatorBehaviorsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ function ($container, $instance) {
);

$config = $serviceManager->get('config');
self::assertInternalType('array', $config, 'Config service did not resolve as expected');
self::assertIsArray($config, 'Config service did not resolve as expected');
self::assertSame(
$config,
$serviceManager->get('config'),
Expand Down Expand Up @@ -722,7 +722,7 @@ public function testCanInjectDelegators()

$foo = $container->get('foo');
self::assertInstanceOf(stdClass::class, $foo);
self::assertAttributeEquals('foo', 'name', $foo);
self::assertEquals('foo', $foo->name);
}

/**
Expand All @@ -748,7 +748,7 @@ public function testCanInjectInitializers()

$foo = $container->get('foo');
self::assertInstanceOf(stdClass::class, $foo);
self::assertAttributeEquals(stdClass::class, 'name', $foo);
self::assertEquals(stdClass::class, $foo->name);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions test/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
use Laminas\ServiceManager\Config;
use Laminas\ServiceManager\ServiceManager;
use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;

/**
* @covers Laminas\ServiceManager\Config
*/
class ConfigTest extends TestCase
{
use ProphecyTrait;

public function testMergeArrays()
{
$config = [
Expand Down
14 changes: 6 additions & 8 deletions test/LazyServiceIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ class LazyServiceIntegrationTest extends TestCase
{
public $proxyDir;

public function setUp()
public function setUp(): void
{
$this->proxyDir = sys_get_temp_dir() . '/laminas-servicemanager-proxy';
if (! is_dir($this->proxyDir)) {
mkdir($this->proxyDir);
}
}

public function tearDown()
public function tearDown(): void
{
if (! is_dir($this->proxyDir)) {
return;
Expand Down Expand Up @@ -144,16 +144,15 @@ public function testCanUseLazyServiceFactoryFactoryToCreateLazyServiceFactoryToA
$instance,
'Service returned does not extend ' . InvokableObject::class
);
self::assertContains(
self::assertStringContainsString(
'TestAssetProxy',
get_class($instance),
'Service returned does not contain expected namespace'
);

// Test proxying works as expected
$options = $instance->getOptions();
self::assertInternalType(
'array',
self::assertIsArray(
$options,
'Expected an array of options'
);
Expand Down Expand Up @@ -219,16 +218,15 @@ public function testWillNotGenerateProxyClassFilesByDefault()
$instance,
'Service returned does not extend ' . InvokableObject::class
);
self::assertContains(
self::assertStringContainsString(
'TestAssetProxy',
get_class($instance),
'Service returned does not contain expected namespace'
);

// Test proxying works as expected
$options = $instance->getOptions();
self::assertInternalType(
'array',
self::assertIsArray(
$options,
'Expected an array of options'
);
Expand Down
2 changes: 1 addition & 1 deletion test/Proxy/LazyServiceFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class LazyServiceFactoryTest extends TestCase
/**
* {@inheritDoc}
*/
protected function setUp()
protected function setUp(): void
{
$this->proxyFactory = $this->getMockBuilder(LazyLoadingValueHolderFactory::class)
->getMock();
Expand Down
Loading