InnerFunctionToPrivateMethodRector produces broken code when the inner function is used as a string callable

[arjannijsink]

Bug Report

Subject	Details
Rector version	2.5.4
Responsible rules	InnerFunctionToPrivateMethodRector, aggravated by RemoveUnusedPrivateMethodRector
PHP version	8.x

Minimal PHP Code Causing Issue

<?php

class Demo
{
    public static function getOptions()
    {
        function sortByOrder(array $a, array $b): int
        {
            return strcmp((string) $a['label'], (string) $b['label']);
        }

        $options = [
            ['value' => 'b', 'label' => 'Bravo'],
            ['value' => 'a', 'label' => 'Alpha'],
        ];

        usort($options, 'sortByOrder');

        return $options;
    }
}

Config

<?php

use Rector\CodeQuality\Rector\Class_\InnerFunctionToPrivateMethodRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector;
use Rector\Config\RectorConfig;

return RectorConfig::configure()
    ->withPaths([__DIR__ . '/src'])
    ->withRules([
        InnerFunctionToPrivateMethodRector::class,
        RemoveUnusedPrivateMethodRector::class,
    ]);

Actual output (both rules — function is deleted entirely, call site untouched)

 {
     public static function getOptions()
     {
-        function sortByOrder(array $a, array $b): int
-        {
-            return strcmp((string) $a['label'], (string) $b['label']);
-        }
-
         $options = [

With only InnerFunctionToPrivateMethodRector enabled, the private method is created, but the
string callable is not rewritten, so the result is still broken:

         usort($options, 'sortByOrder');

         return $options;
+    }
+    private static function sortByOrder(array $a, array $b): int
+    {
+        return strcmp((string) $a['label'], (string) $b['label']);
     }
 }

https://getrector.com/demo/61afb5ea-efc3-4014-96d6-2de5dbb5b774

Expected Behaviour

Either:

1. InnerFunctionToPrivateMethodRector rewrites string-callable usages to reference the new private
   method (e.g. usort($options, self::sortByOrder(...)) or [self::class, 'sortByOrder']), or
2. the rule skips the transformation when the inner function is referenced by name as a string
   callable, since the rename cannot be applied safely.

Rewriting to a real reference (option 1) would also stop RemoveUnusedPrivateMethodRector from
considering the new method dead code.

[TomasVotruba]

Thanks for detailed reporting, let's skip it for now 👍

This one maybe woudl be better to inline (if used once) or passed a named function. How would you change this method manually yourself?

[arjannijsink]

If it was used in more places I'd keep the private method but call it as self::sortByOrder(...). Maybe that's what the rule could emit when the function name is used as a string callable? Then usort still works and the dead-code rulesees a real usage. Or just skip those cases.