Fix: double deprecated calls when already refactored #288
Conversation
This will fix the double deprecated calls. In the future we might need to refactor this, since something might actually need a nested deprecated call. That would be kinda nasty, but is an edge case that could happen (tm).
| protected function isInBackwardsCompatibleCall(Node $node): bool | ||
| { | ||
| if (!class_exists(DeprecationHelper::class)) { | ||
| return false; | ||
| } | ||
|
|
||
| $scope = $node->getAttribute(AttributeKey::SCOPE); | ||
|
|
||
| $callStack = $scope->getFunctionCallStackWithParameters(); | ||
| if (count($callStack) === 0) { | ||
| return false; | ||
| } | ||
| [$function, $parameter] = $callStack[0]; | ||
| if (!$function instanceof MethodReflection) { | ||
| return false; | ||
| } | ||
| if ($function->getName() !== 'backwardsCompatibleCall' | ||
| || $function->getDeclaringClass()->getName() !== DeprecationHelper::class | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| return $parameter !== null && $parameter->getName() === 'deprecatedCallable'; | ||
| } |
There was a problem hiding this comment.
Does Rector have a node visitor to just stop processing so that it doesn't handle nodes in this execution tree?
There was a problem hiding this comment.
Think so yes. My old fix for phpstan would probably also work. This seemed easier is 0retty much why I used this.
There was a problem hiding this comment.
I guess we're calling it in our base class, which all rules should extend. This is fine, but could lead to a "bug" if a rule didn't. But probably a minimal problem
There was a problem hiding this comment.
valid point. But if it doesn't extend this class, it should not add the deprecation helper. But you are right though, it could slip through if we need some sort of exotic way to do things. Still would probably use the base class for the helpers.
Description
This will fix the double deprecated calls. In the future we might need to refactor this, since something might actually need a nested deprecated call. That would be kinda nasty, but is an edge case that could happen (tm).
Codestyle is a bit noisy, annoying, but ah well.
To Test
Run test.
Drupal.org issue
https://www.drupal.org/project/rector/issues/3414169