TASK: Add basic support for PHP8 #2287

albe · 2020-11-28T20:46:29Z

This makes our code base work on PHP 8. It does not yet add support for PHP 8 features. See #2404 for this.

Related to #2233

albe · 2020-11-28T21:33:18Z

This depends on doctrine/migrations#980 now

bwaidelich · 2020-12-09T09:56:45Z

FYI: I converted this PR to a draft so that it doesn't get merged by accident (=> see doctrine/migrations#980)

albe · 2020-12-23T17:53:27Z

The blocker has been solved by the doctrine team for christmas, even for 2.3.x versions :) see doctrine/migrations#1102

But...

Cloning into 'Packages/Framework'...
fatal: reference repository '/home/travis/.cache/composer/vcs/-home-travis-
build-neos-flow-development-collection/' is shallow

❓ ❔ ❓ WTF Travis?

albe · 2020-12-23T19:01:57Z

0.16s$ composer config --json repositories.flow.options '{"symlink": false}'

[RuntimeException]
Failed to update composer.json with a valid format, reverting to the origin
al content. Please report an issue to us with details (command you run and
a copy of your composer.json).

[Composer\Json\JsonValidationException]
"./composer.json" does not match the expected JSON schema

Oh boi... 🤷‍♂️

albe · 2020-12-23T21:13:16Z

Holy cow - there you go ✔️🎁

albe · 2020-12-23T21:21:23Z

Oops, the PHP 8 tests actually fail - they just were allowed to fail because they were still "nightly" 🙈

albe · 2021-01-03T19:49:02Z

Intermediary result: Errors: 18, Failures: 16 -> Errors: 7, Failures: 7

See https://www.php.net/manual/de/migration80.deprecated.php#migration80.deprecated.libxml

…meter::getClass This method is deprecated in favor of getType(). See https://www.php.net/manual/de/migration80.deprecated.php#migration80.deprecated.reflection

See https://www.php.net/manual/en/migration80.incompatible.php#migration80.incompatible.tokenizer

Fully qualified namespaces emit a `T_NAME_FULLY_QUALIFIED` token in PHP8.

Related: #2233

albe

Added some comments for easier review

albe · 2021-03-01T18:23:01Z

Neos.Cache/Tests/Unit/Backend/FileBackendTest.php

@@ -589,7 +589,7 @@ public function requireOnceDoesNotSwallowPhpNoticesOfTheIncludedFile()
        $backend->setCache($mockCache);

        $entryIdentifier = 'SomePhpEntryWithPhpNotice';
-        $backend->set($entryIdentifier, '<?php $undefined ++; ?>');


This is no longer a E_NOTICE in PHP8

albe · 2021-03-01T18:23:45Z

Neos.Cache/Tests/Unit/Backend/SimpleFileBackendTest.php

@@ -405,7 +405,7 @@ public function requireOnceDoesNotSwallowPhpWarningsOfTheIncludedFile()
        $entryIdentifier = 'SomePhpEntryWithPhpWarning';

        $simpleFileBackend = $this->getSimpleFileBackend();
-        $simpleFileBackend->set($entryIdentifier, '<?php trigger_error("Warning!", E_WARNING); ?>');
+        $simpleFileBackend->set($entryIdentifier, '<?php trigger_error("Warning!", E_USER_WARNING); ?>');


Only E_USER_X can be manually triggered. PHP 7 was lenient with this though.

albe · 2021-03-01T18:30:00Z

