Use static closures whenever $this is not used to avoid memory leaks

[westonruter]

Historically closures were not usable in WordPress since they were introduced in PHP 5.3 and WordPress supported PHP 5.2. However, now WordPress requires PHP 5.6 so closures have started making their way into the codebase. Whenever a closure is used inside of a class instance, the $this is automatically captured by the closure. If the closure is added as a filter and not removed, for example, the class instance won't be able to be garbage collected even if there are otherwise no other references. For any such closures that don't reference $this, this can be avoided by adding a static keyword to the closure. Doing so prevents $this from being captured and thus avoids a memory leak. See ​Stack Overflow answer.

This can be avoided in the future by incoporating the ​SlevomatCodingStandard.Functions.StaticClosure sniff.

Also implemented in ​PR for Performance Lab plugin.

• Add SlevomatCodingStandard.Functions.StaticClosure sniff from ​slevomat/coding-standard
• Upgrade PHPCS from 3.6.0 to ^{3.7.1 as needed by slevomat/coding-standard}
• Add static keyword to eligible closures via Run composer run-script format -- --sniffs=SlevomatCodingStandard.Functions.StaticClosure

Trac ticket: https://core.trac.wordpress.org/ticket/58323

[jrf]

I've left a detailed review on the PR.

Summary:

• Making closures which don't use $this static is a good improvement.
• Including the Slevomat sniff is out of the question.

[spacedmonkey]

Replying to jrf:

• Including the Slevomat sniff is out of the question.

+1

Left my thoughts on the PR.

While it is fine to use external sniffs to improve the WP codebase, adding those to the ruleset is a big no-no.

@jrfnl just curious why adding these to the ruleset is not OK. I thought the ruleset here was exclusively for wordpress-develop, as it's not located in the WPCS repo.

While it is fine to use external sniffs to improve the WP codebase, adding those to the ruleset is a big no-no.

@jrfnl Just curious why adding these to the ruleset is not OK. I thought the ruleset here was exclusively for wordpress-develop, as it's not located in the WPCS repo.

True, but if we want to enforce something for WP Core, I keep being told that it has to go through a) a Make post, b) a change to the formal coding standards docs, c) only after that the standard can be changed. I don't see why it would be any different for you. 🤷🏻‍♀️

Aside from that, adding the Slevomat standard would be prohibitive for running a composer install on PHP < 7.2, which is, of course, a no-no.

[spacedmonkey]

Let's PR looks good. Marking as ready to commit.

[westonruter]

In 55822:

General: Use static on closures whenever $this is not used to avoid memory leaks.

Props westonruter, jrf, spacedmonkey.
Fixes #58323.

Committed to trunk in ​https://github.com/WordPress/wordpress-develop/commit/d5792c7a8861330d53adc05dacfdfdccb822b1d4

[westonruter]

Also opened a ​PR for Gutenberg to apply the same change.