Add `ClosureFromCallableToFirstClassCallableRector` rule #7071

devnix · 2025-07-17T23:48:48Z

rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php

samsonasik · 2025-07-18T09:07:57Z

config/set/php81.php

@@ -23,6 +24,7 @@
        SpatieEnumClassToEnumRector::class,
        SpatieEnumMethodCallToEnumConstRector::class,
        NullToStrictStringFuncCallArgRector::class,
+        ClosureFromCallableToFirstClassCallableRector::class,


I prefer to use CodingStyle namespace for this, and no need to register to config set, as it mostly by preference, use of first class callable mostly cause error on unit tests and we skip that over time on projects

@samsonasik

I prefer to use CodingStyle namespace for this, and no need to register to config set

No problem! It should be a pretty safe refactor for PHP 8.1. There are a lot of upgrade rules that I could consider too as more opinionated towards coding style, so I'm a little bit lost here. To me, Rector promotes you to use new language features by applying them to your entire codebase.

use of first class callable mostly cause error on unit tests and we skip that over time on projects

How does that happen? As far as I understand, both are closures, both are bindable, and I can't think of a case when this rule could break something:

If you are aware of a case when this could be a problem, could you share a snippet so I can try to be sure that it doesn't do anything unexpected on specific Closure::fromCallable usages?

The unit test may compare its arguments over its value, [DateTime', 'creatFromInterface] which is an array, vs ... which an variadic place holder.

.../FuncCall/ClosureFromCallableToFirstClassCallableRector/Fixture/class_callable_array.php.inc

devnix · 2025-07-20T12:01:20Z

I've not removed the rule from the 8.1 ruleset as I'm not sure about in which level of CodingStyle it should be. To me, it's a very safe rule 🙂

Where would you place it exactly?

rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php

samsonasik · 2025-07-21T03:12:47Z

rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php

+
+        if ($arg->value instanceof Node\Scalar\String_) {
+            return new Node\Expr\FuncCall(
+                new Node\Name($this->prefixRootNamespace($arg->value->value)),


FullyQualified node can be used instead of manual prefix check

samsonasik · 2025-07-21T08:34:53Z

.../FuncCall/ClosureFromCallableToFirstClassCallableRector/Fixture/class_callable_array.php.inc

@@ -0,0 +1,23 @@
+<?php
+
+use SomeNamespace\Foo;


Suggested change

use SomeNamespace\Foo;

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

How does this help testing relative namespaces?

Unless required by rule, fixture namespace should psr4 per defined in composer.json

samsonasik · 2025-07-21T08:35:02Z

.../FuncCall/ClosureFromCallableToFirstClassCallableRector/Fixture/class_callable_array.php.inc

+-----
+<?php
+
+use SomeNamespace\Foo;


Suggested change

use SomeNamespace\Foo;

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

samsonasik · 2025-07-21T08:35:09Z

...cCall/ClosureFromCallableToFirstClassCallableRector/Fixture/function_callable_string.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Foo;


Suggested change

namespace Foo;

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

samsonasik · 2025-07-21T08:35:14Z

...cCall/ClosureFromCallableToFirstClassCallableRector/Fixture/function_callable_string.php.inc

+-----
+<?php
+
+namespace Foo;


Suggested change

namespace Foo;

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

samsonasik · 2025-07-21T08:38:45Z

...cCall/ClosureFromCallableToFirstClassCallableRector/Fixture/function_callable_string.php.inc

+
+namespace Foo;
+
+\trim(...);


this is fine without \ prefix, you should can use combination of:

$this->getName($funcCall) to verify only short name.

use of :

$funcReflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node);

and verify if that instanceof NativeFunctionReflection

if (! $funcReflection instanceof \PHPStan\Reflection\Native\NativeFunctionReflection\NativeFunctionReflection) { }

Please also add test needs check when function defined inside the namespace:

namespace Foo; function bar() { } Closure::fromCallable('bar');

which should be changed without prefix:

namespace Foo; function bar() { } -Closure::fromCallable('bar'); +bar(...);

This example is not valid: https://3v4l.org/NTdvJ#vnull
AFAIK, strings referring to functions can't be relative and must be FQDN: https://3v4l.org/DFr8s#vnull

I see, I think the native trim is still valid without \\

Regarding to this comment:

this is fine without \ prefix, you should can use combination of:

$this->getName($funcCall) to verify only short name.

use of :

$funcReflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node);

and verify if that instanceof NativeFunctionReflection

if (! $funcReflection instanceof \PHPStan\Reflection\Native\NativeFunctionReflection\NativeFunctionReflection) { }

I've grinded my gears and I don't find a way to get what you want. I guess that the ' $node' I should pass to that resolver is the FuncCall that I'm creating and returning.

In that case, this example

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture { function trim() {} \Closure::fromCallable('Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture\trim'); \Closure::fromCallable('trim'); }

becomes this one:

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture { function trim() {} \Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture\trim(...); trim(...); }

while it should be

namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture { function trim() {} \Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture\trim(...); \trim(...); }

The new node I'm creating, when evaluated by that resolver, is not aware that it will be inside a namespace.

