diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 70f1073..426f1f5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,4 +30,4 @@ jobs: os: >- ['ubuntu-latest', 'windows-latest'] php: >- - ['8.1', '8.2', '8.3'] + ['8.1', '8.2', '8.3', '8.4'] diff --git a/.github/workflows/composer-require-checker.yml b/.github/workflows/composer-require-checker.yml index 822591f..919a073 100644 --- a/.github/workflows/composer-require-checker.yml +++ b/.github/workflows/composer-require-checker.yml @@ -32,4 +32,4 @@ jobs: os: >- ['ubuntu-latest'] php: >- - ['8.1', '8.2', '8.3'] + ['8.1', '8.2', '8.3', '8.4'] diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index dc464eb..7a54fcc 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -29,6 +29,6 @@ jobs: os: >- ['ubuntu-latest'] php: >- - ['8.3'] + ['8.4'] secrets: STRYKER_DASHBOARD_API_KEY: ${{ secrets.STRYKER_DASHBOARD_API_KEY }} diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 873eeb5..9e12684 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -30,4 +30,4 @@ jobs: os: >- ['ubuntu-latest'] php: >- - ['8.1', '8.2', '8.3'] + ['8.1', '8.2', '8.3', '8.4'] diff --git a/.github/workflows/yiisoft-di.yml b/.github/workflows/yiisoft-di.yml index 5f1bdfb..5285df3 100644 --- a/.github/workflows/yiisoft-di.yml +++ b/.github/workflows/yiisoft-di.yml @@ -37,6 +37,7 @@ jobs: - "8.1" - "8.2" - "8.3" + - "8.4" steps: - name: Checkout Yii Definitions diff --git a/.github/workflows/yiisoft-factory.yml b/.github/workflows/yiisoft-factory.yml index 5a0b40c..f75fff3 100644 --- a/.github/workflows/yiisoft-factory.yml +++ b/.github/workflows/yiisoft-factory.yml @@ -37,6 +37,7 @@ jobs: - "8.1" - "8.2" - "8.3" + - "8.4" steps: - name: Checkout Yii Definitions diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 43dd8b5..5c4e99a 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -6,10 +6,12 @@ use PhpCsFixer\Finder; use PhpCsFixer\Runner\Parallel\ParallelConfigFactory; -$finder = (new Finder())->in([ - __DIR__ . '/src', - __DIR__ . '/tests', -]); +$finder = (new Finder()) + ->in([ + __DIR__ . '/src', + __DIR__ . '/tests', + ]) + ->exclude('Php8_4/'); return (new Config()) ->setParallelConfig(ParallelConfigFactory::detect()) diff --git a/CHANGELOG.md b/CHANGELOG.md index f35b51b..a71038c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,12 @@ ## 3.3.2 under development +- Chg #105: Change PHP constraint in `composer.json` to `~8.1.0 || ~8.2.0 || ~8.3.0 || ~8.4.0` (@vjik) - Chg #106: Bump minimal required PHP version to 8.1 (@vjik) - Enh #106: Minor performance optimization: use FQN for PHP functions, remove unnecessary conditions (@vjik) - Enh #106: Mark readonly properties (@vjik) +- Bug #105: Explicitly mark nullable parameters (@vjik) +- Enh #105: Improve definition validation for readonly properties and properties with asymmetric visibility (@vjik) ## 3.3.1 December 16, 2024 diff --git a/composer.json b/composer.json index 33fc9cf..276b7ad 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,7 @@ } ], "require": { - "php": "^8.1", + "php": "~8.1.0 || ~8.2.0 || ~8.3.0 || ~8.4.0", "psr/container": "^1.0 || ^2.0" }, "require-dev": { @@ -37,7 +37,7 @@ "roave/infection-static-analysis-plugin": "^1.35", "spatie/phpunit-watcher": "^1.24", "vimeo/psalm": "^5.26.1 || ^6.7.1", - "yiisoft/test-support": "^3.0" + "yiisoft/test-support": "^3.0.1" }, "autoload": { "psr-4": { @@ -58,7 +58,7 @@ } }, "scripts": { - "php-cs-fixer": "php-cs-fixer fix", + "php-cs-fixer": "PHP_CS_FIXER_IGNORE_ENV=1 php-cs-fixer fix", "rector": "rector", "test": "phpunit --testdox --no-interaction", "test-watch": "phpunit-watcher watch" diff --git a/infection.json.dist b/infection.json.dist index 3776e22..2185614 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -11,6 +11,11 @@ } }, "mutators": { - "@default": true + "@default": true, + "LessThan": { + "ignoreSourceCodeByRegex": [ + ".*\\(PHP_VERSION_ID .*" + ] + } } } diff --git a/phpunit.xml.dist b/phpunit.xml.dist index c21dbf0..00bbbd3 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -21,6 +21,7 @@ ./tests/Unit ./tests/Php8_2 + ./tests/Php8_4 diff --git a/src/Exception/NotInstantiableClassException.php b/src/Exception/NotInstantiableClassException.php index 16250af..eb1b1dc 100644 --- a/src/Exception/NotInstantiableClassException.php +++ b/src/Exception/NotInstantiableClassException.php @@ -11,7 +11,7 @@ */ final class NotInstantiableClassException extends NotInstantiableException { - public function __construct(string $class, string $message = null, int $code = 0, Exception $previous = null) + public function __construct(string $class, ?string $message = null, int $code = 0, ?Exception $previous = null) { if ($message === null) { $message = "Can not instantiate $class."; diff --git a/src/Helpers/DefinitionValidator.php b/src/Helpers/DefinitionValidator.php index 05ca170..110732e 100644 --- a/src/Helpers/DefinitionValidator.php +++ b/src/Helpers/DefinitionValidator.php @@ -6,11 +6,13 @@ use ReflectionClass; use ReflectionException; +use ReflectionProperty; use Yiisoft\Definitions\ArrayDefinition; use Yiisoft\Definitions\Contract\DefinitionInterface; use Yiisoft\Definitions\Contract\ReferenceInterface; use Yiisoft\Definitions\Exception\InvalidConfigException; +use function count; use function in_array; use function is_array; use function is_callable; @@ -85,7 +87,7 @@ public static function validateArrayDefinition(array $definition, ?string $id = } $classPublicProperties = []; foreach ($classReflection->getProperties() as $reflectionProperty) { - if ($reflectionProperty->isPublic()) { + if (self::isPublicWritableProperty($reflectionProperty)) { $classPublicProperties[] = $reflectionProperty->getName(); } } @@ -261,7 +263,7 @@ private static function validateProperty( } elseif (!in_array($parsedKey, $classPublicProperties, true)) { throw new InvalidConfigException( sprintf( - 'Invalid definition: property "%s" must be public.', + 'Invalid definition: property "%s" must be public and writable.', $className . '::' . $key, ), ); @@ -327,4 +329,26 @@ private static function validateString(mixed $class): void throw new InvalidConfigException('Invalid definition: class name must be a non-empty string.'); } } + + private static function isPublicWritableProperty(ReflectionProperty $property): bool + { + if (!$property->isPublic()) { + return false; + } + + if ($property->isReadOnly()) { + return false; + } + + if (PHP_VERSION_ID < 80400) { + return true; + } + + $modifiers = $property->getModifiers(); + + /** + * @psalm-suppress UndefinedConstant, MixedOperand Needs for PHP 8.3 or lower + */ + return ($modifiers & (ReflectionProperty::IS_PRIVATE_SET | ReflectionProperty::IS_PROTECTED_SET)) === 0; + } } diff --git a/src/ParameterDefinition.php b/src/ParameterDefinition.php index 0fbe51c..34fff5a 100644 --- a/src/ParameterDefinition.php +++ b/src/ParameterDefinition.php @@ -216,10 +216,7 @@ private function getCallable(): string if ($class !== null) { $callable[] = $class->getName(); } - $callable[] = $this->parameter - ->getDeclaringFunction() - ->getName() . - '()'; + $callable[] = $this->parameter->getDeclaringFunction()->getName() . '()'; return implode('::', $callable); } diff --git a/tests/Php8_2/ParameterDefinitionTest.php b/tests/Php8_2/ParameterDefinitionTest.php index 75caf79..9d57783 100644 --- a/tests/Php8_2/ParameterDefinitionTest.php +++ b/tests/Php8_2/ParameterDefinitionTest.php @@ -22,7 +22,7 @@ public function testResolveUnionTypeWithIntersectionType(): void ]); $definition = new ParameterDefinition( - $this->getFirstParameter(fn (Bike|(GearBox&stdClass)|Chair $class) => true) + $this->getFirstParameter(fn(Bike|(GearBox&stdClass)|Chair $class) => true), ); $result = $definition->resolve($container); diff --git a/tests/Php8_4/AsymmetricVisibility/DefinitionValidatorTest.php b/tests/Php8_4/AsymmetricVisibility/DefinitionValidatorTest.php new file mode 100644 index 0000000..dfed394 --- /dev/null +++ b/tests/Php8_4/AsymmetricVisibility/DefinitionValidatorTest.php @@ -0,0 +1,40 @@ + PublicGet::class, + '$privateVar' => 'test', + ]; + + $this->expectException(InvalidConfigException::class); + $this->expectExceptionMessage( + 'Invalid definition: property "Yiisoft\Definitions\Tests\Php8_4\AsymmetricVisibility\PublicGet::$privateVar" must be public and writable.', + ); + DefinitionValidator::validate($definition); + } + + public function testProtectedSet(): void + { + $definition = [ + 'class' => PublicGet::class, + '$protectedVar' => 'test', + ]; + + $this->expectException(InvalidConfigException::class); + $this->expectExceptionMessage( + 'Invalid definition: property "Yiisoft\Definitions\Tests\Php8_4\AsymmetricVisibility\PublicGet::$protectedVar" must be public and writable.', + ); + DefinitionValidator::validate($definition); + } +} diff --git a/tests/Php8_4/AsymmetricVisibility/PublicGet.php b/tests/Php8_4/AsymmetricVisibility/PublicGet.php new file mode 100644 index 0000000..ffbfaef --- /dev/null +++ b/tests/Php8_4/AsymmetricVisibility/PublicGet.php @@ -0,0 +1,11 @@ +assertEquals(5, $dependencies['maxGear']->resolve($container)); } - public function testOptionalInterfaceDependency(): void - { - $container = new SimpleContainer(); - /** @var DefinitionInterface[] $dependencies */ - $dependencies = DefinitionExtractor::fromClassName(OptionalInterfaceDependency::class); - $this->assertCount(1, $dependencies); - $this->assertEquals(null, $dependencies['engine']->resolve($container)); - } - public function testNullableInterfaceDependency(): void { $container = new SimpleContainer(); @@ -121,15 +110,6 @@ public function testNullableInterfaceDependency(): void $dependencies['engine']->resolve($container); } - public function testOptionalConcreteDependency(): void - { - $container = new SimpleContainer(); - /** @var DefinitionInterface[] $dependencies */ - $dependencies = DefinitionExtractor::fromClassName(OptionalConcreteDependency::class); - $this->assertCount(1, $dependencies); - $this->assertEquals(null, $dependencies['car']->resolve($container)); - } - public function testNullableConcreteDependency(): void { $container = new SimpleContainer(); @@ -152,7 +132,6 @@ public function testNullableOptionalConcreteDependency(): void public function testNullableOptionalInterfaceDependency(): void { $container = new SimpleContainer(); - /** @var DefinitionInterface[] $dependencies */ $dependencies = DefinitionExtractor::fromClassName(NullableOptionalInterfaceDependency::class); $this->assertCount(1, $dependencies); $this->assertEquals(null, $dependencies['engine']->resolve($container)); diff --git a/tests/Unit/Helpers/DefinitionValidatorTest.php b/tests/Unit/Helpers/DefinitionValidatorTest.php index 43c3e4d..108e709 100644 --- a/tests/Unit/Helpers/DefinitionValidatorTest.php +++ b/tests/Unit/Helpers/DefinitionValidatorTest.php @@ -17,6 +17,7 @@ use Yiisoft\Definitions\Tests\Support\GearBox; use Yiisoft\Definitions\Tests\Support\MagicCall; use Yiisoft\Definitions\Tests\Support\Phone; +use Yiisoft\Definitions\Tests\Support\ReadonlyProperty; use Yiisoft\Definitions\Tests\Support\Recorder; use Yiisoft\Definitions\Tests\Support\UTF8User; use Yiisoft\Definitions\ValueDefinition; @@ -73,7 +74,7 @@ public static function dataInvalidProperty(): array $object1::class, '$invisible', sprintf( - 'Invalid definition: property "%s" must be public.', + 'Invalid definition: property "%s" must be public and writable.', $object1::class . '::$invisible', ), ], @@ -81,7 +82,7 @@ public static function dataInvalidProperty(): array UTF8User::class, '$имя', sprintf( - 'Invalid definition: property "%s" must be public.', + 'Invalid definition: property "%s" must be public and writable.', UTF8User::class . '::$имя', ), ], @@ -367,4 +368,18 @@ public function testIncorrectMethodName(array $config, string $message): void $this->expectExceptionMessage($message); DefinitionValidator::validate($config); } + + public function testReadonlyProperty(): void + { + $definition = [ + 'class' => ReadonlyProperty::class, + '$var' => 'test', + ]; + + $this->expectException(InvalidConfigException::class); + $this->expectExceptionMessage( + 'Invalid definition: property "Yiisoft\Definitions\Tests\Support\ReadonlyProperty::$var" must be public and writable.', + ); + DefinitionValidator::validate($definition); + } } diff --git a/tests/Unit/ParameterDefinitionTest.php b/tests/Unit/ParameterDefinitionTest.php index 239c15f..8357994 100644 --- a/tests/Unit/ParameterDefinitionTest.php +++ b/tests/Unit/ParameterDefinitionTest.php @@ -13,6 +13,7 @@ use ReflectionParameter; use RuntimeException; use stdClass; +use Throwable; use Yiisoft\Definitions\Exception\InvalidConfigException; use Yiisoft\Definitions\Exception\NotInstantiableException; use Yiisoft\Definitions\ParameterDefinition; @@ -84,7 +85,7 @@ public static function dataHasValue(): array static fn( string $a, ?string $b, - string $c = null, + ?string $c = null, string $d = 'hello', ): bool => true, ); @@ -138,10 +139,6 @@ public static function dataResolve(): array 7, self::getFirstParameter(static fn(int $n = 7) => true), ], - 'defaultNull' => [ - null, - self::getFirstParameter(static fn(int $n = null) => true), - ], 'nullableAndDefaultNull' => [ null, self::getFirstParameter(static fn(?int $n = null) => true), @@ -167,13 +164,21 @@ public function testResolveNonTypedParameter(): void ); $container = new SimpleContainer(); - $this->expectException(NotInstantiableException::class); - $this->expectExceptionMessage( - 'Can not determine value of the "x" parameter without type when instantiating ' - . '"Yiisoft\Definitions\Tests\Unit\ParameterDefinitionTest::Yiisoft\Definitions\Tests\Unit\{closure}()"' - . '. Please specify argument explicitly.', + $exception = null; + try { + $definition->resolve($container); + } catch (Throwable $exception) { + } + + $this->assertInstanceOf(NotInstantiableException::class, $exception); + $this->assertStringStartsWith( + 'Can not determine value of the "x" parameter without type when instantiating "Yiisoft\Definitions\Tests\Unit\ParameterDefinitionTest::', + $exception->getMessage(), + ); + $this->assertStringEndsWith( + 'Please specify argument explicitly.', + $exception->getMessage(), ); - $definition->resolve($container); } public function testResolveBuiltinParameter(): void @@ -185,13 +190,21 @@ public function testResolveBuiltinParameter(): void ); $container = new SimpleContainer(); - $this->expectException(NotInstantiableException::class); - $this->expectExceptionMessage( - 'Can not determine value of the "n" parameter of type "int" when instantiating ' - . '"Yiisoft\Definitions\Tests\Unit\ParameterDefinitionTest::Yiisoft\Definitions\Tests\Unit\{closure}()".' - . ' Please specify argument explicitly.', + $exception = null; + try { + $definition->resolve($container); + } catch (Throwable $exception) { + } + + $this->assertInstanceOf(NotInstantiableException::class, $exception); + $this->assertStringStartsWith( + 'Can not determine value of the "n" parameter of type "int" when instantiating "Yiisoft\Definitions\Tests\Unit\ParameterDefinitionTest::', + $exception->getMessage(), + ); + $this->assertStringEndsWith( + 'Please specify argument explicitly.', + $exception->getMessage(), ); - $definition->resolve($container); } public function testResolveSelf(): void @@ -420,9 +433,18 @@ public function testNotResolveIntersectionType(): void self::getFirstParameter(fn(GearBox&stdClass $class) => true), ); - $this->expectException(NotInstantiableException::class); - $this->expectExceptionMessage('Can not determine value of the "class" parameter of type '); - $definition->resolve($container); + $exception = null; + try { + $definition->resolve($container); + } catch (Throwable $exception) { + } + + $this->assertInstanceOf(NotInstantiableException::class, $exception); + $this->assertStringStartsWith( + 'Can not determine value of the "class" parameter of type "Yiisoft\Definitions\Tests\Support\GearBox&stdClass" when instantiating "Yiisoft\Definitions\Tests\Unit\ParameterDefinitionTest::', + $exception->getMessage(), + ); + $this->assertStringEndsWith('}()". Please specify argument explicitly.', $exception->getMessage()); } /**