Fix PHP 7.4 regression, changed behavior of get_declared_classes()

[stronk7]

Since PHP 7.4 get_declared_classes() does not guarantee any order. That
implies that parent classes aren't the first any more, rendering the
array_reverse() technique futile for the loop & break code that follows.
So, additionally, let's try to reduce the list of candidates by removing all
the classes known to be "parents". That way, at the end, only the "main"
class just included with remain.

Source:
https://raw.githubusercontent.com/php/php-src/PHP-7.4/UPGRADING

Text:
Previously get_declared_classes() always returned parent classes before
child classes. This is no longer the case. No particular order is guaranteed
for the get_declared_classes() return value.

---

Part of the move to phpcs3 we were getting some strange errors with phpunit, only reproducible with php74, with error:

PHP_CodeSniffer\Exceptions\RuntimeException: No sniffs were registered

Tracing down the problem to the autoloader, we were getting these classes with php73 (with the "main" one returned the 1st, that is a requirement for the loop in code to work):

Moodle 4.0dev (Build: 20200924), f09429e7f727a4f3ff5bc8da0029781a6fd194a4
Php: 7.3.22, pgsql: 9.6.19, OS: Darwin 19.6.0 x86_64
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.

Array
(
    [0] => PHPCompatibility\Sniffs\FunctionUse\RemovedFunctionsSniff
    [1] => PHPCompatibility\AbstractRemovedFeatureSniff
    [2] => PHPCompatibility\AbstractComplexVersionSniff
    [3] => PHPCompatibility\Sniff
)

And these classes with php74:

Moodle 4.0dev (Build: 20200924), f09429e7f727a4f3ff5bc8da0029781a6fd194a4
Php: 7.4.10, pgsql: 9.6.19, OS: Darwin 19.6.0 x86_64
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.

Array
(
    [0] => PHPCompatibility\Sniff
    [1] => PHPCompatibility\AbstractComplexVersionSniff
    [2] => PHPCompatibility\AbstractRemovedFeatureSniff
    [3] => PHPCompatibility\Sniffs\FunctionUse\RemovedFunctionsSniff

That obviously, doesn't work because the first element in the array is not the just included class.

With the patch applied, and being able to reduce the candidates to all the classes not being parents in the hierarchy... everything is working as expected:

Moodle 4.0dev (Build: 20200924), f09429e7f727a4f3ff5bc8da0029781a6fd194a4
Php: 7.4.10, pgsql: 9.6.19, OS: Darwin 19.6.0 x86_64
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.

Array
(
    [3] => PHPCompatibility\Sniffs\FunctionUse\RemovedFunctionsSniff
)

I've looked for more occurrences of get_declared_classes() in codebase and haven't found any.

And that's the story, so far. For your consideration, ciao :-)

[stronk7]

(patch amended to make codechecker PHPCS style happy, wow)

[jrfnl]

Alternative solution to the same issue being addressed in the already open PR #3052

[stronk7]

Wow, I think I searched by get_declared_classes and/or php 7.4, I'm happy either way, as far as it works. Thanks!

[gsherwood]

The alternate solution in #3052 doesn't cover some edge cases in my testing, but this fix does. I've got to write a few more tests to confirm the behaviour before I merge it, but it's looking good. Thanks a lot for this PR.

[gsherwood]

Thanks a lot for submitting this PR. The fix worked in all my testing, but it would be terrific if anyone else can also confirm that master is now good.

[stronk7]

Thanks @gsherwood !

My side I can confirm that it works both for PHPCompatibility (that is added as a whole to our standard) and for some own Sniffs that are extending from the Generic standard. Sure there are more complex cases out there, heh!

Ciao :-)

[jrfnl]

For the most prominent test case with which I could previously consistently reproduce the issue, the fix now committed seems to work. 👍

I'll be running with master anyway, so will give additional feedback when I have it. That is, other than the concern I pointed out earlier in #3052 (comment):

I am a little worried about potential fatal "class already declared" errors due to the continued use of include instead of include_once and the parent class not being added to the self::$loadedClasses/self::$loadedFiles properties.

Code technically:

• I've been trying to find the commit in the PHP source which introduced this change, but haven't been able to find it yet. Reason being, is that the issue which caused the changelog note to be added, basically says that the order was always unspecified, the order previously being what it was being incidental and shouldn't be counted on, which makes me wonder whether the same doesn't also apply to get_declared_interfaces() and get_declared_traits().
• Other than that, I wonder how this impacts performance for standards with a lot of sniffs as the array_* functions are notorious for their performance issues. I haven't run any tests yet for that though.