Comments (5)
@AlexP11223 closures outside of classes can still be bound to objects if not static (see https://3v4l.org/ASFRq).
That is normally not really relevant: considering those closures should not contain $this
, even if they are bound the result of calling them will be the same.
However, if you make it static, there are still a few (minor) benefits.
static
explicitly states that the closure must not be bound. Being explicit is always a good thing.- Adding "static" to an existing closure is BC break. Imagine you have a closure without static. Someone decides to bind it for any reason. You later add
static
. The other person code breaks. Removing static, on the other hand, is not a BC break. So creating a closure withstatic
offers a tiny benefit in terms of future compatibility. - It prevents one to make some mistakes. Imagine that because of copy & paste there's accidentally a
$this
inside a closure. PHP will not complain at compile time, but will complain at runtime. If that closure isstatic
, static analyzers, like PHPCS or Psalm or even the IDE, are able to discover that sort of mistakes before the code is executed. - To bind a closure prevents the bound object to be garbage collected. Making it static, if is meant to be static, that just can't happen.
As I said, those are minor benefits, but it is also true that typing static
is not a big deal either. This is why the sniff is triggered also for closures ouside classes: if one gets that warning from PHPCS, they can type static
without thinking too much and forget about it.
That said, what's above is my opinion, but these are company's coding styles. So if there're consensus about not suggesting static
for closures outside classes, let's just do it. If that is your opinion please ask our colleagues to express theirs, and I can take care of doing what majority of devs want (unless someone else doesn't want to take care also of the implementation).
from php-coding-standards.
To me does make sense to keep it despite PHPCompatibility
since it sounds more our own rule and we shouldn't rely on a sort of inheritance.
It also make sense to keep it and the rare cases where it's not really needed we can simply use phpcs:ignore
or phpcs:disable
.
My reason is that after almost 3 years we are discussing it now. Just to point out the rarity of the exception.
from php-coding-standards.
Thank you for the enlightening writeup
from php-coding-standards.
If that closure is
static
, static analyzers, like PHPCS
It detects without static too, e.g. in my example above
- $plugin = $f->createPluginFromFilePath($mainFile);
+ $plugin = $this->createPluginFromFilePath($mainFile);
WARNING | Closures / anonymous functions only have access to $this if used within a class or when bound to an object using bindTo().
| | Please verify. (PHPCompatibility.FunctionDeclarations.NewClosure.ThisFoundOutsideClass)
from php-coding-standards.
I'm trying to ge ta clean status of issues for a stable release, so I'm closing this because no activity since months.
from php-coding-standards.
Related Issues (20)
- [Feature Request]: Add check for Emacs UTF-8 file header HOT 1
- [Question] When using phpcbf there is a bug for the rule that leaves the first line with opening php tag HOT 3
- Review Squiz.Scope.MethodScope.Missing rule in interfaces and PhpAnnotator conflict HOT 1
- [Feature Request]: Add check for whitespace after type casting HOT 3
- [Feature Request]: Bump squizlabs/php_codesniffer HOT 1
- [Feature Request]: Add a rule against unused imports HOT 2
- [Feature Request]: Add rules for arrow functions
- [Feature Request]: Add more tests for array spread operator
- [Feature Request]: Add a sniff for unused services HOT 2
- [Feature Request]: Update dependencies to be compatible with PHP 8
- [Feature Request]: Get rid of Neutron PHP Standard
- [Feature Request]: Configure Dependabot HOT 2
- [Feature Request]: Evaluate which "Slevomat" coding standard sniffs to use HOT 2
- [Feature Request]: Embrace PHPCSUtils
- [Upgrade]: Modernize existing sniffs
- [Feature Request]: Add a sniff to enforce property type HOT 5
- [Feature Request]: Replace cyclomatic complexity with Cognitive Complexity
- Having trouble finding a working composer.json configuration HOT 1
- [Feature Request]: Enforce no spaces in array brackets HOT 4
- [Feature Request]: Enforce no space after negation operator
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from php-coding-standards.