Skip to content
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

Rule visits nullsafe method call twice (once with MethodCall, once with NullsafeMethodCall) #9830

Closed
janedbal opened this issue Aug 31, 2023 · 5 comments

Comments

@janedbal
Copy link
Contributor

Bug report

When a rule is hooked on parent of those nodes, e.g. like this:

public function getNodeType(): string
{
    return Node::class; // or CallLike or any other common parent
}

and analyses this simple code

<?php
$var?->call();

The rule is called twice:

  1. Once with MethodCall node from here
  2. Once with NullsafeMethodCall node from here

Code snippet that reproduces the problem

https://phpstan.org/r/2aa36241-7795-4561-bf14-5c8d641f89fb

Expected output

Visit node only once.

Did PHPStan help you today? Did it make you happy in any way?

No response

@ondrejmirtes
Copy link
Member

This is expected behaviour. If you hook your rule on NullsafeMethodCall, it's gonna get called. If you hook your rule on MethodCall, it's also gonna get called (I did this so that existing rules also check NullsafeMethodCall nodes which PHPStan greatly benefits from).

This means that if you hook your rule on a common ancestor like CallLike or Node, it should also get called.

You also get multiple virtual nodes for every method declaration etc.

You can fix your problem if you filter out all NullsafeMethodCall invocations because you're not interested in them.

Alternatively, we could provide some attribute like wasNullsafe to the "virtual" MethodCall.

@janedbal
Copy link
Contributor Author

janedbal commented Sep 1, 2023

Alternatively, we could provide some attribute like wasNullsafe to the "virtual" MethodCall.

I like that.

Because currently, I was unable to distinguish (while ignoring NullsafeMethodCall to avoid this issue) if it was nullsafe or not. Here is the case (currently, I'm unable to show ?-> in the getCallToken method.

@janedbal
Copy link
Contributor Author

janedbal commented Sep 4, 2023

I think we can close this as this has a solution now, WDYT?

@ondrejmirtes
Copy link
Member

Yes :)

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants