[RFC] Reference global functions and constants via use

[Majkl578]

Motivation

As of PHP 5.3, function invocation in namespaced code is two-step operation. First, function is looked up in current namespace, if not found the fallback mode looks up the function in global scope. This effectively means that global function calls from namespaced code are slower and additionally kill further optimizations and/or inlining into special opcodes, done for some core functions. Granted, OPcache likely optimizes the two-step function lookups and further optimizes some other functions, but won't make them compile into specific opcode, and is also usually disabled in CLI.

Solution

There are two ways how to avoid fallback lookups:

1. prepend function calls by backslash (\strlen()),
2. add functions into imports (use function strlen).

Although approach 1) is more compact at first sight, it clutters code a lot with backslashes making it harder to read. Imports on the other hand don't - the code itself stays as simple as before and untouched. PHP optimizations do not differentiate between 1) and 2) or even import aliases so either is okay. With proper IDE support both are pretty easy to handle without much hassle.

Further reading

• https://veewee.github.io/blog/optimizing-php-performance-by-fq-function-calls/
• http://php.net/manual/en/language.namespaces.faq.php#language.namespaces.faq.shortname2
• https://github.com/Roave/FunctionFQNReplacer#rationale
• https://twitter.com/Ocramius/status/811504929357660160
• https://3v4l.org/YiCjS/vld#output

IDE support

PhpStorm supports autoimports of global functions and constants for a while already: https://blog.jetbrains.com/phpstorm/2017/01/phpstorm-2017-1-eap-171-2455/

Based on discussion and implementation for doctrine/orm#6807.
ORM example (unsorted): Majkl578/doctrine-orm@78fc09c

I'd like to hear some feedback on this topic and proposed solution. Thanks.

[Ocramius]

Need a test with a constant too

[Ocramius]

Wait... Constants too? There is no import syntax for them, no?

[lcobucci]

https://3v4l.org/UPWXB

[Ocramius]

Dang, didn't remember use const 👍

[lcobucci]

🎉

[lcobucci]

@Ocramius has a point regarding tests though =)

[alcaeus]

I'm in favor of this. Do we have a sniff in place regarding sort order of use statements? I'm thinking of

use const BLAH;
use Foo\Bar;
use function strsomething;

vs.

use Foo\Bar;
use const BLAH;
use function bleh;

It boils down to either sorting all statements mixed or grouping the different use statements (use (with class being implied), use const and use function) together and sorting them as groups.

Quick addition: I'm in favor of sorting them separately FWIW.

[Ocramius]

sorting all statements mixed

Kinda ugly - would really be better to have 3 separate sections there

[lcobucci]

AFAIK the sniff will sort them separately just like phpstorm does (no line breaks), but that's another good thing to test

[Ocramius]

No need for any spacing, let's just not mix the various use statements together :-P

[alcaeus]

@lcobucci sounds good to me 👍

[kukulich]

Yes, sorting provides this sniff: https://github.com/doctrine/coding-standard/blob/master/lib/Doctrine/ruleset.xml#L103

It sorts types in the same order as phpstorm (classes, fuctions, constants)

[Ocramius]

🚢

[Majkl578]

Do we have a sniff in place regarding sort order of use statements?

Sorry for confusion, there is a sniff for sorted uses (linked above), but I didn't apply it as it would change ton of other uses and diff would be less relevant.

I'll extend a test with some constants later and then we can ship this as 2.1. Glad to see agreement on this one. 👍

[Majkl578]

Test provided in #16.

[nicwortel]

Since this repo has issues disabled, I'm asking this here: as shown in the screenshot, you recommend checking "Enable auto-import from the global space" in PhpStorm. This will also automatically import classes from the global namespace (such as DateTime, the SPL exceptions, etc.). Have you considered enforcing this in the coding standard as well for consistency? I.e. enforcing use DateTime instead of new \DateTime().

[Ocramius]

@nicwortel that would make a lot of sense, to be honest, as that's an open question. Open a PR with just a proposed test change.

[kukulich]

It would be good for consistency because currently both new DateTime() and new \DateTime() are allowed, no version is required.

[Majkl578]

FYI:

I.e. enforcing use DateTime instead of new \DateTime().

^ is done via #43 and will be in 5.0.0. 🎉