Page MenuHomePhabricator
Create Task
Maniphest T359099

MediaWikiNoEmptyIfDefined issue in Cite extension with parsoid class
Closed, InvalidPublic
Actions

Description

In change https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Cite/+/1003529 a suppression for MediaWikiNoEmptyIfDefined was added to pass CI under php7.4.

Locally running phan with php8.2 gives:

Plugin BuiltinSuppressionPlugin suppresses issue MediaWikiNoEmptyIfDefined in this file but this suppression is unused or suppressed elsewhere

It seems there is a issue with the mediawiki plugin handling empty

The parsoid class does not declare the properties and therefor it is a false positive under php7.4.
The parsoid class is using @property, the #[\AllowDynamicProperties] maybe makes this a non-issue in php8.2 (T349432)

This could be a blocker for T353276: Make Phan on PHP 8.1 voting for wmf-deployed extensions/skins

Related Objects

Event Timeline

Umherirrender created this task.Mar 4 2024, 7:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2024, 7:32 PM
Daimona subscribed.Mar 4 2024, 9:35 PM

So, there are a few things going on here...

  • The plugin does account for the AllowDynamicProperties attribute (or more accurately, phan does, via Clazz::hasDynamicProperties), so it makes sense that no error is reported when using PHP 7.4.
  • I see that DataParsoid uses the @property annotation to document properties that might for performance be unset. TTBOMK, phan treats @property the same an actual property declaration, as it trusts the developer to only document properties that actually exist. Obviously this can be used differently, but the consequences are on the developer. I don't think there's a way to fix this.
  • As I said multiple times, we can only pick one between 1) running phan on multiple version and 2) having consistent error output. If we want to keep running phan on multiple PHP versions, we need to be creative with the suppressions to accomodate all the versions. In this case, suppressing BuiltinSuppressionPlugin for the file might work.
Umherirrender closed this task as Invalid.Thu, May 9, 9:20 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL