Detect unused function-call on a separate line with possibly pure function #3020
Conversation
| public function processNode(Node $node, Scope $scope) | ||
| { | ||
| $function = $node->getFunctionReflection(); | ||
| if (!$function->isPure()->maybe()) { |
There was a problem hiding this comment.
We should also eliminate void function here.
There was a problem hiding this comment.
Thinking about it, we should simply ask about hasSideEffects().
| return null; | ||
| } | ||
|
|
||
| $functionReflection = $this->reflectionProvider->getFunction($node->name, $scope); |
There was a problem hiding this comment.
We should also have this condition:
if (!$function->hasSideEffects()->maybe() {
return null;
}Because:
- If it's
no()it's already covered by CallToFunctionStatementWithoutSideEffectsRule - if it's
yes()then it's a valid call and should not be reported in this rule.
The distinction between hasSideEffects() and isPure essentially boils down to function being void and function being marked as @phpstan-pure and @phpstan-impure.
Here we're dealing with the uncertainty so we need both of these to be maybe().
|
This pull request has been marked as ready for review. |
|
Awesome, thank you! |
|
Cool. Will do method call variants tomorrow |
|
Needed to fix this: 57b0ae0 If we looked at issue-bot it would tell us, but no worries :) |
|
This is a false positive: https://phpstan.org/r/eac46a37-248f-41a1-88c8-510241e06b6b Because it works: https://3v4l.org/5XkR6 Please fix it before moving forward, thanks :) |
|
Oh god, we don't have to care about that. It's some kind of weird array loophole. This isn't supported on PHP since 5.4. |
|
To make sure we are on the same page: there is no more open todo and I can start with methods node-type? |
|
Yes 😊 |
similar to 281a87d