I'm personally not bothered by having unnecessary \ in some cases, and I know about a lot of cases that create nodes with an unnecessary FQDN prefixed with the root \. That's why I rely on a formatting tool.

Maybe it's something doable and I'm just not seeing it. If there's a way, I would love to learn about it 🙏🏻

devnix · 2025-07-22T11:44:49Z

I've taken a look at all your comments @samsonasik, please let me know if I can improve it in any other way. Thanks for having a look! <3

samsonasik · 2025-07-22T12:05:19Z

Ok, just unregister from config set, I still don't want it to exists by php set for now, user can test try in the wild :)

devnix · 2025-07-22T14:08:03Z

I will move it to the CodingStyle namespace then, since there's a rule that forces me to register the service if it lives in the PHP 8.1 namespace:

 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php:17
 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  - '#Register "Rector\\Php81\\Rector\\FuncCall\\ClosureFromCallableToFirstClassCallableRector" service to "php81\.php" config set#'
  🪪 rector.upgradeDowngradeRegisteredInSet
 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

samsonasik · 2025-07-22T14:32:14Z

Yep, coding style is better imo :)

devnix · 2025-07-22T14:39:48Z

I hoped it was a problem only on my machine, but it replicates on the CI too 😰

In the PHP 8.1 namespace the tests were green, and in the process of moving it to the new namespace, the tests are failing like the rule is not being applied.

I can't xdebug it launching the whole test suite. Funny enough, if I run the ClosureFromCallableToFirstClassCallableRectorTest only, the fixtures are correctly transformed and the test is green.

samsonasik · 2025-07-22T14:51:06Z

That usually happen when there is typo or invalid character case due to file name vs class name vs folder name due to copy paste and replace

samsonasik · 2025-07-22T14:54:33Z

...yle/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector/config/configured_rule.php

+use Rector\Config\RectorConfig;
+
+return RectorConfig::configure()
+    ->withRules([ClosureFromCallableToFirstClassCallableRector::class]);


You seems need to add ->withPhpVersion() here

ooooof, I've just literally found it right now! thank you anyways 😄

samsonasik · 2025-07-22T15:00:41Z

Okay, let's give it a try, thank you @devnix

devnix marked this pull request as draft July 17, 2025 23:49

devnix mentioned this pull request Jul 17, 2025

Is there an upgrade rule from Closure::fromCallable() to first class callable syntax? rectorphp/rector#9266

Closed

TomasVotruba reviewed Jul 18, 2025

View reviewed changes

rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php Outdated Show resolved Hide resolved

devnix force-pushed the ClosureFromCallableToFirstClassCallableRector branch from ee6d875 to 0a4cbea Compare July 18, 2025 07:48

samsonasik reviewed Jul 18, 2025

View reviewed changes

.../FuncCall/ClosureFromCallableToFirstClassCallableRector/Fixture/class_callable_array.php.inc Outdated Show resolved Hide resolved

devnix marked this pull request as ready for review July 20, 2025 12:01

devnix requested review from samsonasik and TomasVotruba July 20, 2025 12:01

samsonasik reviewed Jul 20, 2025

View reviewed changes

rules/Php81/Rector/FuncCall/ClosureFromCallableToFirstClassCallableRector.php Outdated Show resolved Hide resolved

devnix changed the title ~~Created ClosureFromCallableToFirstClassCallableRector rule, fixture…~~ Add ClosureFromCallableToFirstClassCallableRector rule Jul 20, 2025

samsonasik reviewed Jul 21, 2025

View reviewed changes

devnix force-pushed the ClosureFromCallableToFirstClassCallableRector branch from b49d20a to ae9a5de Compare July 22, 2025 14:34

samsonasik reviewed Jul 22, 2025

View reviewed changes

Add ClosureFromCallableToFirstClassCallableRector rule

8511042

devnix force-pushed the ClosureFromCallableToFirstClassCallableRector branch from ae9a5de to 8511042 Compare July 22, 2025 14:56

samsonasik approved these changes Jul 22, 2025

View reviewed changes

samsonasik merged commit eb07bff into rectorphp:main Jul 22, 2025
47 checks passed

	use SomeNamespace\Foo;
	namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

	namespace Foo;
	namespace Rector\Tests\Php81\Rector\FuncCall\ClosureFromCallableToFirstClassCallableRector\Fixture;

Uh oh!

Add ClosureFromCallableToFirstClassCallableRector rule #7071

Add ClosureFromCallableToFirstClassCallableRector rule #7071

Uh oh!

Conversation

devnix commented Jul 17, 2025 • edited by samsonasik Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devnix commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnix commented Jul 22, 2025

Uh oh!

samsonasik commented Jul 22, 2025

Uh oh!

devnix commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsonasik commented Jul 22, 2025

Uh oh!

devnix commented Jul 22, 2025

Uh oh!

samsonasik commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samsonasik commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

Add `ClosureFromCallableToFirstClassCallableRector` rule #7071

Add `ClosureFromCallableToFirstClassCallableRector` rule #7071

devnix commented Jul 17, 2025 •

edited by samsonasik

Loading

devnix commented Jul 20, 2025 •

edited

Loading

devnix commented Jul 22, 2025 •

edited

Loading