request for additional configuration option for native_function_invocation

[SignpostMarv]

I've finally been getting around to using php-cs-fixer 2.x (not waiting around for #2553 to be resolved), and I ended up experimenting with it on vimeo/psalm#152, but some of the tests produce a rather verbose diff file ([nearly 7000 lines)(https://gist.github.com/SignpostMarv/840cac5070e7b5e840588b8e51903c18#file-native_function_invocation-only-diff)).

The argument behind native_function_invocation is to speed up resolving, and most things that improve performance are good things, but having such large diffs in pull requests is probably a bad idea (for several reasons that I won't go into at the moment).

So a thought occurred to me; use function strtolower; produces the same opcodes as \strtolower(); see 3v4l.org: backslash vs. 3v4l.org: use function.

Since use function has been available since 5.6, would it be possible to have a configuration option added to native_function_invocation to enable smaller diffs to be generated with the same performance improvement? i.e.:

$rules = [
    'native_function_invocation' => [
        'import' => true, // defaults to false to preserve existing behaviour
    ],
];

for convenience/those not wanting to click on 3val.org links:

<?php

namespace Foo;

use function strtolower;

strtolower('Foo');

Finding entry points
Branch analysis from position: 0
Jump found. (Code = 62) Position 1 = -2
filename:       /in/IUsaC
function name:  (null)
number of ops:  4
compiled vars:  none
line     #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
   7     0  E >   INIT_FCALL                                               'strtolower'
         1        SEND_VAL                                                 'Foo'
         2        DO_ICALL                                                 
         3      > RETURN                                                   1

<?php

namespace Foo;

\strtolower('bar');

Finding entry points
Branch analysis from position: 0
Jump found. (Code = 62) Position 1 = -2
filename:       /in/gvU56
function name:  (null)
number of ops:  4
compiled vars:  none
line     #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
   5     0  E >   INIT_FCALL                                               'strtolower'
         1        SEND_VAL                                                 'bar'
         2        DO_ICALL                                                 
         3      > RETURN                                                   1

[julienfalque]

Sounds like a good idea to me 👍

[keradus]

It was already discussed once with author of rule - @localheinz - as nice config option to have, but was postponed to undefined future. ref #2462

feel free to open a PR proposal.

[SignpostMarv]

Is there an existing rule that'd be a good starting point to look at to trigger the insertion of use statements ?

[keradus]

Instead of having this rule configurable, I would rather make new generic rule (that is executed after this, existing one) that would import all used classes from global namespaces (configurable to work with classes/functions/traits)

+use Foo\Bar;
 
-$x = new \Foo\Bar();
+$x = new Bar();

[keradus]

@SignpostMarv I can't remind any

[SignpostMarv]

import_absolute_references ?

[keradus]

sth like this, name could always be adjusted during review, if one would figure out better name (now, I cant ;) )

[SignpostMarv]

I was poking around the php-cs-fixer source last night, thinking "no_absolute_references" under PhpCsFixer\Fixer\Import might be a good fit ?

Also found something that may or may not be a bit of a problem - getPriority(); we'd want this to run after NativeFunctionInvocationFixer, but before NoUnusedImportsFixer & OrderedImportsFixer, might need to change things ?

[keradus]

as it's suppose to import things, import namespace is ok
priority, as long as you know what you want to achieve, won't be a problem

[SignpostMarv]

wouldn't changing the priority on a bunch of other fixers be a problem for others ?

[keradus]

priorities are to set relations between fixers, like fixer A has to be executed before fixer B.
as long as integration tests will be fine after your changes, everything is fine

[SignpostMarv]

poking around in the code a bit more today, so I can probably figure out how to add new use foo; statements, but I'm not sure how to identify the appropriate insertion point- I'm looking at what OrderedImportsFixer does with Tokens::fromCode() & $tokens->overrideRange()

presumably we want to insert tokens on the line after one of

• "<?" or "<?php" (first available)
• namespace Foo; (used instead of opening tag position)
• use Foo;

would appreciate help if I'm looking at the wrong example to figure things out from :D

[keradus]

I would suggest to do it step by step.
First step: create fixer that is not doing what you wanna achieve :D at least not fully.
GeneralImportAddFixer, that will receive configuration what to import and it will add imports for it.

So here, without detecting what to import. After it is done, you could reuse it in new fixer like NoFQCNWhenImportedFixer, that will detect what to import, ask first fixer to import it, and drop FQCN from usage.` But let us come back to this fixer after first part is ready and working.

I would ignore:

• multi-namespaced files (we already have method for it: Tokens->findKinds... and count it)
• non monolitic files (=multiple <?php) (we already have method for it as well: isMonolitic)
  like we already do it some fixers.

then like you said, search for last existing imports, if it's not there go for namespace, if it's not there go for opening tag and skip following declare/docblock

You have the right place to add things. Now you need to build tokens for things you want to import and Tokens->insertAt them (no need to override).
You could convert string use Foo\Bar; via fromCode, but I would rather suggest to build Token items manually (parsing is fast, and we do know what string we want to achieve so we could prepare the tokens manually)

Don't be shy to reach me over gitter if you have any questions !

[jaydiablo]

This would really be nice as an option to the native_function_invocation fixer (which I think is what has been proposed). Used with the scope option set to namespaced may be a requirement, as PHP doesn't like the use function X statements when the code isn't namespaced.

https://3v4l.org/oicuF

Warning: The use statement with non-compound name 'strtolower' has no effect in /in/oicuF on line 3

[SpacePossum]

done in #4355