[Php73] Rector\Php\Rector\FuncCall\StringifyStrNeedlesRector: does the job twice

[gnutix]

Here's a strange result from rector using level "php73.yaml". I've run rector once, it found this fix (adding (string) before $search->getQ() in the strpos call), I applied it, and now when running Rector again it wants to do it all over again.

1) src/FileLogFinder.php

    ---------- begin diff ----------
--- Original
+++ New
@@ -48,7 +48,7 @@
         if (!empty($search->getQ())) {
             $logs = array_filter($logs, function (FileLog $log) use ($search) {
                 $res = false;
-                if (false !== strpos($log->getText(), (string) $search->getQ())) {
+                if (false !== strpos($log->getText(), (string) (string) $search->getQ())) {
                     $res = true;
                 }
    ----------- end diff -----------

Applied rectors:

 * Rector\Php\Rector\FuncCall\StringifyStrNeedlesRector

[larsroettig]

I can verify this Issue I can also reproduce I try to fix it now we need this Rector for a Project Upgrade

[TomasVotruba]

Thanks for reporting 👍

[gnutix]

It's better (doesn't add it twice), but it still does add (string) to an argument even if it's of type string. Example :

# src/test.php
final class Test {
    public function getNeedle(): string {
        return 'needle';
    }
}

strpos('needle', (new Test)->getNeedle());

gives :

1) src/test.php

    ---------- begin diff ----------
--- Original
+++ New
@@ -5,4 +5,4 @@
     }
 }

-strpos('needle', (new Test)->getNeedle());
+strpos('needle', (string) (new Test)->getNeedle());
    ----------- end diff -----------

Applied rectors:

 * Rector\Php\Rector\FuncCall\StringifyStrNeedlesRector

And then, PHPStorm highlights the (string) because it's useless, as getNeedle returns a string anyway.

Should I create a new issue ?

[TomasVotruba]

Better add a test case with this

[gnutix]

Better add a test case with this

I'm not sure how to do that. Could you please reopen the issue so that someone (if not me later) might pick it up?

[TomasVotruba]

Adding test case is simple and it speeds up the fix, look here: https://github.com/rectorphp/rector/pull/1642/files

[gnutix]

@TomasVotruba Took some time to land, but here is a failing test : #2081

[gnutix]

Thanks! Confirm it works on our codebase. :)