Page MenuHomePhabricator

Use static closures where appropriate in MW Core
Closed, ResolvedPublic

Description

To be discussed

This closure can be declared as static (better scoping; in some cases can improve performance)

There's ~550 usages in MW core of closures that can be declared static

Event Timeline

Reedy renamed this task from Use static closures where appropriate to Use static closures where appropriate in MW Core.Feb 6 2021, 6:00 PM

Change 662083 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Tests: Start marking some closures as static

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

Change 662085 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Tests: Mark rest of closures as static

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

Change 662083 merged by jenkins-bot:
[mediawiki/core@master] Tests: Start marking some closures as static

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

Change 662101 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] maintenance: Mark some closures as static

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

Change 662102 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] maintenance: Mark some closures as static

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

Change 662103 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] maintenance: Mark some closures as static

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

Change 662111 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Tests: Mark more more closures as static

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

Change 662085 merged by jenkins-bot:
[mediawiki/core@master] Tests: Mark more closures as static

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

What does this change actually do? Does it just prevent using $this inside the function?

What does this change actually do? Does it just prevent using $this inside the function?

Pretty much, you also can't use Closure::bindTo() - https://www.php.net/manual/en/functions.anonymous.php#functions.anonymous-functions.static

Change 662111 merged by jenkins-bot:
[mediawiki/core@master] Tests: Mark more more closures as static

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

Change 662101 merged by jenkins-bot:
[mediawiki/core@master] maintenance: Mark some closures as static

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

Change 662102 merged by jenkins-bot:
[mediawiki/core@master] Resources.php: Mark some closures as static

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

Change 662103 merged by jenkins-bot:
[mediawiki/core@master] Language.php: Mark some closures as static

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

Most cases are done, few new cases and may be some old cases which are hard to tell from a static code analyzer.

Not sure about the count in the task description now looks like

Not sure if it is possible to see any (positive) performance impact on the metrics in production

Umherirrender claimed this task.