Page MenuHomePhabricator

MediaWiki core should pass Phan on PHP 8.1
Closed, ResolvedPublic

Description

I'm not sure if simultaneous support for running Phan on PHP 7.4 and PHP 8.1 is necessary. If it is, some ugly hacks will be needed since the versions contradict each other.

I have a patch which reduces the number of failures from 27 to 6.

Event Timeline

Change 852326 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Respond to some messages from Phan on PHP 8.1

https://gerrit.wikimedia.org/r/852326

Reedy edited projects, added phan, MediaWiki-General; removed Continuous-Integration-Config.
Reedy moved this task from Backlog to MediaWiki core on the PHP 8.1 support board.
Reedy removed subscribers: Tacsipacsi, hashar.

+ @Daimona our Phan expert.

I have triggers Phan under PHP 8.1 by commenting check experimental on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/852326/3 which yields:

includes/htmlform/fields/HTMLTimezoneField.php:88
  PhanImpossibleTypeComparison Impossible attempt to check if $identifiers of type array is identical to false of type false

includes/language/LocalisationCache.php:1029
  UnusedPluginSuppression Plugin BuiltinSuppressionPlugin suppresses issue PhanTypePossiblyInvalidDimOffset on this line but this suppression is unused or suppressed elsewhere

includes/libs/MappedIterator.php:79
  PhanUndeclaredMethod Call to undeclared method \Traversable::current

includes/libs/filebackend/fileiteration/FileBackendStoreShardListIterator.php:68
  PhanUndeclaredMethod Call to undeclared method \Traversable::current

includes/parser/Parser.php:3538
  PhanPluginRedundantAssignment Assigning null to variable $revisionRecord which already has that value

includes/password/Pbkdf2PasswordUsingHashExtension.php:49
  PhanRedundantCondition Redundant attempt to cast $hash of type string to string

Some have @phan-suppress-next-line indentation albeit with a different error name. Maybe the names are slightly different when run under php 8+ or they hit some differences. The last one explicitly states the cast is no more necessary on PHP 8.0+.

Under PHP 8.0 there are a lot of PhanUndeclaredClassAttribute Reference to undeclared class \ReturnTypeWillChange in an attribute (annotations added via T289879). Maybe we need a stub for this class when running under PHP 8.0?

Yes, those are the 6 errors I mentioned. My analysis:

  • HTMLTimezoneField: It was discussed at https://github.com/phan/phan/issues/3162. It just needs a suppression, but the issue name has changed.
  • LocalisationCache: unused suppression but it is needed by PHP 7.4.
  • MappedIterator, FileBackendStoreShardListIterator: obviously a false positive since the type is Iterator not Traversable. Ideally I'd like to tell Phan what the type is, but @method didn't seem to work. We may need to add a variable. Or just suppress.
  • Parser: No idea what Phan is thinking here. $revisionRecord is false.
  • Pbkdf2PasswordUsingHashExtension: This check is needed until PHP 8.0 as I noted in a comment on my patch. Needs a suppression.

Change 852326 merged by jenkins-bot:

[mediawiki/core@master] Respond to some messages from Phan on PHP 8.1

https://gerrit.wikimedia.org/r/852326

Change 854649 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Improve LocalisationCache post-merge validation check

https://gerrit.wikimedia.org/r/854649

Change 854653 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Fix the remaining Phan failures on PHP 8.1

https://gerrit.wikimedia.org/r/854653

Change 854649 merged by jenkins-bot:

[mediawiki/core@master] Improve LocalisationCache post-merge validation check

https://gerrit.wikimedia.org/r/854649

Change 854653 merged by jenkins-bot:

[mediawiki/core@master] Fix the remaining Phan failures on PHP 8.1

https://gerrit.wikimedia.org/r/854653

Change 855046 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] Zuul: [mediawiki/core] Add PHP 8.1 phan job

https://gerrit.wikimedia.org/r/855046

Under PHP 8.0 there are a lot of PhanUndeclaredClassAttribute Reference to undeclared class \ReturnTypeWillChange in an attribute (annotations added via T289879). Maybe we need a stub for this class when running under PHP 8.0?

I talked with @Krinkle about this yesterday. He suggested not running Phan on PHP 8.0. Just run PHPUnit only.

Change 855046 merged by jenkins-bot:

[integration/config@master] Zuul: [mediawiki/core] Add PHP 8.1 phan job

https://gerrit.wikimedia.org/r/855046

Mentioned in SAL (#wikimedia-releng) [2022-11-09T23:02:25Z] <James_F> Zuul: [mediawiki/core] Add PHP 8.1 phan job for T322278