From 0fb8b5fd52ef3c6cc423f27d1a4c95fc9c716bb9 Mon Sep 17 00:00:00 2001 From: bjorn Date: Tue, 16 Jan 2024 20:58:07 +0100 Subject: [PATCH 1/2] Fix double deprecated calls when already refactored 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). --- src/Rector/AbstractDrupalCoreRector.php | 31 +++++++++++++++++++ .../fixture/watchdog_exception.php.inc | 4 +++ 2 files changed, 35 insertions(+) diff --git a/src/Rector/AbstractDrupalCoreRector.php b/src/Rector/AbstractDrupalCoreRector.php index d44e0b7b..55bf1777 100644 --- a/src/Rector/AbstractDrupalCoreRector.php +++ b/src/Rector/AbstractDrupalCoreRector.php @@ -8,7 +8,9 @@ use DrupalRector\Contract\VersionedConfigurationInterface; use PhpParser\Node; use PhpParser\Node\Expr\ArrowFunction; +use PHPStan\Reflection\MethodReflection; use Rector\Contract\Rector\ConfigurableRectorInterface; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; abstract class AbstractDrupalCoreRector extends AbstractRector implements ConfigurableRectorInterface @@ -29,6 +31,31 @@ public function configure(array $configuration): void $this->configuration = $configuration; } + 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'; + } + public function refactor(Node $node) { $drupalVersion = str_replace(['.x-dev', '-dev'], '.0', \Drupal::VERSION); @@ -38,6 +65,10 @@ public function refactor(Node $node) continue; } + if ($this->isInBackwardsCompatibleCall($node)) { + continue; + } + $result = $this->refactorWithConfiguration($node, $configuration); // Skip if no result. diff --git a/tests/src/Drupal10/Rector/Deprecation/WatchdogExceptionRector/fixture/watchdog_exception.php.inc b/tests/src/Drupal10/Rector/Deprecation/WatchdogExceptionRector/fixture/watchdog_exception.php.inc index 8d40c958..b3219444 100644 --- a/tests/src/Drupal10/Rector/Deprecation/WatchdogExceptionRector/fixture/watchdog_exception.php.inc +++ b/tests/src/Drupal10/Rector/Deprecation/WatchdogExceptionRector/fixture/watchdog_exception.php.inc @@ -11,6 +11,8 @@ function advanced() { watchdog_exception('update', $exception, 'My custom message @foo', ['@foo' => 'bar'], RfcLogLevel::CRITICAL, 'http://example.com'); watchdog_exception('update', $exception, 'My custom message @foo', ['@foo' => 'bar']); + + \Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => \Drupal\Core\Utility\Error::logException(\Drupal::logger('update'), $exception), fn() => watchdog_exception('update', $exception)); } ?> ----- @@ -27,5 +29,7 @@ function advanced() { \Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => \Drupal\Core\Utility\Error::logException(\Drupal::logger('update'), $exception, 'My custom message @foo', ['@foo' => 'bar', 'link' => 'http://example.com'], RfcLogLevel::CRITICAL), fn() => watchdog_exception('update', $exception, 'My custom message @foo', ['@foo' => 'bar', 'link' => 'http://example.com'], RfcLogLevel::CRITICAL, 'http://example.com')); \Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => \Drupal\Core\Utility\Error::logException(\Drupal::logger('update'), $exception, 'My custom message @foo', ['@foo' => 'bar']), fn() => watchdog_exception('update', $exception, 'My custom message @foo', ['@foo' => 'bar'])); + + \Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => \Drupal\Core\Utility\Error::logException(\Drupal::logger('update'), $exception), fn() => watchdog_exception('update', $exception)); } ?> From f486107c9f201ff3c67567a0dc1e399c886b6757 Mon Sep 17 00:00:00 2001 From: bjorn Date: Tue, 16 Jan 2024 20:58:40 +0100 Subject: [PATCH 2/2] Codestyle fixes, sure. --- src/Drupal8/Rector/Deprecation/DBRector.php | 22 +++++++++---------- .../Rector/Deprecation/EntityLoadRector.php | 2 +- ...nctionalTestDefaultThemePropertyRector.php | 2 +- .../Deprecation/AssertLegacyTraitRector.php | 2 +- .../Deprecation/AssertNoFieldByNameRector.php | 2 +- .../UiHelperTraitDrupalPostFormRector.php | 2 +- .../ProtectedStaticModulesPropertyRector.php | 2 +- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Drupal8/Rector/Deprecation/DBRector.php b/src/Drupal8/Rector/Deprecation/DBRector.php index feb8aeb7..4963b5f7 100644 --- a/src/Drupal8/Rector/Deprecation/DBRector.php +++ b/src/Drupal8/Rector/Deprecation/DBRector.php @@ -98,37 +98,37 @@ public function refactor(Node $node): ?Node { assert($node instanceof Node\Stmt\Expression); - $isFuncCall = $node->expr instanceof Node\Expr\FuncCall; - $isMethodCall = $node->expr instanceof Node\Expr\MethodCall; - $isAssignedFuncCall = $node->expr instanceof Node\Expr\Assign && $node->expr->expr instanceof Node\Expr\FuncCall; + $isFuncCall = $node->expr instanceof Expr\FuncCall; + $isMethodCall = $node->expr instanceof MethodCall; + $isAssignedFuncCall = $node->expr instanceof Expr\Assign && $node->expr->expr instanceof Expr\FuncCall; if (!$isFuncCall && !$isAssignedFuncCall && !$isMethodCall) { return null; } foreach ($this->configuration as $configuration) { - if ($node->expr instanceof Node\Expr\FuncCall && $this->getName($node->expr->name) !== $configuration->getDeprecatedMethodName()) { + if ($node->expr instanceof Expr\FuncCall && $this->getName($node->expr->name) !== $configuration->getDeprecatedMethodName()) { continue; } - if ($node->expr instanceof Node\Expr\Assign && $node->expr->expr instanceof Node\Expr\FuncCall && $this->getName($node->expr->expr->name) !== $configuration->getDeprecatedMethodName()) { + if ($node->expr instanceof Expr\Assign && $node->expr->expr instanceof Expr\FuncCall && $this->getName($node->expr->expr->name) !== $configuration->getDeprecatedMethodName()) { continue; } - if ($node->expr instanceof Node\Expr\FuncCall) { + if ($node->expr instanceof Expr\FuncCall) { $methodCall = $this->getMethodCall($node->expr, $node, $configuration); $node->expr = $methodCall; return $node; } - if ($node->expr instanceof Node\Expr\Assign && $node->expr->expr instanceof Node\Expr\FuncCall) { + if ($node->expr instanceof Expr\Assign && $node->expr->expr instanceof Expr\FuncCall) { $methodCall = $this->getMethodCall($node->expr->expr, $node, $configuration); $node->expr->expr = $methodCall; return $node; } - if ($node->expr instanceof Node\Expr\MethodCall) { + if ($node->expr instanceof MethodCall) { $funcCall = $this->findRootFuncCallForMethodCall($node->expr); if ($funcCall === null || $this->getName($funcCall->name) !== $configuration->getDeprecatedMethodName()) { continue; @@ -187,7 +187,7 @@ public function replaceFuncCallForMethodCall(MethodCall $expr, MethodCall $metho return null; } - public function getMethodCall(Node\Expr\FuncCall $expr, Node\Stmt\Expression $statement, DBConfiguration $configuration): Node\Expr\MethodCall + public function getMethodCall(Expr\FuncCall $expr, Node\Stmt\Expression $statement, DBConfiguration $configuration): MethodCall { // TODO: Check if we have are in a class and inject \Drupal\Core\Database\Connection // TODO: Check if we have are in a class and don't have access to the container, use `\Drupal\core\Database\Database::getConnection()`. @@ -229,11 +229,11 @@ public function getMethodCall(Node\Expr\FuncCall $expr, Node\Stmt\Expression $st $this->commentService->addDrupalRectorComment($statement, 'You will need to use `\Drupal\core\Database\Database::getConnection()` if you do not yet have access to the container here.'); } - $var = new Node\Expr\StaticCall($name, $call, $method_arguments); + $var = new Expr\StaticCall($name, $call, $method_arguments); $method_name = new Node\Identifier(substr($configuration->getDeprecatedMethodName(), 3)); - $methodCall = new Node\Expr\MethodCall($var, $method_name, $expr->args); + $methodCall = new MethodCall($var, $method_name, $expr->args); return $methodCall; } diff --git a/src/Drupal8/Rector/Deprecation/EntityLoadRector.php b/src/Drupal8/Rector/Deprecation/EntityLoadRector.php index 66e9bcc2..7c3bb7fa 100644 --- a/src/Drupal8/Rector/Deprecation/EntityLoadRector.php +++ b/src/Drupal8/Rector/Deprecation/EntityLoadRector.php @@ -172,7 +172,7 @@ public function refactor(Node $node): ?Node if (!class_exists('\PhpParser\Node\ArrayItem')) { $arrayItems = [new Node\Expr\ArrayItem($entity_id->value)]; } else { - $arrayItems = [new \PhpParser\Node\ArrayItem($entity_id->value)]; + $arrayItems = [new Node\ArrayItem($entity_id->value)]; } $reset_args = [ diff --git a/src/Drupal8/Rector/Deprecation/FunctionalTestDefaultThemePropertyRector.php b/src/Drupal8/Rector/Deprecation/FunctionalTestDefaultThemePropertyRector.php index 68316c57..3cf939eb 100644 --- a/src/Drupal8/Rector/Deprecation/FunctionalTestDefaultThemePropertyRector.php +++ b/src/Drupal8/Rector/Deprecation/FunctionalTestDefaultThemePropertyRector.php @@ -67,7 +67,7 @@ public function getNodeTypes(): array } /** - * @param \PhpParser\Node\Stmt\Class_ $node + * @param Node\Stmt\Class_ $node */ public function refactorWithScope(Node $node, Scope $scope): ?Node { diff --git a/src/Drupal9/Rector/Deprecation/AssertLegacyTraitRector.php b/src/Drupal9/Rector/Deprecation/AssertLegacyTraitRector.php index 95e3a374..ad86c765 100644 --- a/src/Drupal9/Rector/Deprecation/AssertLegacyTraitRector.php +++ b/src/Drupal9/Rector/Deprecation/AssertLegacyTraitRector.php @@ -56,7 +56,7 @@ public function getNodeTypes(): array * @param string $method * @param array $args * - * @return \PhpParser\Node\Expr\MethodCall + * @return Node\Expr\MethodCall */ protected function createAssertSessionMethodCall(string $method, array $args): Node\Expr\MethodCall { diff --git a/src/Drupal9/Rector/Deprecation/AssertNoFieldByNameRector.php b/src/Drupal9/Rector/Deprecation/AssertNoFieldByNameRector.php index 72fc873a..5ffd8f69 100644 --- a/src/Drupal9/Rector/Deprecation/AssertNoFieldByNameRector.php +++ b/src/Drupal9/Rector/Deprecation/AssertNoFieldByNameRector.php @@ -100,7 +100,7 @@ public function refactor(Node $node): ?Node * @param string $method * @param array $args * - * @return \PhpParser\Node\Expr\MethodCall + * @return Node\Expr\MethodCall */ protected function createAssertSessionMethodCall(string $method, array $args): Node\Expr\MethodCall { diff --git a/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php b/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php index eb2a9421..208f4ebf 100644 --- a/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php +++ b/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php @@ -44,7 +44,7 @@ public function getNodeTypes(): array } /** - * @param \PhpParser\Node\Expr\MethodCall $node + * @param Node\Expr\MethodCall $node * * @throws ShouldNotHappenException * diff --git a/src/Drupal9/Rector/Property/ProtectedStaticModulesPropertyRector.php b/src/Drupal9/Rector/Property/ProtectedStaticModulesPropertyRector.php index b7a6ae09..7206b85f 100644 --- a/src/Drupal9/Rector/Property/ProtectedStaticModulesPropertyRector.php +++ b/src/Drupal9/Rector/Property/ProtectedStaticModulesPropertyRector.php @@ -51,7 +51,7 @@ public function getNodeTypes(): array } /** - * @param \PhpParser\Node\Stmt\Property $node + * @param Node\Stmt\Property $node */ public function refactor(Node $node): ?Node {