Page MenuHomePhabricator

Define varargs in \IDatabase::buildLike() in a way phan can understand it
Closed, ResolvedPublic

Description

Running phan (0.12.5) on some extensions I see often:

<error line="" severity="info" message="Call with 2 arg(s) to \Wikimedia\Rdbms\IDatabase::buildLike() which only takes 0 arg(s) defined at ../includes\libs\rdbms\database\IDatabase.php:1138" source="PhanParamTooMany"/>

https://gerrit.wikimedia.org/g/mediawiki/core/+/master/includes/libs/rdbms/database/IDatabase.php#1138

I would say it needs @param string|LikeMatch,...

I have no idea how to fix this for phan.
Doing things only for a static analyzer is not nice, but the tool is helpful to find other mistakes.
Maybe it needs an own Plugin to declare the varargs on runtime inside phan

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 6 2018, 9:16 PM

Change 439303 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Document varargs for IDatabase::buildLike

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

Change 439303 had a related patch set uploaded (by Krinkle; owner: Umherirrender):
[mediawiki/core@master] Document varargs for IDatabase::buildLike

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

Change 502019 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] rdbms: Use @phan-param to simulate variadic argument

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

Krinkle triaged this task as High priority.Apr 13 2019, 7:02 PM
Krinkle edited projects, added Performance-Team; removed MediaWiki-extensions-General.
Krinkle claimed this task.Apr 15 2019, 7:56 PM

Change 502019 abandoned by Krinkle:
rdbms: Use @phan-param to simulate variadic argument

Reason:
Superseded by I4b8f2b8a8b

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

Krinkle reassigned this task from Krinkle to aaron.Apr 23 2019, 5:33 PM
Krinkle moved this task from Blocked or Needs-CR to Doing on the Performance-Team board.
Krinkle added a subscriber: Krinkle.
Krinkle added a comment.EditedJun 17 2019, 10:03 PM

Copied here from code review (https://gerrit.wikimedia.org/r/439303) for easy reference:


Fatal error: Parameter $params is variadic and has a type constraint (array); variadic params with type constraints are not supported in non-Hack files in /workspace/src/vendor/phpunit/phpunit-mock-objects/src/Framework/MockObject/Generator.php(290)(4aa354d35306d0068dd7cfe6fa0b0a25) : eval()'d code on line 1726

Comes from PHPUnit, which is wrongly inferring it from.. something.
Looks like this is due to a bug in HHVM's implementation of ReflectorClass/ReflectorParameter
https://3v4l.org/1spSX
On HHVM 1.13+, a variadic parameter is considered as having a type of 'array'. This makes some sense in that the lexical binding will indeed hold an array. But, it's a problem for the way PHPUnit reads it, because it uses that information to build a to-be-eval()'ed signature having the type "array", like "function( array ....$params )" which is not valid syntax in PHP 5.6 mode on HHVM. Because it does not support type-hinted splatted parameters.


So, the only issue is that PHPUnit 4 on HHVM is unable to create a mock object for this kind of signature.

Change 439303 merged by jenkins-bot:
[mediawiki/core@master] rdbms: Document varargs for IDatabase::buildLike

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

aaron closed this task as Resolved.Jun 18 2019, 3:28 PM

Change 519728 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] rdbms: Type IDatabase::buildLike param as variadic to Phan (really)

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

This is still failing on the same JADE patch...
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Jade/+/511080/
https://integration.wikimedia.org/ci/job/mwext-php72-phan-docker/995/console

14:09:11 <?xml version="1.0" encoding="ISO-8859-15"?>
14:09:11 <checkstyle version="6.5">
14:09:11   <file name="maintenance/CleanJudgmentLinks.php">
14:09:11     <error line="171" severity="info" message="Call with 2 arg(s) to \Wikimedia\Rdbms\IDatabase::buildLike() which only takes 1 arg(s) defined at ../../includes/libs/rdbms/database/IDatabase.php:1234" source="PhanParamTooMany"/>
14:09:11   </file>
14:09:11 </checkstyle>

Change 519728 abandoned by Krinkle:
rdbms: Type IDatabase::buildLike param as variadic to Phan (really)

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

Restricted Application added a project: Core Platform Team. · View Herald TranscriptJul 30 2019, 2:28 PM
Krinkle closed this task as Resolved.Aug 15 2019, 4:38 PM

Looks like the Jade repo is no longer affected. And gated extensions are also fine. Closing for now.

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:09 PM

Change 540830 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Use varargs for IDatabase::buildLike

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

Change 540830 merged by jenkins-bot:
[mediawiki/core@master] Use varargs for IDatabase::buildLike

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

Change 542248 had a related patch set uploaded (by Reedy; owner: Umherirrender):
[mediawiki/core@REL1_33] rdbms: Document varargs for IDatabase::buildLike

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

Change 542249 had a related patch set uploaded (by Reedy; owner: Umherirrender):
[mediawiki/core@REL1_32] rdbms: Document varargs for IDatabase::buildLike

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

Change 542249 abandoned by Reedy:
Backport NameTableStoreTest::getCallCheckingDb simplification

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

Change 542248 abandoned by Reedy:
Backport NameTableStoreTest::getCallCheckingDb simplification

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