[CodingStyle] Add new rector to replace hardcoded class name reference in string with class keyword reference

[dobryy]

It seems like SplitStringClassConstantToClassConstFetchRector might be replaced with this new rector.

Closes #3389

[dobryy]

@TomasVotruba, I wanted to use something like this

+        $concat = new Concat($classConstFetch, new String_('::' . $after));
+        if (! empty($before)) {
+            $concat = new Concat(new String_($before), $concat);
+        }
-        $concat = new Concat($classConstFetch, new String_('::' . $after));
-        if (! empty($before)) {
-            $concat = new Concat(new String_($before), $classConstFetch);
-            $concat = new Concat($concat, new String_('::' . $after));
-        }

but for some reason it generates unexpected (to me :)) string

-'<?php if (' . (SomeClass::class . '::createCache($netteCacheStorage, %var,  $this->global->cacheStack, %node.array?)) { ?>')
+'<?php if (' . SomeClass::class . '::createCache($netteCacheStorage, %var,  $this->global->cacheStack, %node.array?)) { ?>'

[TomasVotruba]

Could you write it again here in a diff what you want and what's changed? It would be easier for me to read and understand.

-$value . 'String'
+$value . 'StringCorrect'

The shorter, the better (withotu all the long classssss names)

[dobryy]

The comment was updated with diffs

[TomasVotruba]

I still can't see the problem (too much nette irelevant code).
If it's extra ( . ) than it's irelevant, as concat basically joins it all.

[dobryy]

Maximum simplification:

-'<?php if (' . (SomeClass::class . '::method()) { ?>')
+'<?php if (' . SomeClass::class . '::method()) { ?>'

Doesn't look like it just joins...

[TomasVotruba]

Does reordering of Concat node helps? I think they're working the other way around.

[dobryy]

If I concat in "proper" order everything works well.

[dobryy]

Basically... this code works well but doesn't look that optimized:

        $concat = new Concat($classConstFetch, new String_('::' . $after));
        if (! empty($before)) {
            $concat = new Concat(new String_($before), $classConstFetch);
            $concat = new Concat($concat, new String_('::' . $after));
        }

This code looks more optimized and from the first view should work fine but it doesn't

        $concat = new Concat($classConstFetch, new String_('::' . $after));
        if (! empty($before)) {
            $concat = new Concat(new String_($before), $concat);
        }

I'm just wondering why it doesn't and I don't mind to leave it as it is now :)

[TomasVotruba]

Not sure what $before is

[dobryy]

$before is everything before the class name. See the regex https://regex101.com/r/Vv41Qr/1
In some cases, it might be empty and I don't think we would like to generate empty string before the class name.

-'' . ClassName . '::method()'
+ClassName . '::method()'

[TomasVotruba]

I see. Please add one test fixture for that too.

[dobryy]

Ready

[TomasVotruba]

I've added first review. Technically it looks good 👍

[TomasVotruba]

Looks good! 👍

Please rebase on master, squash to 1 commit and it's ready to merge

[TomasVotruba]

Thanks!

[TomasVotruba]

I just realized we should handle also this case:

-$value = 'App\SomeClass::someMethod() && App\SomeClass::someMethod()';
+$value = \App\SomeClass::class . 'someMethod() && ' . \App\SomeClass::class . 'someMethod()';

Could you cover it too in new PR?

[dobryy]

I just realized we should handle also this case:

-$value = 'App\SomeClass::someMethod() && App\SomeClass::someMethod()';
+$value = \App\SomeClass::class . 'someMethod() && ' . \App\SomeClass::class . 'someMethod()';

Could you cover it too in new PR?

@TomasVotruba, ready - #3648