Skip to content

Conversation

ondrejmirtes
Copy link
Member

No description provided.

@ondrejmirtes ondrejmirtes force-pushed the node-callback-invoker branch from 0a70f9d to c3e50ec Compare October 12, 2025 14:29
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Properties\PropertyReflectionFinder;

final class DirectInternalScopeFactoryFactory implements InternalScopeFactoryFactory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting java vibes.. where is the FactoryFactoryFactory :-)

}

public function processNode(Node $node, Scope $scope): array
public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a testing-only change which we need to be cleaned up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we're calling another rule, which might need it. If you revert it, you get PHPStan error.

return Node\Stmt\Echo_::class;
}

public function processNode(Node $node, NodeCallbackInvoker&Scope $scope): array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function processNode(Node $node, NodeCallbackInvoker&Scope $scope): array
public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed the downgrader to also support the intersection type when notated in different order:
ondrejmirtes/simple-downgrader#17

Comment on lines +7 to +28
/**
* The interface NodeCallbackInvoker can be typehinted in 2nd parameter of Rule::processNode():
*
* ```php
* public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
* ```
*
* It can be used to invoke rules for virtual made-up nodes.
*
* For example: You're writing a rule for a method with declaration like:
*
* ```php
* public static create(string $class, mixed ...$args)
* ```
*
* And you'd like to check `Factory::create(Foo::class, 1, 2, 3)` as if it were
* `new Foo(1, 2, 3)`.
*
* You can call `$scope->invokeNodeCallback(new New_(new Name($className), $args))`
*
* And PHPStan will call all the registered rules for New_, checking as if the instantiation
* is actually in the code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow, this is awesome! 🚀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I vaguely remember you're using FunctionCallParametersCheck somewhere, which is not part of BC promise. This would allow you to write simple and clean code instead 😊

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this PHPDoc is lying a little bit. You don't need to change your processNode signature to take advantage of this. phpstan-src uses native intersection type which is downgraded to PHPDoc because of PHP 7.4+ support. Which means that unless you have your own PHPDoc in your custom rule, PHPStan will implicitly inherit the Rule interface PHPDoc and you can call $scope->invokeNodeCallback without changing the signature :)

@ondrejmirtes ondrejmirtes force-pushed the node-callback-invoker branch from c3e50ec to 51f9c39 Compare October 13, 2025 10:45
@ondrejmirtes ondrejmirtes merged commit c8a1ae1 into 2.1.x Oct 13, 2025
538 of 549 checks passed
@ondrejmirtes ondrejmirtes deleted the node-callback-invoker branch October 13, 2025 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants