-
-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add findInstancesOfScoped() to BetterNodeFinder, to ease re-use #6323
Conversation
88b0301
to
e70613f
Compare
if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { | ||
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; | ||
} | ||
|
||
foreach ($types as $type) { | ||
if ($subNode instanceof $type) { | ||
$isFoundNode = true; | ||
return NodeTraverser::STOP_TRAVERSAL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stop traversal is essential here to not looking more if found, probably make another method with $stopOnFound
flag so we have same performance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, there is already findInstancesOfInFunctionLikeScoped
, if there is a $nodes
needed, probably better to allow array there, with make another private method for re-use as both method:
public function findInstancesOfInFunctionLikeScoped( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion is, make it allow FunctionLike
in array of nodes[0]:
- Keep
hasInstancesOfInFunctionLikeScoped
as is for performance reason - Modify the method to allow single
$nodes
, asFunctionLike
reuse
/**
* @api to be used
*
* @template T of Node
* @param Node[] $nodes
* @param class-string<T>|array<class-string<T>> $type
* @return T[]
*/
public function findInstancesOfScoped(array $nodes, string|array $type): array
{
// here verify only pass single nodes as FunctionLike
if (count($nodes) === 1 && ($nodes[0] instanceof ClassMethod || $nodes[0] instanceof Function_ || $nodes[0] instanceof Closure)) {
$nodes = (array) $nodes[0]->stmts;
}
if (is_string($types)) {
$types = [$types];
}
/** @var T[] $foundNodes */
$foundNodes = [];
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
$nodes,
static function (Node $subNode) use ($types, &$foundNodes): ?int {
if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) {
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}
foreach ($types as $type) {
if ($subNode instanceof $type) {
$foundNodes[] = $subNode;
return null;
}
}
return null;
}
);
return $foundNodes;
- Then, in
findInstancesOfInFunctionLikeScoped
, change to just call it:
public function findInstancesOfInFunctionLikeScoped(
ClassMethod | Function_ | Closure $functionLike,
string|array $types
): array {
return $this->findInstancesOfScoped([$functionLike], $types);
}
I will create an alternative PR for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍
One of methods should return "all", the other only "first" for performance reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the alternative PR:
No description provided.