Neos.Flow/Classes/Reflection/MethodReflection.php

     */
    public function getDeclaredReturnType()
    {
        if (!is_callable([$this, 'getReturnType'])) {
            return null;
        }
        $type = $this->getReturnType();
-        return $type !== null ? (string)$type : null;
+        return $type !== null ? ltrim((string)$type, '?') : null;


PHP8 returns the nullable type operator

albe · 2021-03-01T18:30:58Z

Neos.Flow/Classes/Reflection/ParameterReflection.php

     */
    public function getBuiltinType()
    {
        $type = $this->getType();
-        if ($type === null || !$type->isBuiltin()) {
+        if (!$type instanceof \ReflectionNamedType) {


Could be a ReflectionUnionType in PHP 8

albe · 2021-03-01T18:34:18Z

Neos.Flow/Classes/SignalSlot/Dispatcher.php

@@ -149,7 +149,7 @@ public function dispatch(string $signalClassName, string $signalName, array $sig
        }

        foreach ($this->slots[$signalClassName][$signalName] as $slotInformation) {
-            $finalSignalArguments = $signalArguments;
+            $finalSignalArguments = array_values($signalArguments);


PHP8 named parameters...

albe · 2021-03-01T18:44:10Z

Neos.Flow/Classes/Reflection/ReflectionService.php

            // We use parameter type here to make class_alias usage work and return the hinted class name instead of the alias
            $parameterInformation[self::DATA_PARAMETER_CLASS] = $parameterType;
+        } elseif ($parameterType === 'array') {


Else here makes sure that array is not marked as DATA_PARAMETER_SCALAR_DECLARATION

albe · 2021-03-01T18:46:01Z

Neos.Flow/Classes/Utility/PhpAnalyzer.php

                if ($type === T_STRING) {
-                    $namespaceParts[] = $value;
+                    $namespaceParts[] = ltrim($value, '\\');


Not 100% sure any more this is required, because in PHP8 namespace is parsed with above tokens. But we always expect namespaces to not start with \ and this guarantees it.

Neos.Flow/Tests/Unit/SignalSlot/DispatcherTest.php

albe · 2021-03-01T18:49:15Z

Neos.Eel/Classes/Helper/MathHelper.php

@@ -422,14 +422,14 @@ public function round($subject, $precision = 0)
     */
    public function sign($x)
    {
-        if ($x < 0) {
+        if (!is_numeric($x)) {


In PHP8 'foo' compares differently to a number: see https://3v4l.org/Zhdsh
It is more sane to first check the non-numeric case and return NAN anyway, as it is done similarly in other math helper functions.

Neos.Cache/Classes/Backend/FileBackend.php

albe · 2021-03-14T11:05:34Z

Now only Behat fails...
Created database schema.
...............---..........................................
--- Failed hooks:
BeforeScenario @fixtures # FeatureContext::resetTestFixtures()
  There is no active transaction (PDOException)
Likely related to doctrine/migrations#1104

Edit: Fixed with neos/behat@f68dea2 and all green ✔️ - the exception basically just states that the transaction around the migration was already committed. BUT that is only a broken fix, because effectively migrations will stop at the first migration containing DDL. The right solution would be to catch and ignore the exception around every single migration, which we can not do and needs to be done by doctrine/migrations. The other alternative is to change all existing mysql Migrations to include isTransactional() { return false; }
See also doctrine/migrations#1104 (comment)

Will be resolved once doctrine/migrations#1131 is released. It's merged into 2.3.x-dev already, so won't take too long I guess. I would still keep the odd semi-fix of neos/behat@f68dea2, just in case.

bwaidelich

Nice one, thanks for taking care.
+1 (by reading). Just a question/suggestion re "double-shrug" comments :)

Neos.Cache/Classes/Backend/FileBackend.php

Neos.Flow/Classes/Core/LockManager.php

Neos.Flow/Classes/Package/PackageManager.php

Neos.Utility.Files/Classes/Files.php

Neos.Cache/Classes/Backend/FileBackend.php

Neos.Flow/Classes/Core/LockManager.php

albe · 2021-03-21T20:12:45Z

https://twitter.com/paxuclus/status/1373351604930412547 - still need a second approval :)

kdambekalns

👏 Some minor tweaks, please.

Neos.Flow/Classes/ObjectManagement/Configuration/ConfigurationBuilder.php

Neos.Flow/Tests/Unit/SignalSlot/DispatcherTest.php

Neos.Flow/Classes/Reflection/ReflectionService.php

Neos.Flow/Classes/ObjectManagement/Configuration/ConfigurationBuilder.php

Neos.Flow/Tests/Unit/SignalSlot/DispatcherTest.php

albe · 2021-03-22T08:27:03Z

Thanks @kdambekalns ! All valid feedback and applied :)

kdambekalns

❤️

albe · 2021-03-22T09:19:07Z

👿 were are those unrelated test failures coming from now? 😫

kdambekalns · 2021-03-22T09:30:33Z

👿 were are those unrelated test failures coming from now? 😫

Maybe they are not enitrely unrelated? Could be from 23f4bc3#diff-7d0a1ee9681327fd82562aa2d7fcd5ed3b083a2283085f8c760eb4fab0e8c73aR1808, no?

albe · 2021-03-22T09:39:26Z

👿 were are those unrelated test failures coming from now? 😫

Maybe they are not enitrely unrelated? Could be from 23f4bc3#diff-7d0a1ee9681327fd82562aa2d7fcd5ed3b083a2283085f8c760eb4fab0e8c73aR1808, no?

🤔 ...

Neos.Flow/Classes/Reflection/ReflectionService.php

albe · 2021-03-22T12:04:19Z

There you go :)

albe added the dependencies label Nov 28, 2020

bwaidelich marked this pull request as draft December 9, 2020 09:56

albe changed the base branch from master to 7.0 December 23, 2020 21:16

albe marked this pull request as ready for review December 23, 2020 21:16

albe requested review from kdambekalns and bwaidelich and removed request for kdambekalns and bwaidelich December 23, 2020 21:16

albe mentioned this pull request Dec 26, 2020

TASK: Use GH actions for builds #2273

Merged

albe added help wanted I: WIP labels Jan 4, 2021

albe marked this pull request as draft January 4, 2021 22:58

albe mentioned this pull request Jan 27, 2021

FEATURE: Increase PHP 8 compatibility #2381

Merged

albe added 7 commits February 24, 2021 22:09

TASK: Allow doctrine/orm 2.8.x-dev to test PHP 8 compatibility

0326ac6

TASK: Conditionally invoke libxml_disable_entity_loader

7093af5

See https://www.php.net/manual/de/migration80.deprecated.php#migration80.deprecated.libxml

TASK: Remove usage of ReflectionParamater::isArray and ReflectionPara…

5169338

…meter::getClass This method is deprecated in favor of getType(). See https://www.php.net/manual/de/migration80.deprecated.php#migration80.deprecated.reflection

TASK: Avoid named parameter passing in Signal DispatcherTest

9077b6f

TASK: Fix PhpAnalyzer for PHP8

7561613

See https://www.php.net/manual/en/migration80.incompatible.php#migration80.incompatible.tokenizer

TASK: Fix PhpAnalyzer for PHP8

3cfe0c9

Fully qualified namespaces emit a `T_NAME_FULLY_QUALIFIED` token in PHP8.

TASK: Enable PHP 8.0 build

e083f68

albe force-pushed the albe-doctrine-php8 branch from 3c93473 to e083f68 Compare February 24, 2021 21:12

albe and others added 2 commits February 24, 2021 23:22

BUGFIX: Fix signal named arguments

d84c96b

FEATURE: Increase PHP 8 compatibility

0da755f

Related: #2233

albe requested a review from robertlemke February 26, 2021 20:47

albe removed I: WIP help wanted labels Feb 26, 2021

albe mentioned this pull request Mar 1, 2021

TASK: Support PHP8 types and class property promotion #2404

Merged

albe commented Mar 1, 2021

View reviewed changes

bwaidelich approved these changes Mar 14, 2021

View reviewed changes

Neos.Cache/Classes/Backend/FileBackend.php Outdated Show resolved Hide resolved

albe commented Mar 15, 2021

View reviewed changes

bwaidelich reviewed Mar 15, 2021

View reviewed changes

Neos.Cache/Classes/Backend/FileBackend.php Outdated Show resolved Hide resolved

TASK: Improve double-shrug catching comment

b2c60da

albe commented Mar 15, 2021

View reviewed changes

Neos.Flow/Classes/Core/LockManager.php Outdated Show resolved Hide resolved

albe added 4 commits March 15, 2021 11:50

TASK: Try/catch every single unlink instead of multiple

a417883

Merge branch '7.0' into albe-doctrine-php8

b94e684

TASK: Replace missing double-shrug catching comment

b61942b

Merge branch '7.0' into albe-doctrine-php8

6d3c5ca

kdambekalns requested changes Mar 22, 2021

View reviewed changes

Neos.Flow/Classes/ObjectManagement/Configuration/ConfigurationBuilder.php Outdated Show resolved Hide resolved

Neos.Flow/Tests/Unit/SignalSlot/DispatcherTest.php Show resolved Hide resolved

Neos.Flow/Classes/Reflection/ReflectionService.php Show resolved Hide resolved

albe commented Mar 22, 2021

View reviewed changes

albe added 2 commits March 22, 2021 09:22

TASK: Apply suggestions from code review

23f4bc3

TASK: Add test for unnamed dispatch signal arguments

82d5c85

kdambekalns approved these changes Mar 22, 2021

View reviewed changes

albe commented Mar 22, 2021

View reviewed changes

Neos.Flow/Classes/Reflection/ReflectionService.php Outdated Show resolved Hide resolved

BUGFIX: Use typehint annotations for arrays

c5268c8

albe merged commit 1f88b67 into 7.0 Mar 22, 2021

albe deleted the albe-doctrine-php8 branch March 22, 2021 12:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TASK: Add basic support for PHP8 #2287

TASK: Add basic support for PHP8 #2287

albe commented Nov 28, 2020 •

edited

Loading

albe commented Nov 28, 2020

bwaidelich commented Dec 9, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Jan 3, 2021 •

edited

Loading

albe left a comment

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe Mar 1, 2021

albe commented Mar 14, 2021

bwaidelich left a comment

albe commented Mar 21, 2021

kdambekalns left a comment

albe commented Mar 22, 2021

kdambekalns left a comment

albe commented Mar 22, 2021

kdambekalns commented Mar 22, 2021

albe commented Mar 22, 2021

albe commented Mar 22, 2021

TASK: Add basic support for PHP8 #2287

TASK: Add basic support for PHP8 #2287

Conversation

albe commented Nov 28, 2020 • edited Loading

albe commented Nov 28, 2020

bwaidelich commented Dec 9, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Dec 23, 2020

albe commented Jan 3, 2021 • edited Loading

albe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albe commented Mar 14, 2021

bwaidelich left a comment

Choose a reason for hiding this comment

albe commented Mar 21, 2021

kdambekalns left a comment

Choose a reason for hiding this comment

albe commented Mar 22, 2021

kdambekalns left a comment

Choose a reason for hiding this comment

albe commented Mar 22, 2021

kdambekalns commented Mar 22, 2021

albe commented Mar 22, 2021

albe commented Mar 22, 2021

albe commented Nov 28, 2020 •

edited

Loading

albe commented Jan 3, 2021 •

edited

Loading