- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Went searching for something like this. I think this could be better titlted: "Require backslash prefix for global functions and constants" so renamed.
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Article on the performance improvement of using FQNS'd global functions https://veewee.github.io/blog/optimizing-php-performance-by-fq-function-...
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
I think it would be easier if this was only required for new code.
- π¬π§United Kingdom catch
Agreed we should do this. There are also compiler-level optimisations for
\array_key_exists()
that only work when it's explicitly namespaced.I think we could 'require' it for all code, but not enable the rule until core is actually compliant?
- π©πͺGermany donquixote
Is there any argument for "use" vs in-place backslash?
Personally I think I prefer in-place backslash, but "use" seems more common in composer packages.Clearly in-place backslash is more git-friendly, that is, it reduces merge conflicts from changes in unrelated places.
(In my day to day work I currently don't use either of the two methods, because I would have to convince my colleagues first.)
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
phpcbf can fix this so we can turn the rule on and make the decision at the same time
- π¬π§United Kingdom catch
I think we should only use
use
for functions where they're namespaced functions, which we currently don't have any/many of but could in the future. - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
#8 yeah that makes sense to me too.
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
I think this is just
SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalFunctions
? - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Added the template to the issue summary.
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Added myself and @kim.pepper as two supporters
- π¦πΊAustralia dpi Perth, Australia
+ support
Discussion for core: π± Call PHP native functions fully qualified (like \array_key_exists()) Active
External discussion in derivative standard: https://github.com/previousnext/coding-standard/pull/69
- π³πΏNew Zealand quietone
Current practice is to use 'use' statements for constants. So, I think we can reduce scope here to global functions.
- π³πΏNew Zealand quietone
This was discussed at the last coding standards meeting. There was some performance testing done, the results are in #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC β .
- πΊπΈUnited States daletrexel Minnesota, USA
(As noted over in [#])
I was curious about this topic in general after seeing the pattern appear in some code I was reviewing. A search turned up this article, which has a very different opinion on the matter:
https://medium.com/@steven_3682/the-definitive-case-against-a-backslash-before-a-php-native-function-9f1f51c4415
Has anyone done benchmark tests other than the ones in the original article to confirm that a change this big in Drupal is worth the predicted performance gains? It looks like the ones run in #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC β were only the ones from the original article, and even then found to be pretty insignificant.
- π©πͺGermany donquixote
I would not put too much emphasis on the performance argument.
The medium article from #20 looks interesting, but I have not looked into it enough to assess it.
Certainly it's good to understand what we are optimizing for.Currently we live in a world where most functions are in the global namespace.
This applies to native php functions and to functions from Drupal modules.
Many packages don't want to add functions, they only want to add classes.
(there have been interesting discussions in the php internals list about static methods vs functions)If more packages and modules embrace and provide namespaced functions, the potential for ambiguity becomes bigger.
One thing here is technical ambiguity, where actually the wrong function is called.
The other thing is confusion when reading code, because you don't immediately see if a function is in the global namespace or not.I was long convinced that backslash is the way to go, but maybe we should also consider imports, and check how projects outside of Drupal do it.
- π§πͺBelgium BramDriesen Belgium π§πͺ
This has been a discussion point in the MR's of our team for our projects. Not so much about what is in the IS but more about the global
\Drupal::
calls in none name-spaced files (e.g. .module).I would usually write my code with the leading backslash.
$facet_manager = \Drupal::service('facets.manager');
But then a team member would come in saying the leading backslash is not required as we're not in a namespace. Looking at Drupal core, I can see a mix of both being used, so I think it's good if we could standardise this as a standard.
Core example where it is done with a
\
: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/autom...Core example where it is done withouth a
\
: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/toolb...Happy to say thought that there are not that many cases without the
\
PS: I find coding standards difficult English to read π so if this is not the same thing we're discussing let me know and I'll create a new issue.
- π§πͺBelgium borisson_ Mechelen, π§πͺ
#22, this is about calls to functions specifically, because of the performance part. Yours is different enough to warrant it's own issue. Especially since I think it is a lot less controversial.
- π§πͺBelgium BramDriesen Belgium π§πͺ
Okay π done! β¨ Coding standard to enforce backslash on function calls outside of a namespace Active
- πΊπΈUnited States cmlara
One item I don't see mentioned is mockability for Unit Testing.
Inside a namespaced class a Global functions preceded by a backslash can not be mocked while global functions called without a preceding backslash can be.
How widely this trick is used I am not sure, and ideally we are moving away from global functions, however eventually they do get called (especially when you start working with more 'low level' code), If you are going to simulate every code flow execution sometimes you just have to mock the response.
IIRC I've tried this once and it worked decently to abstract away having to include 3rd party code into the test case.
- π³πΏNew Zealand quietone
Another issue that needs a change record.