Skip to content

Fix tests for PHP 8.4 #898

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

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Fix tests for PHP 8.4 #898

merged 4 commits into from
Jan 13, 2025

Conversation

zonuexe
Copy link
Contributor

@zonuexe zonuexe commented Jan 7, 2025

Make to ensure compatibility with PHP 8.4:

  • Explicitly add ? nullable type to optional parameters.
  • Moves optional parameters (with default values) after required parameters.
  • For regression testing, we will separate test cases for PHP 8.3 and earlier from newer PHP versions.

It is similar to #897, but this PR is focused only on PHP 8.4 support.

\stdClass $lazyService,
#[Inject('attribute')]
\stdClass $attribute,
?\stdClass $typedOptionalValue = new stdClass(),
?\stdClass $typedOptionalValueDefaultNull = null,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are examples where adding ? to a type changes behavior.

Checking for both = new stdClass() and = null highlights the difference in behavior.

PHP 8.4 prohibits non-optional (mandatory) parameters from appearing
after optional parameters. This commit updates various constructors
and methods so that optional parameters with default values (including
null or new stdClass instances) are placed at the end, ensuring
compliance with the new PHP 8.4 requirements.
@zonuexe
Copy link
Contributor Author

zonuexe commented Jan 7, 2025

PHP Fatal error:  Constant expression contains invalid operations in /home/runner/work/PHP-DI/PHP-DI/tests/IntegrationTest/Definitions/AttributeTest.php on line 152

ok.

@zonuexe zonuexe marked this pull request as draft January 7, 2025 15:48
@zonuexe
Copy link
Contributor Author

zonuexe commented Jan 7, 2025

In my environment, the tests passed with PHP 8.0, 8.3 and 8.4.

The files separated due to syntax incompatibilities have the following differences:

diff
% diff -u tests/IntegrationTest/Definitions/Attribute/class-php83.php tests/IntegrationTest/Definitions/Attribute/class.php
--- tests/IntegrationTest/Definitions/Attribute/class-php83.php 2025-01-08 01:02:44
+++ tests/IntegrationTest/Definitions/Attribute/class.php       2025-01-08 01:02:38
@@ -40,7 +40,7 @@
         \stdClass $lazyService,
         #[Inject('attribute')]
         \stdClass $attribute,
-        \stdClass $typedOptionalValue = null,
+        ?\stdClass $typedOptionalValue = new stdClass(),
         ?\stdClass $typedOptionalValueDefaultNull = null,
         string $optionalValue = 'hello'
     ) {
@@ -87,7 +87,7 @@
         \stdClass $lazyService,
         #[Inject('attribute')]
         stdClass $attribute,
-        \stdClass $typedOptionalValue = null,
+        ?\stdClass $typedOptionalValue = new stdClass,
         ?\stdClass $typedOptionalValueDefaultNull = null,
         $optionalValue = 'hello'
     ) {

% diff -u tests/IntegrationTest/Definitions/CreateDefinition/class-php83.php tests/IntegrationTest/Definitions/CreateDefinition/class.php
--- tests/IntegrationTest/Definitions/CreateDefinition/class-php83.php  2025-01-08 01:06:26
+++ tests/IntegrationTest/Definitions/CreateDefinition/class.php        2025-01-08 01:05:53
@@ -25,7 +25,7 @@
         string $scalarValue,
         \stdClass $typedValue,
         \stdClass $lazyService,
-        \stdClass $typedOptionalValue = null,
+        ?\stdClass $typedOptionalValue = new \stdClass(),
         ?\stdClass $typedOptionalValueDefaultNull = null,
         $optionalValue = 'hello'
     ) {
@@ -63,7 +63,7 @@
         string $scalarValue,
         \stdClass $typedValue,
         \stdClass $lazyService,
-        \stdClass $typedOptionalValue = null,
+        ?\stdClass $typedOptionalValue = new stdClass(),
         ?\stdClass $typedOptionalValueDefaultNull = null,
         $optionalValue = 'hello'
     ) {

@zonuexe zonuexe marked this pull request as ready for review January 7, 2025 16:25
@zonuexe zonuexe changed the title PHP 8.4 support Fix tests for PHP 8.4 Jan 7, 2025
@zonuexe
Copy link
Contributor Author

zonuexe commented Jan 8, 2025

The only remaining coding standard violations are in the implementation code in the src directory, which is unchanged in this PR.

On branch support/php84
Your branch is up to date with 'origin/support/php84'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   src/Attribute/Inject.php
        modified:   src/Attribute/Injectable.php
        modified:   src/CompiledContainer.php
        modified:   src/Compiler/Compiler.php
        modified:   src/Container.php
        modified:   src/ContainerBuilder.php
        modified:   src/Definition/Resolver/ArrayResolver.php
        modified:   src/Definition/Resolver/DecoratorResolver.php
        modified:   src/Definition/Resolver/EnvironmentVariableResolver.php
        modified:   src/Definition/Resolver/ObjectCreator.php
        modified:   src/Definition/Source/AttributeBasedAutowiring.php
        modified:   src/Definition/Source/Autowiring.php
        modified:   src/Definition/Source/DefinitionArray.php
        modified:   src/Definition/Source/DefinitionFile.php
        modified:   src/Definition/Source/DefinitionSource.php
        modified:   src/Definition/Source/NoAutowiring.php
        modified:   src/Definition/Source/ReflectionBasedAutowiring.php
        modified:   src/Definition/Source/SourceCache.php
        modified:   src/Definition/Source/SourceChain.php
        modified:   src/Definition/StringDefinition.php
        modified:   src/Invoker/DefinitionParameterResolver.php
        modified:   src/Invoker/FactoryParameterResolver.php

@esetnik
Copy link

esetnik commented Jan 13, 2025

Also relates to #896. laravel/serializable-closure^2.0 required for php 8.4 support.

@mnapoli
Copy link
Member

mnapoli commented Jan 13, 2025

Thanks!

@mnapoli mnapoli merged commit 9639582 into PHP-DI:master Jan 13, 2025
6 of 7 checks passed
@zonuexe zonuexe deleted the support/php84 branch January 15, 2025 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants