Page MenuHomePhabricator

Excimer PHP 8.0 support
Closed, ResolvedPublic

Description

@Ammarpad @Reedy I'm note expert on how phan stubs are used, but https://gerrit.wikimedia.org/g/mediawiki/core/+/22febcecc591d8ad5050837c24dcaf15cbc5ebd8/.phan/internal_stubs/excimer.php includes 2 private final functions
Do those need to be removed?

The stubs in Excimer probably want updating before we update them in MW core too

Event Timeline

I don't know how those stubs were generated, but they don't seem to come from the internal stub generator included provided by phan. To regenerate them (which might or might not fix the issue with private final, depending on where it comes from), one should follow these instructions.

Note that the extension itself also needs to compile with PHP 8.

I don't know how those stubs were generated, but they don't seem to come from the internal stub generator included provided by phan. To regenerate them (which might or might not fix the issue with private final, depending on where it comes from), one should follow these instructions.

They were written by hand AFAICT: https://gerrit.wikimedia.org/r/c/mediawiki/php/excimer/+/533147

So all that's left is to regenerate the stubs the way phan wants them? Or can we just remove the private final methods and call it good enough?

Note that the extension itself also needs to compile with PHP 8.

1.0.2 builds fine on 8.0 and 8.1 now with various fixes from Tim and Remi.

Change 754499 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Re-generate excimer stubs using phan's make_stub

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

I've regenerated the stubs using make_stub, but the final privates are still there. At this point, I assume it's something within the extension that sets both modifiers.

Ah yeah, it's declared at https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/php/excimer/+/refs/heads/master/excimer.c#384.

But now that I looked into it, only the two constructors are marked as final private, which per https://php.watch/versions/8.0/final-private-function is still allowed as an exception to the final private deprecation.

Hah :)

But now that I looked into it, only the two constructors are marked as final private, which per https://php.watch/versions/8.0/final-private-function is still allowed as an exception to the final private deprecation.

Nice to hear that! Given that the extension seems to work fine (or at least compile) on 8.0 and 8.1, should we close this as resolved?

Changing the stub to be automatically generated should be a good idea regardless, but it doesn't have to happen as part of this task (also, phan is failing because aggregateByFunction() has no declared return type, and phan assumes that the result of unknown_type + unknown_type will be a number, rather than an array, so this needs to be fixed).

Nice to hear that! Given that the extension seems to work fine (or at least compile) on 8.0 and 8.1, should we close this as resolved?

Yep.

Changing the stub to be automatically generated should be a good idea regardless, but it doesn't have to happen as part of this task (also, phan is failing because aggregateByFunction() has no declared return type, and phan assumes that the result of unknown_type + unknown_type will be a number, rather than an array, so this needs to be fixed).

Ask and you shall receive :) https://gerrit.wikimedia.org/r/c/mediawiki/php/excimer/+/754574

Change 773878 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/php/excimer@master] Release 1.0.3

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

Change 759201 had a related patch set uploaded (by Krinkle; author: Legoktm):

[mediawiki/php/excimer@master] Set return type for ExcimerProfiler::getLog() too

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

Change 759201 merged by jenkins-bot:

[mediawiki/php/excimer@master] Set return type for ExcimerProfiler::getLog() too

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

Change 773878 merged by jenkins-bot:

[mediawiki/php/excimer@master] Release 1.0.3

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

Change 773878 merged by jenkins-bot:

[mediawiki/php/excimer@master] Release 1.0.3

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

... and tagged in Git, and uploaded to https://pecl.php.net/package/excimer as per docs.

Change 789165 had a related patch set uploaded (by Krinkle; author: Remi Collet):

[mediawiki/php/excimer@master] Fix arginfo in PHP 7.1 for ExcimerLog::aggregateByFunction()

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

Change 789165 merged by jenkins-bot:

[mediawiki/php/excimer@master] Fix arginfo in PHP 7.1 for ExcimerLog::aggregateByFunction()

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

Change 789905 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/php/excimer@master] Release 1.0.4

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

Change 789905 merged by jenkins-bot:

[mediawiki/php/excimer@master] Release 1.0.4

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

Change 940488 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] tests: Update Phab stub to php-excimer 1.1.1

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

Change 940488 merged by jenkins-bot:

[mediawiki/core@master] tests: Update Phan stub to php-excimer 1.1.1

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