Page MenuHomePhabricator

Phan rejects variadic calls to BagOStuff::makeKey()
Closed, ResolvedPublic

Description

Unrelated to this patch but appeared while checking: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/524637/

10:15:35 <?xml version="1.0" encoding="ISO-8859-15"?>
10:15:35 <checkstyle version="6.5">
10:15:35   <file name="src/Key/TOTPKey.php">
10:15:35     <error line="123" severity="info" message="Call with 3 arg(s) to \BagOStuff::makeKey() which only takes 2 arg(s) defined at ../../includes/libs/objectcache/BagOStuff.php:456" source="PhanParamTooMany"/>
10:15:35   </file>
10:15:35 </checkstyle>

Thanks.

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 20 2019, 10:21 AM
Daimona added a subscriber: aaron.Jul 20 2019, 1:56 PM

This happens because AFAICS, BagOStuff::makeKey is variadic and can take an arbitrary amount of parameters. However, it's not documented in the docblock or inside the signature, so phan can't know that. And the same goes for humans, actually.

As for why this didn't happen before, I see @aaron made some changes to the BagOStuff & related classes recently, so some change in the class structure may have altered the analysis.

Sort of related to T191668 and T191666.

Krinkle renamed this task from OATHAuth phan failure to Phan rejects variadic calls to BagOStuff::makeKey().Jul 20 2019, 4:51 PM
Krinkle added a project: MediaWiki-Cache.

Change 524646 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] objectcache: Use variadic signature for makeKey()

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

Change 524648 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/Babel@master] tests: Use empty WANObjectCache instead of mock

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

Change 524647 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/EventLogging@master] tests: Simplify RemoteSchemaTest to not hardcode MemcachedPhpBagOStuff

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

Change 524647 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@master] tests: Simplify RemoteSchemaTest to not hardcode MemcachedPhpBagOStuff

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

Change 524648 merged by jenkins-bot:
[mediawiki/extensions/Babel@master] tests: Use empty WANObjectCache instead of mock

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

Daimona triaged this task as High priority.Jul 22 2019, 12:31 PM

This is blocking merges in whatever project is calling make(Global)Key with more than 2 args and has phan enabled.

Change 524839 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/Echo@master] Fix nonsense Phan errors about too many arguments

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

Change 524646 merged by jenkins-bot:
[mediawiki/core@master] objectcache: Use variadic signature for makeKey()

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

Daimona closed this task as Resolved.Jul 23 2019, 7:02 PM
Daimona assigned this task to Krinkle.
Daimona removed a project: Patch-For-Review.

Change 524839 abandoned by Catrope:
Fix nonsense Phan errors about too many arguments

Reason:
Superseded by core patch

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

Change 525623 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@wmf/1.34.0-wmf.15] objectcache: Use variadic signature for makeKey()

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

Change 525623 abandoned by Krinkle:
objectcache: Use variadic signature for makeKey()

Reason:
Not a test-only change. Will avoid for now. Better luck next time :)

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