From b9c636cc9005d0a0d0d4d9daf743cd928ee37d91 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Thu, 17 Oct 2024 16:43:17 -0400 Subject: [PATCH 01/64] Add HookConvert --- src/Rector/Convert/HookConvert.php | 295 +++++++++++++++++++++++++++++ 1 file changed, 295 insertions(+) create mode 100644 src/Rector/Convert/HookConvert.php diff --git a/src/Rector/Convert/HookConvert.php b/src/Rector/Convert/HookConvert.php new file mode 100644 index 00000000..6e877369 --- /dev/null +++ b/src/Rector/Convert/HookConvert.php @@ -0,0 +1,295 @@ +> + */ + public function getNodeTypes(): array + { + return [Function_::class, Node\Stmt\Use_::class]; + } + + public function refactor(Node $node): ?Node + { + $filePath = $this->file->getFilePath(); + $ext = pathinfo($filePath, \PATHINFO_EXTENSION); + if (!in_array($ext, ['inc', 'module'])) + { + return $node; + } + if ($filePath !== $this->inputFilename) + { + $this->initializeHookClass(); + } + if ($node instanceof Node\Stmt\Use_) { + $this->useStmts[] = $node; + } + + if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) + { + $this->hookClass->stmts[] = $method; + // See the note in ::getMethod() about how it's important to not + // change any object property of $node here. + // Rewrite the function body to be a single service call. + $node->stmts = [$this->getServiceCall($node)]; + // Mark this function as a legacy hook. + $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\LegacyHook'))]); + } + return $node; + } + + + protected function initializeHookClass(): void + { + $this->__destruct(); + $this->moduleDir = $this->file->getFilePath(); + $this->inputFilename = $this->moduleDir; + // Find the relevant info.yml: it's either in the current directory or + // one of the parents. + while (($this->moduleDir = dirname($this->moduleDir)) && !($info = glob("$this->moduleDir/*.info.yml"))); + if ($infoFile = reset($info)) { + $this->module = basename($infoFile, '.info.yml'); + $hookClassName = ucfirst(CaseStringHelper::camelCase($this->module . '_hooks')); + $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); + $this->hookClass = new Node\Stmt\Class_(new Node\Name($hookClassName)); + // Using $this->nodeFactory->createStaticCall() results in + // use \Drupal; on top which is not desirable. + $classConst = new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified("$namespace\\$hookClassName"), 'CLASS'); + $this->drupalServiceCall = new Node\Expr\StaticCall(new Node\Name\FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); + } + } + + public function __destruct() + { + if ($this->module && $this->hookClass->stmts) + { + $className = $this->hookClass->name->toString(); + $namespace = "Drupal\\$this->module\\Hook"; + // Create the statement use Drupal\Core\Hook\Hook; + $name = new Node\Name('Drupal\Core\Hook\Hook'); + // UseItem contains an unguarded class_alias to the deprecated + // UseUse class. However, due to some version mix-up it seems the + // old class is used for parsing so only use the UseItem class if + // it is already loaded or the UseUse class is completely gone. + $useHook = class_exists('PhpParser\Node\UseItem', FALSE) || !class_exists('PhpParser\Node\Stmt\UseUse') ? new Node\UseItem($name) : new Node\Stmt\UseUse($name); + // Put the class together. + $hookClassStmts = [ + new Node\Stmt\Namespace_(new Node\Name($namespace)), + ... $this->useStmts, + new Node\Stmt\Use_([$useHook]), + $this->hookClass, + ]; + // Write it out. + @mkdir("$this->moduleDir/src"); + @mkdir("$this->moduleDir/src/Hook"); + $hookClassFilename = "$this->moduleDir/src/Hook/$className.php"; + $printer = new BetterStandardPrinter(); + file_put_contents($hookClassFilename, $printer->prettyPrintFile($hookClassStmts)); + static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); + $this->module = ''; + } + } + + protected function createMethodFromFunction(Function_ $node): ?ClassMethod + { + $name = $node->name->toString(); + if (str_starts_with($name, '_')) + { + return NULL; + } + $hook = ''; + // If the function name starts with the module name, presume it's a hook. + if (preg_match("/^{$this->module}_(.*)/", $name, $matches)) + { + $hook = $matches[1]; + } + // If there is doxygen but there's no hook yet then try to find the + // hook and module by parsing the doxygen for "Implements hook_foo()." + if (($doc = $node->getDocComment()) && !$hook) + { + $implementsModule = static::getImplementsModule($hook, $name, $doc->getReformattedText()); + } + if ($hook) + { + // I prefer static because it shows what the method actually + // depends on. + return static::getMethod($node, $doc, $hook, $implementsModule ?? '', fn($name) => $this->nodeFactory->createPublicMethod($name)); + } + elseif (!str_starts_with($name, 'template_preprocess_')) + { + $this->addCommentService->addDrupalRectorComment($node, 'If this is a hook then convert it according to https://www.drupal.org/node/3442349'); + } + return NULL; + } + + protected static function getImplementsModule(string &$hook, string $functionName, string $doxygen): string { + // If the doxygen contains "Implements hook_foo()" then parse the hook + // name. A difficulty here is "Implements hook_form_FORM_ID_alter". + // Find these by looking for an optional part starting with an + // uppercase letter. + if (preg_match('/^ \* Implements hook_([a-z0-9_]+)(([A-Z][A-Z0-9_]+)(_[a-z0-9_]*))?/m', $doxygen, $matches)) + { + $preg = $matches[1]; + // If the optional part is present then replace the uppercase + // portions with an appropriate regex. + if (isset($matches[4])) + { + $preg .= '[a-z0-9_]+' . $matches[4]; + } + // And now find the module and the hook. + if (preg_match("/^(.*?)_($preg)$/", $functionName, $matches)) + { + $hook = $matches[2]; + return $matches[1]; + } + } + return ''; + } + + protected static function getMethod(Function_ $node, ?Doc $doc, string $hook, string $implementsModule, callable $methodFactory): ClassMethod { + $method = $methodFactory(static::getMethodName($node)); + assert($method instanceof ClassMethod); + if ($doc) + { + $method->setDocComment($doc); + } + // Do the actual copying. + foreach ($node->getSubNodeNames() as $subNodeName) + { + // The name is set up in the constructor. + if ($subNodeName !== 'name') + { + // Copying an object property could be a problem as those + // are copied by handle so changing it would change it in + // both places. But ::refactor() only changes stmts and + // attrGroups and both are arrays. This function also only + // changes attrGroups. + $method->$subNodeName = $node->$subNodeName; + } + } + // Assemble the arguments for the #[Hook] attribute. + $arguments = [new Node\Arg(new Node\Scalar\String_($hook))]; + if ($implementsModule) + { + $arguments[] = new Node\Arg(new Node\Scalar\String_($implementsModule), name: new Node\Identifier('module')); + } + $hookAttribute = new Node\Attribute(new Node\Name('Hook'), $arguments); + $method->attrGroups[] = new Node\AttributeGroup([$hookAttribute]); + return $method; + } + + /** + * @param \PhpParser\Node\Stmt\Function_ $node + * E.g.: user_entity_operation(EntityInterface $entity) + * + * @return \PhpParser\Node\Stmt\Expression + * E.g.: Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); + * + */ + protected function getServiceCall(Function_ $node): Node\Stmt\Expression + { + $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); + return new Node\Stmt\Expression($this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args)); + } + + /** + * @param \PhpParser\Node\Stmt\Function_ $node + * A function declaration for example the entire user_user_role_insert() + * function. + * + * @return string + * The function name converted to camelCase for e.g. userUserRoleInsert. + */ + public static function getMethodName(Function_ $node): string + { + return CaseStringHelper::camelCase($node->name->toString()); + } + + protected static function writeServicesYml(string $fileName, string $fullyClassifiedClassName): void + { + $services = is_file($fileName) ? file_get_contents($fileName) : ''; + $id = "\n $fullyClassifiedClassName:\n"; + if (!str_contains($services, $id)) + { + if (!str_contains($services, 'services:')) + { + $services .= "\nservices:"; + } + $services .= "$id class: $fullyClassifiedClassName\n autowire: true\n"; + file_put_contents($fileName, $services); + } + } + +} From 846858f1a97abba3e2e69e500cb0291dcc24a3d2 Mon Sep 17 00:00:00 2001 From: nicxvan Date: Thu, 17 Oct 2024 18:51:59 -0400 Subject: [PATCH 02/64] Move to set list --- config/hook-convert/hook-convert.php | 9 +++++++++ .../Convert => Convert/Rector}/HookConvert.php | 2 +- src/Set/HookConvertSetList.php | 12 ++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 config/hook-convert/hook-convert.php rename src/{Rector/Convert => Convert/Rector}/HookConvert.php (99%) create mode 100644 src/Set/HookConvertSetList.php diff --git a/config/hook-convert/hook-convert.php b/config/hook-convert/hook-convert.php new file mode 100644 index 00000000..2fbd8588 --- /dev/null +++ b/config/hook-convert/hook-convert.php @@ -0,0 +1,9 @@ +rule(\DrupalRector\Convert\Rector\HookConvertRector::class); +}; diff --git a/src/Rector/Convert/HookConvert.php b/src/Convert/Rector/HookConvert.php similarity index 99% rename from src/Rector/Convert/HookConvert.php rename to src/Convert/Rector/HookConvert.php index 6e877369..52134619 100644 --- a/src/Rector/Convert/HookConvert.php +++ b/src/Convert/Rector/HookConvert.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace DrupalRector\Rector\Convert; +namespace DrupalRector\Convert\Rector; use DrupalRector\Services\AddCommentService; use PhpParser\Comment\Doc; diff --git a/src/Set/HookConvertSetList.php b/src/Set/HookConvertSetList.php new file mode 100644 index 00000000..4ecfaa73 --- /dev/null +++ b/src/Set/HookConvertSetList.php @@ -0,0 +1,12 @@ + Date: Thu, 17 Oct 2024 18:59:12 -0400 Subject: [PATCH 03/64] Fix name --- config/hook-convert/hook-convert.php | 6 +++++- .../Rector/{HookConvert.php => HookConvertRector.php} | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) rename src/Convert/Rector/{HookConvert.php => HookConvertRector.php} (99%) diff --git a/config/hook-convert/hook-convert.php b/config/hook-convert/hook-convert.php index 2fbd8588..95b09388 100644 --- a/config/hook-convert/hook-convert.php +++ b/config/hook-convert/hook-convert.php @@ -2,8 +2,12 @@ declare(strict_types=1); +use DrupalRector\Convert\Rector\HookConvertRector; use Rector\Config\RectorConfig; return static function (RectorConfig $rectorConfig): void { - $rectorConfig->rule(\DrupalRector\Convert\Rector\HookConvertRector::class); + $rectorConfig->rule(HookConvertRector::class); + $rectorConfig->bootstrapFiles([ + __DIR__.'/../drupal-phpunit-bootstrap-file.php', + ]); }; diff --git a/src/Convert/Rector/HookConvert.php b/src/Convert/Rector/HookConvertRector.php similarity index 99% rename from src/Convert/Rector/HookConvert.php rename to src/Convert/Rector/HookConvertRector.php index 52134619..7340f25e 100644 --- a/src/Convert/Rector/HookConvert.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -15,7 +15,7 @@ use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; -class HookConvert extends AbstractRector +class HookConvertRector extends AbstractRector { protected string $inputFilename = ''; From 9c6b3e2bde7447c30e5534ec49f703820527b64f Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 11:03:58 -0400 Subject: [PATCH 04/64] Add naming rules to fix collisions --- src/Convert/Rector/HookConvertRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 7340f25e..5a1bafa3 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -121,7 +121,8 @@ protected function initializeHookClass(): void while (($this->moduleDir = dirname($this->moduleDir)) && !($info = glob("$this->moduleDir/*.info.yml"))); if ($infoFile = reset($info)) { $this->module = basename($infoFile, '.info.yml'); - $hookClassName = ucfirst(CaseStringHelper::camelCase($this->module . '_hooks')); + $filename = pathinfo($this->file->getFilePath(), \PATHINFO_FILENAME); + $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename) . '_hooks')); $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); $this->hookClass = new Node\Stmt\Class_(new Node\Name($hookClassName)); // Using $this->nodeFactory->createStaticCall() results in From 9211f278591cb96871a5fa8d64eb3e71dc6117dc Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 12:38:49 -0400 Subject: [PATCH 05/64] Fix legacy hook use statements --- src/Convert/Rector/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 5a1bafa3..36a0c826 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -105,7 +105,7 @@ public function refactor(Node $node): ?Node // Rewrite the function body to be a single service call. $node->stmts = [$this->getServiceCall($node)]; // Mark this function as a legacy hook. - $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\LegacyHook'))]); + $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); } return $node; } From 7ca59a35d9cc1965a511fe570bbb5dc6c3d3d046 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 12:52:46 -0400 Subject: [PATCH 06/64] Fix hook use statement --- src/Convert/Rector/HookConvertRector.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 36a0c826..a7d0f507 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -138,8 +138,8 @@ public function __destruct() { $className = $this->hookClass->name->toString(); $namespace = "Drupal\\$this->module\\Hook"; - // Create the statement use Drupal\Core\Hook\Hook; - $name = new Node\Name('Drupal\Core\Hook\Hook'); + // Create the statement use Drupal\Core\Hook\Attribute\Hook; + $name = new Node\Name('Drupal\Core\Hook\Attribute\Hook'); // UseItem contains an unguarded class_alias to the deprecated // UseUse class. However, due to some version mix-up it seems the // old class is used for parsing so only use the UseItem class if From a33cc764ecbb3f19ed55ec98841e0598ffdc8f82 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 14:18:40 -0400 Subject: [PATCH 07/64] Update return --- src/Convert/Rector/HookConvertRector.php | 34 ++++++++++++++---------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index a7d0f507..398578ee 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -2,11 +2,12 @@ declare(strict_types=1); -namespace DrupalRector\Convert\Rector; +namespace Utils\Rector\Rector; use DrupalRector\Services\AddCommentService; use PhpParser\Comment\Doc; use PhpParser\Node; +use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; @@ -103,7 +104,7 @@ public function refactor(Node $node): ?Node // See the note in ::getMethod() about how it's important to not // change any object property of $node here. // Rewrite the function body to be a single service call. - $node->stmts = [$this->getServiceCall($node)]; + $node->stmts = [new Node\Stmt\Return_($this->getServiceCall($node))]; // Mark this function as a legacy hook. $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); } @@ -137,7 +138,13 @@ public function __destruct() if ($this->module && $this->hookClass->stmts) { $className = $this->hookClass->name->toString(); - $namespace = "Drupal\\$this->module\\Hook"; + $counter = ''; + do { + $candidate = "$className$counter"; + $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; + $this->hookClass->name = new Node\Name($candidate); + $counter = $counter ? $counter + 1 : 1; + } while (file_exists($hookClassFilename)); // Create the statement use Drupal\Core\Hook\Attribute\Hook; $name = new Node\Name('Drupal\Core\Hook\Attribute\Hook'); // UseItem contains an unguarded class_alias to the deprecated @@ -146,6 +153,7 @@ public function __destruct() // it is already loaded or the UseUse class is completely gone. $useHook = class_exists('PhpParser\Node\UseItem', FALSE) || !class_exists('PhpParser\Node\Stmt\UseUse') ? new Node\UseItem($name) : new Node\Stmt\UseUse($name); // Put the class together. + $namespace = "Drupal\\$this->module\\Hook"; $hookClassStmts = [ new Node\Stmt\Namespace_(new Node\Name($namespace)), ... $this->useStmts, @@ -155,7 +163,6 @@ public function __destruct() // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); - $hookClassFilename = "$this->moduleDir/src/Hook/$className.php"; $printer = new BetterStandardPrinter(); file_put_contents($hookClassFilename, $printer->prettyPrintFile($hookClassStmts)); static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); @@ -251,18 +258,17 @@ protected static function getMethod(Function_ $node, ?Doc $doc, string $hook, st return $method; } - /** - * @param \PhpParser\Node\Stmt\Function_ $node - * E.g.: user_entity_operation(EntityInterface $entity) - * - * @return \PhpParser\Node\Stmt\Expression - * E.g.: Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); - * - */ - protected function getServiceCall(Function_ $node): Node\Stmt\Expression + /** + * @param \PhpParser\Node\Stmt\Function_ $node + * E.g.: user_entity_operation(EntityInterface $entity) + * + * @return \PhpParser\Node\Expr\MethodCall + * E.g.: Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); + */ + protected function getServiceCall(Function_ $node): MethodCall { $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); - return new Node\Stmt\Expression($this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args)); + return $this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args); } /** From 13ccf24e46c7dae1a541bfcf8355fb0a7bdffcc4 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 15:37:16 -0400 Subject: [PATCH 08/64] Handle return statements --- src/Convert/Rector/HookConvertRector.php | 48 ++++++++++++++++-------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 398578ee..0b4acb11 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -7,9 +7,11 @@ use DrupalRector\Services\AddCommentService; use PhpParser\Comment\Doc; use PhpParser\Node; -use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; +use PhpParser\Node\Stmt\Return_; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor\FirstFindingVisitor; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; @@ -54,11 +56,10 @@ class HookConvertRector extends AbstractRector */ protected Node\Expr\StaticCall $drupalServiceCall; - public function __construct(protected AddCommentService $addCommentService) { + public function __construct(protected AddCommentService $addCommentService) + { } - - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Hook conversion script', [ @@ -82,7 +83,7 @@ public function getNodeTypes(): array return [Function_::class, Node\Stmt\Use_::class]; } - public function refactor(Node $node): ?Node + public function refactor(Node $node): Node|array|NULL { $filePath = $this->file->getFilePath(); $ext = pathinfo($filePath, \PATHINFO_EXTENSION); @@ -101,17 +102,23 @@ public function refactor(Node $node): ?Node if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; + // Rewrite the function body to be a single service call. + $serviceCall = $this->getServiceCall($node); + // If the function body doesn't contain a return statement, + // remove the return from the service call. + if (!$this->hasReturn($node)) + { + $serviceCall = new Node\Stmt\Expression($serviceCall->expr); + } // See the note in ::getMethod() about how it's important to not // change any object property of $node here. - // Rewrite the function body to be a single service call. - $node->stmts = [new Node\Stmt\Return_($this->getServiceCall($node))]; + $node->stmts = [$serviceCall]; // Mark this function as a legacy hook. $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); } return $node; } - protected function initializeHookClass(): void { $this->__destruct(); @@ -140,10 +147,10 @@ public function __destruct() $className = $this->hookClass->name->toString(); $counter = ''; do { - $candidate = "$className$counter"; - $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; - $this->hookClass->name = new Node\Name($candidate); - $counter = $counter ? $counter + 1 : 1; + $candidate = "$className$counter"; + $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; + $this->hookClass->name = new Node\Name($candidate); + $counter = $counter ? $counter + 1 : 1; } while (file_exists($hookClassFilename)); // Create the statement use Drupal\Core\Hook\Attribute\Hook; $name = new Node\Name('Drupal\Core\Hook\Attribute\Hook'); @@ -262,13 +269,13 @@ protected static function getMethod(Function_ $node, ?Doc $doc, string $hook, st * @param \PhpParser\Node\Stmt\Function_ $node * E.g.: user_entity_operation(EntityInterface $entity) * - * @return \PhpParser\Node\Expr\MethodCall - * E.g.: Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); + * @return \PhpParser\Node\Stmt\Return_ + * E.g.: return Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); */ - protected function getServiceCall(Function_ $node): MethodCall + protected function getServiceCall(Function_ $node): Return_ { $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); - return $this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args); + return new Return_($this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args)); } /** @@ -299,4 +306,13 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi } } + protected function hasReturn(Function_ $node): bool + { + $returnVisitor = new FirstFindingVisitor(fn(Node $node) => $node instanceof Return_); + $traverser = new NodeTraverser(); + $traverser->addVisitor($returnVisitor); + $traverser->traverse([$node]); + return $returnVisitor->getFoundNode() !== NULL; + } + } From 75bbed33b30fe39dd0e3777801a5532b93087806 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 15:39:03 -0400 Subject: [PATCH 09/64] HookConvert --- src/Convert/Rector/HookConvertRector.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 0b4acb11..61d95aa9 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -10,6 +10,7 @@ use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Return_; +use PhpParser\NodeFinder; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\FirstFindingVisitor; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; @@ -106,7 +107,7 @@ public function refactor(Node $node): Node|array|NULL $serviceCall = $this->getServiceCall($node); // If the function body doesn't contain a return statement, // remove the return from the service call. - if (!$this->hasReturn($node)) + if (!(new NodeFinder)->findInstanceOf([$node], Return_::class)) { $serviceCall = new Node\Stmt\Expression($serviceCall->expr); } @@ -306,13 +307,4 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi } } - protected function hasReturn(Function_ $node): bool - { - $returnVisitor = new FirstFindingVisitor(fn(Node $node) => $node instanceof Return_); - $traverser = new NodeTraverser(); - $traverser->addVisitor($returnVisitor); - $traverser->traverse([$node]); - return $returnVisitor->getFoundNode() !== NULL; - } - } From 25ae09712ec23fe437e6ecbecca6b1c13056dae4 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 15:41:42 -0400 Subject: [PATCH 10/64] Fix namespace --- src/Convert/Rector/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 61d95aa9..d25caa07 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Utils\Rector\Rector; +namespace DrupalRector\Convert\Rector; use DrupalRector\Services\AddCommentService; use PhpParser\Comment\Doc; From eaa26da24ed017bb1b29a3c3317cefd767911c10 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Fri, 18 Oct 2024 16:55:24 -0400 Subject: [PATCH 11/64] Update from upstream --- src/Convert/Rector/HookConvertRector.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index d25caa07..5fead131 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -11,8 +11,6 @@ use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Return_; use PhpParser\NodeFinder; -use PhpParser\NodeTraverser; -use PhpParser\NodeVisitor\FirstFindingVisitor; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; @@ -57,7 +55,7 @@ class HookConvertRector extends AbstractRector */ protected Node\Expr\StaticCall $drupalServiceCall; - public function __construct(protected AddCommentService $addCommentService) + public function __construct(protected AddCommentService $addCommentService, protected BetterStandardPrinter $printer) { } @@ -107,7 +105,7 @@ public function refactor(Node $node): Node|array|NULL $serviceCall = $this->getServiceCall($node); // If the function body doesn't contain a return statement, // remove the return from the service call. - if (!(new NodeFinder)->findInstanceOf([$node], Return_::class)) + if (!(new NodeFinder)->findFirstInstanceOf([$node], Return_::class)) { $serviceCall = new Node\Stmt\Expression($serviceCall->expr); } @@ -171,8 +169,7 @@ public function __destruct() // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); - $printer = new BetterStandardPrinter(); - file_put_contents($hookClassFilename, $printer->prettyPrintFile($hookClassStmts)); + file_put_contents($hookClassFilename, $this->printer->prettyPrintFile($hookClassStmts)); static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); $this->module = ''; } From f034d9b62b07aa7f1ead7217e2ceec5da1289f95 Mon Sep 17 00:00:00 2001 From: nicxvan Date: Fri, 18 Oct 2024 20:44:17 -0400 Subject: [PATCH 12/64] Handle Variable --- src/Convert/Rector/HookConvertRector.php | 78 +++++++++--------------- 1 file changed, 28 insertions(+), 50 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 5fead131..253037a1 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -4,7 +4,6 @@ namespace DrupalRector\Convert\Rector; -use DrupalRector\Services\AddCommentService; use PhpParser\Comment\Doc; use PhpParser\Node; use PhpParser\Node\Stmt\ClassMethod; @@ -55,7 +54,7 @@ class HookConvertRector extends AbstractRector */ protected Node\Expr\StaticCall $drupalServiceCall; - public function __construct(protected AddCommentService $addCommentService, protected BetterStandardPrinter $printer) + public function __construct(protected BetterStandardPrinter $printer) { } @@ -178,66 +177,44 @@ public function __destruct() protected function createMethodFromFunction(Function_ $node): ?ClassMethod { $name = $node->name->toString(); - if (str_starts_with($name, '_')) - { - return NULL; - } - $hook = ''; - // If the function name starts with the module name, presume it's a hook. - if (preg_match("/^{$this->module}_(.*)/", $name, $matches)) - { - $hook = $matches[1]; - } - // If there is doxygen but there's no hook yet then try to find the - // hook and module by parsing the doxygen for "Implements hook_foo()." - if (($doc = $node->getDocComment()) && !$hook) - { - $implementsModule = static::getImplementsModule($hook, $name, $doc->getReformattedText()); - } - if ($hook) - { - // I prefer static because it shows what the method actually - // depends on. - return static::getMethod($node, $doc, $hook, $implementsModule ?? '', fn($name) => $this->nodeFactory->createPublicMethod($name)); - } - elseif (!str_starts_with($name, 'template_preprocess_')) + // If the doxygen contains "Implements hook_foo()" then parse the hook + // name. A difficulty here is "Implements hook_form_FORM_ID_alter". + // Find these by looking for an optional part starting with an + // uppercase letter. + if (($doc = $node->getDocComment()) && ($return = static::getHookAndModuleName($name, $doc))) { - $this->addCommentService->addDrupalRectorComment($node, 'If this is a hook then convert it according to https://www.drupal.org/node/3442349'); + [, $implementsModule, $hook] = $return; + if (str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'preprocess')) + { + return NULL; + } + if ($implementsModule === $this->module) { + $implementsModule = ''; + } + $method = $this->nodeFactory->createPublicMethod($this->getMethodName($node)); + return static::copyFunctionToMethod($node, $doc, $hook, $implementsModule, $method); } return NULL; } - protected static function getImplementsModule(string &$hook, string $functionName, string $doxygen): string { - // If the doxygen contains "Implements hook_foo()" then parse the hook - // name. A difficulty here is "Implements hook_form_FORM_ID_alter". - // Find these by looking for an optional part starting with an - // uppercase letter. - if (preg_match('/^ \* Implements hook_([a-z0-9_]+)(([A-Z][A-Z0-9_]+)(_[a-z0-9_]*))?/m', $doxygen, $matches)) + protected static function getHookAndModuleName(string $functionName, Doc $doc): array + { + if (preg_match('/^ \* Implements hook_([a-z0-9_]*)(([A-Z][A-Z0-9_]+)(_[a-z0-9_]*))?/m', $doc->getReformattedText(), $matches)) { $preg = $matches[1]; // If the optional part is present then replace the uppercase // portions with an appropriate regex. - if (isset($matches[4])) - { + if (isset($matches[4])) { $preg .= '[a-z0-9_]+' . $matches[4]; } // And now find the module and the hook. - if (preg_match("/^(.*?)_($preg)$/", $functionName, $matches)) - { - $hook = $matches[2]; - return $matches[1]; - } + preg_match("/^(.+)_($preg)$/", $functionName, $matches); } - return ''; + return $matches; } - protected static function getMethod(Function_ $node, ?Doc $doc, string $hook, string $implementsModule, callable $methodFactory): ClassMethod { - $method = $methodFactory(static::getMethodName($node)); - assert($method instanceof ClassMethod); - if ($doc) - { - $method->setDocComment($doc); - } + protected static function copyFunctionToMethod(Function_ $node, Doc $doc, string $hook, string $implementsModule, ClassMethod $method): ClassMethod { + $method->setDocComment($doc); // Do the actual copying. foreach ($node->getSubNodeNames() as $subNodeName) { @@ -273,7 +250,7 @@ protected static function getMethod(Function_ $node, ?Doc $doc, string $hook, st protected function getServiceCall(Function_ $node): Return_ { $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); - return new Return_($this->nodeFactory->createMethodCall($this->drupalServiceCall, static::getMethodName($node), $args)); + return new Return_($this->nodeFactory->createMethodCall($this->drupalServiceCall, $this->getMethodName($node), $args)); } /** @@ -284,9 +261,10 @@ protected function getServiceCall(Function_ $node): Return_ * @return string * The function name converted to camelCase for e.g. userUserRoleInsert. */ - public static function getMethodName(Function_ $node): string + protected function getMethodName(Function_ $node): string { - return CaseStringHelper::camelCase($node->name->toString()); + $name = preg_replace("/^$this->module" . '_/', '', $node->name->toString()); + return CaseStringHelper::camelCase($name); } protected static function writeServicesYml(string $fileName, string $fullyClassifiedClassName): void From 06bffcc6581899d893bb3f1b8dcfe7d76e63173c Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Sat, 19 Oct 2024 12:26:42 -0400 Subject: [PATCH 13/64] Update handle core vs contrib --- src/Convert/Rector/HookConvertRector.php | 177 ++++++++++------------- 1 file changed, 77 insertions(+), 100 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index 253037a1..b287a45c 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -1,15 +1,20 @@ drupalCore = realpath($corePath); + } + } + catch (\OutOfBoundsException $e) { } } public function getRuleDefinition(): RuleDefinition @@ -81,40 +95,25 @@ public function getNodeTypes(): array return [Function_::class, Node\Stmt\Use_::class]; } - public function refactor(Node $node): Node|array|NULL + public function refactor(Node $node): Node|NULL|int { $filePath = $this->file->getFilePath(); $ext = pathinfo($filePath, \PATHINFO_EXTENSION); - if (!in_array($ext, ['inc', 'module'])) - { - return $node; + if (!in_array($ext, ['inc', 'module'])) { + return NULL; } - if ($filePath !== $this->inputFilename) - { + if ($filePath !== $this->inputFilename) { $this->initializeHookClass(); } if ($node instanceof Node\Stmt\Use_) { $this->useStmts[] = $node; } - if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) - { + if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; - // Rewrite the function body to be a single service call. - $serviceCall = $this->getServiceCall($node); - // If the function body doesn't contain a return statement, - // remove the return from the service call. - if (!(new NodeFinder)->findFirstInstanceOf([$node], Return_::class)) - { - $serviceCall = new Node\Stmt\Expression($serviceCall->expr); - } - // See the note in ::getMethod() about how it's important to not - // change any object property of $node here. - $node->stmts = [$serviceCall]; - // Mark this function as a legacy hook. - $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); + return str_starts_with($filePath, $this->drupalCore) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); } - return $node; + return NULL; } protected function initializeHookClass(): void @@ -133,15 +132,14 @@ protected function initializeHookClass(): void $this->hookClass = new Node\Stmt\Class_(new Node\Name($hookClassName)); // Using $this->nodeFactory->createStaticCall() results in // use \Drupal; on top which is not desirable. - $classConst = new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified("$namespace\\$hookClassName"), 'CLASS'); + $classConst = new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified("$namespace\\$hookClassName"), 'class'); $this->drupalServiceCall = new Node\Expr\StaticCall(new Node\Name\FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); } } public function __destruct() { - if ($this->module && $this->hookClass->stmts) - { + if ($this->module && $this->hookClass->stmts) { $className = $this->hookClass->name->toString(); $counter = ''; do { @@ -150,107 +148,76 @@ public function __destruct() $this->hookClass->name = new Node\Name($candidate); $counter = $counter ? $counter + 1 : 1; } while (file_exists($hookClassFilename)); - // Create the statement use Drupal\Core\Hook\Attribute\Hook; - $name = new Node\Name('Drupal\Core\Hook\Attribute\Hook'); - // UseItem contains an unguarded class_alias to the deprecated - // UseUse class. However, due to some version mix-up it seems the - // old class is used for parsing so only use the UseItem class if - // it is already loaded or the UseUse class is completely gone. - $useHook = class_exists('PhpParser\Node\UseItem', FALSE) || !class_exists('PhpParser\Node\Stmt\UseUse') ? new Node\UseItem($name) : new Node\Stmt\UseUse($name); - // Put the class together. + // Put the file together. $namespace = "Drupal\\$this->module\\Hook"; $hookClassStmts = [ new Node\Stmt\Namespace_(new Node\Name($namespace)), ... $this->useStmts, - new Node\Stmt\Use_([$useHook]), + new Node\Stmt\Use_([new Node\Stmt\UseUse(new Node\Name('Drupal\Core\Hook\Attribute\Hook'))]), $this->hookClass, ]; // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); file_put_contents($hookClassFilename, $this->printer->prettyPrintFile($hookClassStmts)); - static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); + if (!str_starts_with($this->moduleDir, $this->drupalCore)) { + static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); + } $this->module = ''; + $this->useStmts = []; } } protected function createMethodFromFunction(Function_ $node): ?ClassMethod { - $name = $node->name->toString(); // If the doxygen contains "Implements hook_foo()" then parse the hook // name. A difficulty here is "Implements hook_form_FORM_ID_alter". // Find these by looking for an optional part starting with an // uppercase letter. - if (($doc = $node->getDocComment()) && ($return = static::getHookAndModuleName($name, $doc))) - { - [, $implementsModule, $hook] = $return; - if (str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'preprocess')) - { + if ($info = static::getHookAndModuleName($node)) { + ['hook' => $hook, 'module' => $implementsModule] = $info; + $procOnly = [ + 'install', + 'module_preinstall', + 'module_preuninstall', + 'modules_installed', + 'modules_uninstalled', + 'requirements', + 'schema', + 'uninstall', + 'update_last_removed', + ]; + if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return NULL; } - if ($implementsModule === $this->module) { - $implementsModule = ''; + $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); + $method->flags = Class_::MODIFIER_PUBLIC; + // Assemble the arguments for the #[Hook] attribute. + $arguments = [new Node\Arg(new Node\Scalar\String_($hook))]; + if ($implementsModule !== $this->module) { + $arguments[] = new Node\Arg(new Node\Scalar\String_($implementsModule), name: new Node\Identifier('module')); } - $method = $this->nodeFactory->createPublicMethod($this->getMethodName($node)); - return static::copyFunctionToMethod($node, $doc, $hook, $implementsModule, $method); + $hookAttribute = new Node\Attribute(new Node\Name('Hook'), $arguments); + $method->attrGroups[] = new Node\AttributeGroup([$hookAttribute]); + return $method; } return NULL; } - protected static function getHookAndModuleName(string $functionName, Doc $doc): array + protected static function getHookAndModuleName(Function_ $node): array { - if (preg_match('/^ \* Implements hook_([a-z0-9_]*)(([A-Z][A-Z0-9_]+)(_[a-z0-9_]*))?/m', $doc->getReformattedText(), $matches)) - { - $preg = $matches[1]; + if (preg_match('/^ \* Implements hook_([a-z0-9_]*)(?:[A-Z][A-Z0-9_]+(_[a-z0-9_]*))?/m', (string) $node->getDocComment()?->getReformattedText(), $matches)) { + $hookRegex = $matches[1]; // If the optional part is present then replace the uppercase // portions with an appropriate regex. - if (isset($matches[4])) { - $preg .= '[a-z0-9_]+' . $matches[4]; + if (isset($matches[2])) { + $hookRegex .= '[a-z0-9_]+' . $matches[2]; } // And now find the module and the hook. - preg_match("/^(.+)_($preg)$/", $functionName, $matches); - } - return $matches; - } - - protected static function copyFunctionToMethod(Function_ $node, Doc $doc, string $hook, string $implementsModule, ClassMethod $method): ClassMethod { - $method->setDocComment($doc); - // Do the actual copying. - foreach ($node->getSubNodeNames() as $subNodeName) - { - // The name is set up in the constructor. - if ($subNodeName !== 'name') - { - // Copying an object property could be a problem as those - // are copied by handle so changing it would change it in - // both places. But ::refactor() only changes stmts and - // attrGroups and both are arrays. This function also only - // changes attrGroups. - $method->$subNodeName = $node->$subNodeName; - } + preg_match("/^(?.+?)_(?$hookRegex)$/", $node->name->toString(), $matches); + return $matches; } - // Assemble the arguments for the #[Hook] attribute. - $arguments = [new Node\Arg(new Node\Scalar\String_($hook))]; - if ($implementsModule) - { - $arguments[] = new Node\Arg(new Node\Scalar\String_($implementsModule), name: new Node\Identifier('module')); - } - $hookAttribute = new Node\Attribute(new Node\Name('Hook'), $arguments); - $method->attrGroups[] = new Node\AttributeGroup([$hookAttribute]); - return $method; - } - - /** - * @param \PhpParser\Node\Stmt\Function_ $node - * E.g.: user_entity_operation(EntityInterface $entity) - * - * @return \PhpParser\Node\Stmt\Return_ - * E.g.: return Drupal::service('Drupal\user\Hook\UserHooks')->userEntityOperation($entity); - */ - protected function getServiceCall(Function_ $node): Return_ - { - $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); - return new Return_($this->nodeFactory->createMethodCall($this->drupalServiceCall, $this->getMethodName($node), $args)); + return []; } /** @@ -263,18 +230,27 @@ protected function getServiceCall(Function_ $node): Return_ */ protected function getMethodName(Function_ $node): string { - $name = preg_replace("/^$this->module" . '_/', '', $node->name->toString()); - return CaseStringHelper::camelCase($name); + $name = preg_replace("/^{$this->module}_/", '', $node->name->toString()); + return CaseStringHelper::camelCase($name); + } + + public function getLegacyHookFunction(Function_ $node): Function_ + { + $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); + $methodCall = $this->nodeFactory->createMethodCall($this->drupalServiceCall, $this->getMethodName($node), $args); + $hasReturn = (new NodeFinder)->findFirstInstanceOf([$node], Return_::class); + $node->stmts = [$hasReturn ? new Return_($methodCall) : new Node\Stmt\Expression($methodCall)]; + // Mark this function as a legacy hook. + $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); + return $node; } protected static function writeServicesYml(string $fileName, string $fullyClassifiedClassName): void { $services = is_file($fileName) ? file_get_contents($fileName) : ''; $id = "\n $fullyClassifiedClassName:\n"; - if (!str_contains($services, $id)) - { - if (!str_contains($services, 'services:')) - { + if (!str_contains($services, $id)) { + if (!str_contains($services, 'services:')) { $services .= "\nservices:"; } $services .= "$id class: $fullyClassifiedClassName\n autowire: true\n"; @@ -282,4 +258,5 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi } } + } From 64f47674a02409ff90833bff9077ec5d6cbf0920 Mon Sep 17 00:00:00 2001 From: nicxvan Date: Sat, 19 Oct 2024 19:36:36 -0400 Subject: [PATCH 14/64] Fix duplicate comments and use statements --- src/Convert/Rector/HookConvertRector.php | 70 +++++++++--------------- 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Convert/Rector/HookConvertRector.php index b287a45c..eafb94fb 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Convert/Rector/HookConvertRector.php @@ -9,10 +9,7 @@ use Composer\InstalledVersions; use PhpParser\Node; -use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Function_; -use PhpParser\Node\Stmt\Return_; +use PhpParser\Node\Stmt\{Use_, Class_, ClassMethod, Function_, Return_}; use PhpParser\NodeFinder; use PhpParser\NodeTraverser; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; @@ -26,30 +23,12 @@ class HookConvertRector extends AbstractRector protected string $inputFilename = ''; - /** - * @var Node\Stmt\Use_[] - */ protected array $useStmts = []; - /** - * @var \PhpParser\Node\Stmt\Class_ - * - * The hook class itself. - */ - protected Node\Stmt\Class_ $hookClass; + protected Class_ $hookClass; - /** - * @var string - * - * The name of the module, used to guess which functions are hooks. - */ protected string $module = ''; - /** - * @var string - * - * THe module directory, used to write services.yml - */ protected string $moduleDir = ''; /** @@ -59,14 +38,14 @@ class HookConvertRector extends AbstractRector */ protected Node\Expr\StaticCall $drupalServiceCall; - private string $drupalCore = "\0"; + private string $drupalCorePath = "\0"; public function __construct(protected BetterStandardPrinter $printer) { try { - if ($corePath = InstalledVersions::getInstallPath('drupal/core')) { - $this->drupalCore = realpath($corePath); + if (class_exists(InstalledVersions::class) && ($corePath = InstalledVersions::getInstallPath('drupal/core'))) { + $this->drupalCorePath = realpath($corePath); } } catch (\OutOfBoundsException $e) { } @@ -92,7 +71,7 @@ public function getRuleDefinition(): RuleDefinition */ public function getNodeTypes(): array { - return [Function_::class, Node\Stmt\Use_::class]; + return [Function_::class, Use_::class]; } public function refactor(Node $node): Node|NULL|int @@ -105,13 +84,13 @@ public function refactor(Node $node): Node|NULL|int if ($filePath !== $this->inputFilename) { $this->initializeHookClass(); } - if ($node instanceof Node\Stmt\Use_) { - $this->useStmts[] = $node; + if ($node instanceof Use_) { + $this->useStmts[$this->printer->prettyPrint([$node])] = $node; } if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; - return str_starts_with($filePath, $this->drupalCore) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); + return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); } return NULL; } @@ -129,11 +108,12 @@ protected function initializeHookClass(): void $filename = pathinfo($this->file->getFilePath(), \PATHINFO_FILENAME); $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename) . '_hooks')); $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); - $this->hookClass = new Node\Stmt\Class_(new Node\Name($hookClassName)); + $this->hookClass = new Class_(new Node\Name($hookClassName)); // Using $this->nodeFactory->createStaticCall() results in // use \Drupal; on top which is not desirable. $classConst = new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified("$namespace\\$hookClassName"), 'class'); $this->drupalServiceCall = new Node\Expr\StaticCall(new Node\Name\FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); + $this->useStmts = []; } } @@ -153,28 +133,23 @@ public function __destruct() $hookClassStmts = [ new Node\Stmt\Namespace_(new Node\Name($namespace)), ... $this->useStmts, - new Node\Stmt\Use_([new Node\Stmt\UseUse(new Node\Name('Drupal\Core\Hook\Attribute\Hook'))]), + new Use_([new Node\Stmt\UseUse(new Node\Name('Drupal\Core\Hook\Attribute\Hook'))]), $this->hookClass, ]; // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); file_put_contents($hookClassFilename, $this->printer->prettyPrintFile($hookClassStmts)); - if (!str_starts_with($this->moduleDir, $this->drupalCore)) { + if (!str_starts_with($this->moduleDir, $this->drupalCorePath)) { static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); } - $this->module = ''; - $this->useStmts = []; } + $this->module = ''; } protected function createMethodFromFunction(Function_ $node): ?ClassMethod { - // If the doxygen contains "Implements hook_foo()" then parse the hook - // name. A difficulty here is "Implements hook_form_FORM_ID_alter". - // Find these by looking for an optional part starting with an - // uppercase letter. - if ($info = static::getHookAndModuleName($node)) { + if ($info = $this->getHookAndModuleName($node)) { ['hook' => $hook, 'module' => $implementsModule] = $info; $procOnly = [ 'install', @@ -204,8 +179,12 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod return NULL; } - protected static function getHookAndModuleName(Function_ $node): array + protected function getHookAndModuleName(Function_ $node): array { + // If the doxygen contains "Implements hook_foo()" then parse the hook + // name. A difficulty here is "Implements hook_form_FORM_ID_alter". + // Find these by looking for an optional part starting with an + // uppercase letter. if (preg_match('/^ \* Implements hook_([a-z0-9_]*)(?:[A-Z][A-Z0-9_]+(_[a-z0-9_]*))?/m', (string) $node->getDocComment()?->getReformattedText(), $matches)) { $hookRegex = $matches[1]; // If the optional part is present then replace the uppercase @@ -213,9 +192,14 @@ protected static function getHookAndModuleName(Function_ $node): array if (isset($matches[2])) { $hookRegex .= '[a-z0-9_]+' . $matches[2]; } + $hookRegex = "_(?$hookRegex)"; + $functionName = $node->name->toString(); // And now find the module and the hook. - preg_match("/^(?.+?)_(?$hookRegex)$/", $node->name->toString(), $matches); - return $matches; + foreach ([$this->module, '.+?'] as $module) { + if (preg_match("/^(?$module)$hookRegex$/", $functionName, $matches)) { + return $matches; + } + } } return []; } From 97dfebb9162cf02274474b00eb8d186393413a9b Mon Sep 17 00:00:00 2001 From: bjorn Date: Sun, 20 Oct 2024 11:28:39 +0200 Subject: [PATCH 15/64] fix: revert adding the Hook into its own set and move class --- config/hook-convert/hook-convert.php | 13 ------------- .../Rector => Rector/Convert}/HookConvertRector.php | 2 +- src/Set/HookConvertSetList.php | 12 ------------ 3 files changed, 1 insertion(+), 26 deletions(-) delete mode 100644 config/hook-convert/hook-convert.php rename src/{Convert/Rector => Rector/Convert}/HookConvertRector.php (99%) delete mode 100644 src/Set/HookConvertSetList.php diff --git a/config/hook-convert/hook-convert.php b/config/hook-convert/hook-convert.php deleted file mode 100644 index 95b09388..00000000 --- a/config/hook-convert/hook-convert.php +++ /dev/null @@ -1,13 +0,0 @@ -rule(HookConvertRector::class); - $rectorConfig->bootstrapFiles([ - __DIR__.'/../drupal-phpunit-bootstrap-file.php', - ]); -}; diff --git a/src/Convert/Rector/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php similarity index 99% rename from src/Convert/Rector/HookConvertRector.php rename to src/Rector/Convert/HookConvertRector.php index eafb94fb..b2740c8b 100644 --- a/src/Convert/Rector/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -5,7 +5,7 @@ declare(strict_types=1); -namespace DrupalRector\Convert\Rector; +namespace DrupalRector\Rector\Convert; use Composer\InstalledVersions; use PhpParser\Node; diff --git a/src/Set/HookConvertSetList.php b/src/Set/HookConvertSetList.php deleted file mode 100644 index 4ecfaa73..00000000 --- a/src/Set/HookConvertSetList.php +++ /dev/null @@ -1,12 +0,0 @@ - Date: Sun, 20 Oct 2024 12:52:03 +0200 Subject: [PATCH 16/64] feat: initial new test structure --- .../functional_test__single_rectors.yml | 73 +++++++++++++++++++ .../fixture/module/module.module | 7 ++ .../fixture/module_updated/module.module | 9 +++ tests/functional/HookConverRector/rector.php | 28 +++++++ 4 files changed, 117 insertions(+) create mode 100644 .github/workflows/functional_test__single_rectors.yml create mode 100644 tests/functional/HookConverRector/fixture/module/module.module create mode 100644 tests/functional/HookConverRector/fixture/module_updated/module.module create mode 100644 tests/functional/HookConverRector/rector.php diff --git a/.github/workflows/functional_test__single_rectors.yml b/.github/workflows/functional_test__single_rectors.yml new file mode 100644 index 00000000..c88a0951 --- /dev/null +++ b/.github/workflows/functional_test__single_rectors.yml @@ -0,0 +1,73 @@ + +name: functional_test_single + +# This test will run on every pull request, and on every commit on any branch +on: + push: + branches: + - main + pull_request: + schedule: + # Run tests every week (to check for rector changes) + - cron: '0 0 * * 0' + +jobs: + run_functional_test_single: + name: Functional Test | single rectors" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: shivammathur/setup-php@v2 + with: + php-version: "${{ matrix.php-version }}" + coverage: none + tools: composer:v2 + extensions: dom, curl, libxml, mbstring, zip, pdo, mysql, pdo_mysql, gd + - name: Setup Drupal + uses: bluehorndigital/setup-drupal@v1.0.4 + with: + version: '^11.0' + path: ~/drupal + - name: Install Drupal Rector + run: | + cd ~/drupal + composer require palantirnet/drupal-rector:@dev --no-progress + - name: Prepare rector_examples folder in the drupal modules directory + run: | + cd ~/drupal + mkdir -p web/modules/custom + cp -R vendor/palantirnet/drupal-rector/tests/functional/* web/modules/custom + # dry-run is expected to return exit code 2 if there are changes, which we are expecting to happen, here. + # an error code of 1 represents other errors. + # @see \Rector\Core\Console\ExitCode::CHANGED_CODE + - name: Run rector against Drupal (dry-run) + run: | + cd ~/drupal + for d in web/modules/custom/*; do + if [ -d "$d" ]; then + cp ${d}/rector.php . + vendor/bin/rector process $d/fixture/module --dry-run --debug || if (($? == 2)); then true; else exit 1; fi + fi + done + - name: Run rector against Drupal + run: | + cd ~/drupal + for d in web/modules/custom/*; do + if [ -d "$d" ]; then + cp ${d}/rector.php . + vendor/bin/rector process $d/fixture/module --debug + fi + done + # diff options: + # -r: recursive + # -u: show the joined context, like git diff + # -b: ignore whitespace + # -B: ignore lines that are only whitespace + - name: Check that the updated examples match expectations + run: | + cd ~/drupal + for d in web/modules/custom/*; do + if [ -d "$d" ]; then + diff -rubB "$d/fixture/module" "$d/fixture/module_updated" + fi + done diff --git a/tests/functional/HookConverRector/fixture/module/module.module b/tests/functional/HookConverRector/fixture/module/module.module new file mode 100644 index 00000000..68de33b3 --- /dev/null +++ b/tests/functional/HookConverRector/fixture/module/module.module @@ -0,0 +1,7 @@ + diff --git a/tests/functional/HookConverRector/fixture/module_updated/module.module b/tests/functional/HookConverRector/fixture/module_updated/module.module new file mode 100644 index 00000000..b5958e7d --- /dev/null +++ b/tests/functional/HookConverRector/fixture/module_updated/module.module @@ -0,0 +1,9 @@ + diff --git a/tests/functional/HookConverRector/rector.php b/tests/functional/HookConverRector/rector.php new file mode 100644 index 00000000..bf76cf80 --- /dev/null +++ b/tests/functional/HookConverRector/rector.php @@ -0,0 +1,28 @@ +rule(\DrupalRector\Rector\Convert\HookConvertRector::class); + + $drupalFinder = new DrupalFinder(); + $drupalFinder->locateRoot(__DIR__); + $drupalRoot = $drupalFinder->getDrupalRoot(); + $rectorConfig->autoloadPaths([ + $drupalRoot . '/core', + $drupalRoot . '/modules', + $drupalRoot . '/profiles', + $drupalRoot . '/themes' + ]); + + $rectorConfig->skip(['*/upgrade_status/tests/modules/*']); + $rectorConfig->fileExtensions(['php', 'module', 'theme', 'install', 'profile', 'inc', 'engine']); + $rectorConfig->importNames(true, false); + $rectorConfig->importShortClasses(false); +}; From d69b619322d2624137bfd918eddf41b01aee639e Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Sun, 20 Oct 2024 17:42:21 +0200 Subject: [PATCH 17/64] add comments --- src/Rector/Convert/HookConvertRector.php | 59 ++++++++++++++++-------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index b2740c8b..ce794943 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -1,17 +1,15 @@ initializeHookClass(); } if ($node instanceof Use_) { + // For some unknown reason some Use_ statements are passed twice + // to this method. $this->useStmts[$this->printer->prettyPrint([$node])] = $node; } @@ -103,16 +106,17 @@ protected function initializeHookClass(): void // Find the relevant info.yml: it's either in the current directory or // one of the parents. while (($this->moduleDir = dirname($this->moduleDir)) && !($info = glob("$this->moduleDir/*.info.yml"))); - if ($infoFile = reset($info)) { + if (!empty($info)) { + $infoFile = reset($info); $this->module = basename($infoFile, '.info.yml'); $filename = pathinfo($this->file->getFilePath(), \PATHINFO_FILENAME); $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename) . '_hooks')); $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); - $this->hookClass = new Class_(new Node\Name($hookClassName)); + $this->hookClass = new Class_(new Node\Identifier($hookClassName)); // Using $this->nodeFactory->createStaticCall() results in // use \Drupal; on top which is not desirable. - $classConst = new Node\Expr\ClassConstFetch(new Node\Name\FullyQualified("$namespace\\$hookClassName"), 'class'); - $this->drupalServiceCall = new Node\Expr\StaticCall(new Node\Name\FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); + $classConst = new Node\Expr\ClassConstFetch(new FullyQualified("$namespace\\$hookClassName"), 'class'); + $this->drupalServiceCall = new Node\Expr\StaticCall(new FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); $this->useStmts = []; } } @@ -125,7 +129,7 @@ public function __destruct() do { $candidate = "$className$counter"; $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; - $this->hookClass->name = new Node\Name($candidate); + $this->hookClass->name = new Node\Identifier($candidate); $counter = $counter ? $counter + 1 : 1; } while (file_exists($hookClassFilename)); // Put the file together. @@ -165,6 +169,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return NULL; } + // Convert the function to a method. $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); $method->flags = Class_::MODIFIER_PUBLIC; // Assemble the arguments for the #[Hook] attribute. @@ -172,13 +177,30 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod if ($implementsModule !== $this->module) { $arguments[] = new Node\Arg(new Node\Scalar\String_($implementsModule), name: new Node\Identifier('module')); } - $hookAttribute = new Node\Attribute(new Node\Name('Hook'), $arguments); - $method->attrGroups[] = new Node\AttributeGroup([$hookAttribute]); + $method->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name('Hook'), $arguments)]); return $method; } return NULL; } + /** + * Get the hook and module name from a function name and doxygen. + * + * If the doxygen has Implements hook_foo() in it then this method attempts + * to find a matching module name and hook. Function names like + * user_access_test_user_access() are ambiguous: it could be the user module + * implementing the hook_ENTITY_TYPE_access hook for the access_test_user + * entity type or it could be the user_access_test module implementing it for + * the user entity type. The current module name is preferred by the method + * then the shortest possible module name producing a match is returned. + * + * @param \PhpParser\Node\Stmt\Function_ $node + * A function node. + * + * @return array + * If a match was found then an associative array with keys hook and module + * with corresponding values. Otherwise, the array is empty. + */ protected function getHookAndModuleName(Function_ $node): array { // If the doxygen contains "Implements hook_foo()" then parse the hook @@ -210,7 +232,8 @@ protected function getHookAndModuleName(Function_ $node): array * function. * * @return string - * The function name converted to camelCase for e.g. userUserRoleInsert. + * The function name converted to camelCase for e.g. userRoleInsert. The + * current module name is removed from the beginning. */ protected function getMethodName(Function_ $node): string { @@ -220,12 +243,12 @@ protected function getMethodName(Function_ $node): string public function getLegacyHookFunction(Function_ $node): Function_ { - $args = array_map(fn($param) => $this->nodeFactory->createArg($param->var), $node->getParams()); - $methodCall = $this->nodeFactory->createMethodCall($this->drupalServiceCall, $this->getMethodName($node), $args); - $hasReturn = (new NodeFinder)->findFirstInstanceOf([$node], Return_::class); - $node->stmts = [$hasReturn ? new Return_($methodCall) : new Node\Stmt\Expression($methodCall)]; + $args = array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); + $methodCall = new Node\Expr\MethodCall($this->drupalServiceCall, $this->getMethodName($node), $args); + $hasReturn = (new NodeFinder)->findFirstInstanceOf([$node], Node\Stmt\Return_::class); + $node->stmts = [$hasReturn ? new Node\Stmt\Return_($methodCall) : new Node\Stmt\Expression($methodCall)]; // Mark this function as a legacy hook. - $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name\FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); + $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); return $node; } From bb1bdbb86e998e1ed1f70345038abf6b47df876c Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Mon, 21 Oct 2024 19:18:40 +0200 Subject: [PATCH 18/64] resolve __FUNCTION__ --- src/Rector/Convert/HookConvertRector.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index ce794943..57b3948e 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -7,7 +7,9 @@ use Composer\InstalledVersions; use PhpParser\Node; use PhpParser\Node\Name\FullyQualified; +use PhpParser\Node\Scalar\String_; use PhpParser\NodeTraverser; +use PhpParser\NodeVisitorAbstract; use PhpParser\Node\Stmt\{Use_, Class_, ClassMethod, Function_}; use PhpParser\NodeFinder; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; @@ -169,6 +171,18 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return NULL; } + $functionConstantResolver = new class(new String_($node->name->toString())) extends NodeVisitorAbstract { + public function __construct(protected String_ $name) + { + } + public function leaveNode(Node $node) + { + return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->name : parent::leaveNode($node); + } + }; + $traverser = new NodeTraverser(); + $traverser->addVisitor($functionConstantResolver); + $traverser->traverse([$node]); // Convert the function to a method. $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); $method->flags = Class_::MODIFIER_PUBLIC; From b242183d8fdd0c5941689aac4de27211a0b8912b Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Mon, 21 Oct 2024 19:26:29 +0200 Subject: [PATCH 19/64] remove file doxygen --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 57b3948e..a690e612 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -91,6 +91,7 @@ public function refactor(Node $node): Node|NULL|int // For some unknown reason some Use_ statements are passed twice // to this method. $this->useStmts[$this->printer->prettyPrint([$node])] = $node; + $node->setAttribute('comments', []); } if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { From c71fd2502d54b7372493c4645c17d4d274893a42 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 20:50:33 +0200 Subject: [PATCH 20/64] do not add backslash to true and friends --- src/Rector/Convert/HookConvertRector.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index a690e612..b64ab49c 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -178,6 +178,12 @@ public function __construct(protected String_ $name) } public function leaveNode(Node $node) { + if ($node instanceof Node\Expr\ConstFetch && $node->name instanceof FullyQualified) { + $parts = $node->name->getParts(); + if (count($parts) === 1) { + $node->name = new Node\Name(reset($parts)); + } + } return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->name : parent::leaveNode($node); } }; From 11542681bb06cbecb86bd77e95cbed9a0046e271 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 20:55:53 +0200 Subject: [PATCH 21/64] unqualify everything --- src/Rector/Convert/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index b64ab49c..ec2717c5 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -178,7 +178,7 @@ public function __construct(protected String_ $name) } public function leaveNode(Node $node) { - if ($node instanceof Node\Expr\ConstFetch && $node->name instanceof FullyQualified) { + if (isset($node->name) && $node->name instanceof FullyQualified) { $parts = $node->name->getParts(); if (count($parts) === 1) { $node->name = new Node\Name(reset($parts)); From e8385fae1f79127c06dfc4b1620b3a06f00b5366 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 20:56:27 +0200 Subject: [PATCH 22/64] rename --- src/Rector/Convert/HookConvertRector.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index ec2717c5..8e8ef8c3 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -172,7 +172,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return NULL; } - $functionConstantResolver = new class(new String_($node->name->toString())) extends NodeVisitorAbstract { + $visitor = new class(new String_($node->name->toString())) extends NodeVisitorAbstract { public function __construct(protected String_ $name) { } @@ -188,7 +188,7 @@ public function leaveNode(Node $node) } }; $traverser = new NodeTraverser(); - $traverser->addVisitor($functionConstantResolver); + $traverser->addVisitor($visitor); $traverser->traverse([$node]); // Convert the function to a method. $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); From df749abbbad54f360948da0ef388b9bd6444e560 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 20:57:02 +0200 Subject: [PATCH 23/64] and comment --- src/Rector/Convert/HookConvertRector.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 8e8ef8c3..61a58e14 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -172,6 +172,8 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return NULL; } + // Resolve __FUNCTION__ and unqualify things so TRUE doesn't + // become \TRUE. $visitor = new class(new String_($node->name->toString())) extends NodeVisitorAbstract { public function __construct(protected String_ $name) { From 037b361ba903013963d3cb9609c1e8df4ba29727 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 20:57:49 +0200 Subject: [PATCH 24/64] one more rename --- src/Rector/Convert/HookConvertRector.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 61a58e14..1e59c9ca 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -175,7 +175,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod // Resolve __FUNCTION__ and unqualify things so TRUE doesn't // become \TRUE. $visitor = new class(new String_($node->name->toString())) extends NodeVisitorAbstract { - public function __construct(protected String_ $name) + public function __construct(protected String_ $functionName) { } public function leaveNode(Node $node) @@ -186,7 +186,7 @@ public function leaveNode(Node $node) $node->name = new Node\Name(reset($parts)); } } - return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->name : parent::leaveNode($node); + return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->functionName : parent::leaveNode($node); } }; $traverser = new NodeTraverser(); From 17de8284852f3c21459a0fb3be9982f3cb2120fa Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 21:00:30 +0200 Subject: [PATCH 25/64] stop processing once done --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 1e59c9ca..f71aead9 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -184,6 +184,7 @@ public function leaveNode(Node $node) $parts = $node->name->getParts(); if (count($parts) === 1) { $node->name = new Node\Name(reset($parts)); + return $node; } } return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->functionName : parent::leaveNode($node); From 8fe87f87f7f56ab41b4161eb1dae54ef2221ae83 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 22:20:03 +0200 Subject: [PATCH 26/64] simplify --- src/Rector/Convert/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index f71aead9..24df8dfd 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -183,7 +183,7 @@ public function leaveNode(Node $node) if (isset($node->name) && $node->name instanceof FullyQualified) { $parts = $node->name->getParts(); if (count($parts) === 1) { - $node->name = new Node\Name(reset($parts)); + $node->name = new Node\Name($parts); return $node; } } From 01c3dcfe90b4aa9207c5df0796fa62cb0964f8b8 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 22:21:43 +0200 Subject: [PATCH 27/64] simplify --- src/Rector/Convert/HookConvertRector.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 24df8dfd..7b594d78 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -181,9 +181,9 @@ public function __construct(protected String_ $functionName) public function leaveNode(Node $node) { if (isset($node->name) && $node->name instanceof FullyQualified) { - $parts = $node->name->getParts(); - if (count($parts) === 1) { - $node->name = new Node\Name($parts); + $name = new Node\Name($node->name); + if ($name->isUnqualified()) { + $node->name = $name; return $node; } } From 78b85d5fb4f4b968ed87ba4491054b6825b9ab65 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 23 Oct 2024 23:25:12 +0200 Subject: [PATCH 28/64] move filename to init --- src/Rector/Convert/HookConvertRector.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 7b594d78..c4b1525e 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -114,11 +114,17 @@ protected function initializeHookClass(): void $this->module = basename($infoFile, '.info.yml'); $filename = pathinfo($this->file->getFilePath(), \PATHINFO_FILENAME); $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename) . '_hooks')); + $counter = ''; + do { + $candidate = "$hookClassName$counter"; + $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; + $counter = $counter ? $counter + 1 : 1; + } while (file_exists($hookClassFilename)); $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); - $this->hookClass = new Class_(new Node\Identifier($hookClassName)); + $this->hookClass = new Class_(new Node\Identifier($candidate)); // Using $this->nodeFactory->createStaticCall() results in // use \Drupal; on top which is not desirable. - $classConst = new Node\Expr\ClassConstFetch(new FullyQualified("$namespace\\$hookClassName"), 'class'); + $classConst = new Node\Expr\ClassConstFetch(new FullyQualified("$namespace\\$candidate"), 'class'); $this->drupalServiceCall = new Node\Expr\StaticCall(new FullyQualified('Drupal'), 'service', [new Node\Arg($classConst)]); $this->useStmts = []; } @@ -128,13 +134,6 @@ public function __destruct() { if ($this->module && $this->hookClass->stmts) { $className = $this->hookClass->name->toString(); - $counter = ''; - do { - $candidate = "$className$counter"; - $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; - $this->hookClass->name = new Node\Identifier($candidate); - $counter = $counter ? $counter + 1 : 1; - } while (file_exists($hookClassFilename)); // Put the file together. $namespace = "Drupal\\$this->module\\Hook"; $hookClassStmts = [ @@ -146,7 +145,7 @@ public function __destruct() // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); - file_put_contents($hookClassFilename, $this->printer->prettyPrintFile($hookClassStmts)); + file_put_contents("$this->moduleDir/src/Hook/$className.php", $this->printer->prettyPrintFile($hookClassStmts)); if (!str_starts_with($this->moduleDir, $this->drupalCorePath)) { static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); } From 0e49e604fc7fb500bc0d071bdcea5673328a6176 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 25 Oct 2024 18:02:37 +0200 Subject: [PATCH 29/64] add class doxygen --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index c4b1525e..f9cbda25 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -142,6 +142,7 @@ public function __destruct() new Use_([new Node\Stmt\UseUse(new Node\Name('Drupal\Core\Hook\Attribute\Hook'))]), $this->hookClass, ]; + $this->hookClass->setDocComment(new \PhpParser\Comment\Doc("/**\n * Hook implementations for $this->module.\n */")); // Write it out. @mkdir("$this->moduleDir/src"); @mkdir("$this->moduleDir/src/Hook"); From bd77e4d50642d409c4beca3674dbecb7540565de Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Sat, 26 Oct 2024 00:18:14 +0200 Subject: [PATCH 30/64] not so final, eh? --- src/Rector/Convert/HookConvertRector.php | 28 ++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index f9cbda25..81303009 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -10,10 +10,10 @@ use PhpParser\Node\Scalar\String_; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; +use Rector\PhpParser\Printer\BetterStandardPrinter; use PhpParser\Node\Stmt\{Use_, Class_, ClassMethod, Function_}; use PhpParser\NodeFinder; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; -use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -43,8 +43,32 @@ class HookConvertRector extends AbstractRector private string $drupalCorePath = "\0"; - public function __construct(protected BetterStandardPrinter $printer) + /** + * @var \Rector\PhpParser\Printer\BetterStandardPrinter + */ + private BetterStandardPrinter $printer; + + public function __construct(BetterStandardPrinter $printer) { + if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { + $this->printer = new class extends BetterStandardPrinter { + protected bool $multilineArray = FALSE; + protected function pExpr_Array(Node\Expr\Array_ $array): string + { + $result = $this->multilineArray ? '' : $this->pCommaSeparated($array->items); + if ($this->multilineArray || strlen($result) > 80) { + $multilineArray = $this->multilineArray; + $this->multilineArray = TRUE; + $result = $this->pCommaSeparatedMultiline($array->items, TRUE) . $this->nl; + $this->multilineArray = $multilineArray; + } + return '[' . $result . ']'; + } + }; + } + else { + $this->printer = $printer; + } try { if (class_exists(InstalledVersions::class) && ($corePath = InstalledVersions::getInstallPath('drupal/core'))) { From a12078418a86c0e08973f22fa6dce283be1053e2 Mon Sep 17 00:00:00 2001 From: bjorn Date: Sat, 26 Oct 2024 13:16:59 +0200 Subject: [PATCH 31/64] style: fix codestyle --- src/Rector/Convert/HookConvertRector.php | 125 ++++++++++--------- tests/functional/HookConverRector/rector.php | 13 +- 2 files changed, 73 insertions(+), 65 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 81303009..685a3662 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -8,19 +8,21 @@ use PhpParser\Node; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Function_; +use PhpParser\Node\Stmt\Use_; +use PhpParser\NodeFinder; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; -use Rector\PhpParser\Printer\BetterStandardPrinter; -use PhpParser\Node\Stmt\{Use_, Class_, ClassMethod, Function_}; -use PhpParser\NodeFinder; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; +use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; class HookConvertRector extends AbstractRector { - protected string $inputFilename = ''; /** @@ -44,7 +46,7 @@ class HookConvertRector extends AbstractRector private string $drupalCorePath = "\0"; /** - * @var \Rector\PhpParser\Printer\BetterStandardPrinter + * @var BetterStandardPrinter */ private BetterStandardPrinter $printer; @@ -52,30 +54,30 @@ public function __construct(BetterStandardPrinter $printer) { if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { $this->printer = new class extends BetterStandardPrinter { - protected bool $multilineArray = FALSE; + protected bool $multilineArray = false; + protected function pExpr_Array(Node\Expr\Array_ $array): string { $result = $this->multilineArray ? '' : $this->pCommaSeparated($array->items); if ($this->multilineArray || strlen($result) > 80) { - $multilineArray = $this->multilineArray; - $this->multilineArray = TRUE; - $result = $this->pCommaSeparatedMultiline($array->items, TRUE) . $this->nl; - $this->multilineArray = $multilineArray; + $multilineArray = $this->multilineArray; + $this->multilineArray = true; + $result = $this->pCommaSeparatedMultiline($array->items, true).$this->nl; + $this->multilineArray = $multilineArray; } - return '[' . $result . ']'; + + return '['.$result.']'; } }; - } - else { + } else { $this->printer = $printer; } - try - { + try { if (class_exists(InstalledVersions::class) && ($corePath = InstalledVersions::getInstallPath('drupal/core'))) { $this->drupalCorePath = realpath($corePath); } + } catch (\OutOfBoundsException $e) { } - catch (\OutOfBoundsException $e) { } } public function getRuleDefinition(): RuleDefinition @@ -101,12 +103,12 @@ public function getNodeTypes(): array return [Function_::class, Use_::class]; } - public function refactor(Node $node): Node|NULL|int + public function refactor(Node $node): Node|int|null { $filePath = $this->file->getFilePath(); $ext = pathinfo($filePath, \PATHINFO_EXTENSION); if (!in_array($ext, ['inc', 'module'])) { - return NULL; + return null; } if ($filePath !== $this->inputFilename) { $this->initializeHookClass(); @@ -120,9 +122,11 @@ public function refactor(Node $node): Node|NULL|int if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; + return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); } - return NULL; + + return null; } protected function initializeHookClass(): void @@ -132,17 +136,18 @@ protected function initializeHookClass(): void $this->inputFilename = $this->moduleDir; // Find the relevant info.yml: it's either in the current directory or // one of the parents. - while (($this->moduleDir = dirname($this->moduleDir)) && !($info = glob("$this->moduleDir/*.info.yml"))); + while (($this->moduleDir = dirname($this->moduleDir)) && !($info = glob("$this->moduleDir/*.info.yml"))) { + } if (!empty($info)) { $infoFile = reset($info); $this->module = basename($infoFile, '.info.yml'); $filename = pathinfo($this->file->getFilePath(), \PATHINFO_FILENAME); - $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename) . '_hooks')); + $hookClassName = ucfirst(CaseStringHelper::camelCase(str_replace('.', '_', $filename).'_hooks')); $counter = ''; do { - $candidate = "$hookClassName$counter"; - $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; - $counter = $counter ? $counter + 1 : 1; + $candidate = "$hookClassName$counter"; + $hookClassFilename = "$this->moduleDir/src/Hook/$candidate.php"; + $counter = $counter ? $counter + 1 : 1; } while (file_exists($hookClassFilename)); $namespace = implode('\\', ['Drupal', $this->module, 'Hook']); $this->hookClass = new Class_(new Node\Identifier($candidate)); @@ -162,7 +167,7 @@ public function __destruct() $namespace = "Drupal\\$this->module\\Hook"; $hookClassStmts = [ new Node\Stmt\Namespace_(new Node\Name($namespace)), - ... $this->useStmts, + ...$this->useStmts, new Use_([new Node\Stmt\UseUse(new Node\Name('Drupal\Core\Hook\Attribute\Hook'))]), $this->hookClass, ]; @@ -194,7 +199,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'update_last_removed', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { - return NULL; + return null; } // Resolve __FUNCTION__ and unqualify things so TRUE doesn't // become \TRUE. @@ -202,15 +207,18 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod public function __construct(protected String_ $functionName) { } + public function leaveNode(Node $node) { if (isset($node->name) && $node->name instanceof FullyQualified) { $name = new Node\Name($node->name); if ($name->isUnqualified()) { $node->name = $name; + return $node; } } + return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->functionName : parent::leaveNode($node); } }; @@ -221,34 +229,36 @@ public function leaveNode(Node $node) $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); $method->flags = Class_::MODIFIER_PUBLIC; // Assemble the arguments for the #[Hook] attribute. - $arguments = [new Node\Arg(new Node\Scalar\String_($hook))]; + $arguments = [new Node\Arg(new String_($hook))]; if ($implementsModule !== $this->module) { - $arguments[] = new Node\Arg(new Node\Scalar\String_($implementsModule), name: new Node\Identifier('module')); + $arguments[] = new Node\Arg(new String_($implementsModule), name: new Node\Identifier('module')); } $method->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new Node\Name('Hook'), $arguments)]); + return $method; } - return NULL; + + return null; } - /** - * Get the hook and module name from a function name and doxygen. - * - * If the doxygen has Implements hook_foo() in it then this method attempts - * to find a matching module name and hook. Function names like - * user_access_test_user_access() are ambiguous: it could be the user module - * implementing the hook_ENTITY_TYPE_access hook for the access_test_user - * entity type or it could be the user_access_test module implementing it for - * the user entity type. The current module name is preferred by the method - * then the shortest possible module name producing a match is returned. - * - * @param \PhpParser\Node\Stmt\Function_ $node - * A function node. - * - * @return array - * If a match was found then an associative array with keys hook and module - * with corresponding values. Otherwise, the array is empty. - */ + /** + * Get the hook and module name from a function name and doxygen. + * + * If the doxygen has Implements hook_foo() in it then this method attempts + * to find a matching module name and hook. Function names like + * user_access_test_user_access() are ambiguous: it could be the user module + * implementing the hook_ENTITY_TYPE_access hook for the access_test_user + * entity type or it could be the user_access_test module implementing it for + * the user entity type. The current module name is preferred by the method + * then the shortest possible module name producing a match is returned. + * + * @param Function_ $node + * A function node + * + * @return array + * If a match was found then an associative array with keys hook and module + * with corresponding values. Otherwise, the array is empty. + */ protected function getHookAndModuleName(Function_ $node): array { // If the doxygen contains "Implements hook_foo()" then parse the hook @@ -260,32 +270,34 @@ protected function getHookAndModuleName(Function_ $node): array // If the optional part is present then replace the uppercase // portions with an appropriate regex. if (isset($matches[2])) { - $hookRegex .= '[a-z0-9_]+' . $matches[2]; + $hookRegex .= '[a-z0-9_]+'.$matches[2]; } $hookRegex = "_(?$hookRegex)"; $functionName = $node->name->toString(); // And now find the module and the hook. foreach ([$this->module, '.+?'] as $module) { if (preg_match("/^(?$module)$hookRegex$/", $functionName, $matches)) { - return $matches; + return $matches; } } } + return []; } /** - * @param \PhpParser\Node\Stmt\Function_ $node - * A function declaration for example the entire user_user_role_insert() - * function. + * @param Function_ $node + * A function declaration for example the entire user_user_role_insert() + * function * * @return string - * The function name converted to camelCase for e.g. userRoleInsert. The - * current module name is removed from the beginning. + * The function name converted to camelCase for e.g. userRoleInsert. The + * current module name is removed from the beginning. */ protected function getMethodName(Function_ $node): string { $name = preg_replace("/^{$this->module}_/", '', $node->name->toString()); + return CaseStringHelper::camelCase($name); } @@ -293,10 +305,11 @@ public function getLegacyHookFunction(Function_ $node): Function_ { $args = array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); $methodCall = new Node\Expr\MethodCall($this->drupalServiceCall, $this->getMethodName($node), $args); - $hasReturn = (new NodeFinder)->findFirstInstanceOf([$node], Node\Stmt\Return_::class); + $hasReturn = (new NodeFinder())->findFirstInstanceOf([$node], Node\Stmt\Return_::class); $node->stmts = [$hasReturn ? new Node\Stmt\Return_($methodCall) : new Node\Stmt\Expression($methodCall)]; // Mark this function as a legacy hook. $node->attrGroups[] = new Node\AttributeGroup([new Node\Attribute(new FullyQualified('Drupal\Core\Hook\Attribute\LegacyHook'))]); + return $node; } @@ -312,6 +325,4 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi file_put_contents($fileName, $services); } } - - } diff --git a/tests/functional/HookConverRector/rector.php b/tests/functional/HookConverRector/rector.php index bf76cf80..2b882d5b 100644 --- a/tests/functional/HookConverRector/rector.php +++ b/tests/functional/HookConverRector/rector.php @@ -3,22 +3,19 @@ declare(strict_types=1); use DrupalFinder\DrupalFinder; -use DrupalRector\Set\Drupal10SetList; -use DrupalRector\Set\Drupal8SetList; -use DrupalRector\Set\Drupal9SetList; use Rector\Config\RectorConfig; return static function (RectorConfig $rectorConfig): void { - $rectorConfig->rule(\DrupalRector\Rector\Convert\HookConvertRector::class); + $rectorConfig->rule(DrupalRector\Rector\Convert\HookConvertRector::class); $drupalFinder = new DrupalFinder(); $drupalFinder->locateRoot(__DIR__); $drupalRoot = $drupalFinder->getDrupalRoot(); $rectorConfig->autoloadPaths([ - $drupalRoot . '/core', - $drupalRoot . '/modules', - $drupalRoot . '/profiles', - $drupalRoot . '/themes' + $drupalRoot.'/core', + $drupalRoot.'/modules', + $drupalRoot.'/profiles', + $drupalRoot.'/themes', ]); $rectorConfig->skip(['*/upgrade_status/tests/modules/*']); From 6e6c05685263a1cba8f1185085cfef5d1d4c9063 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Sat, 26 Oct 2024 22:45:41 +0200 Subject: [PATCH 32/64] opsie --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 685a3662..d48b3aef 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -197,6 +197,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'schema', 'uninstall', 'update_last_removed', + 'module_implements_alter' ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From 50a219757c76540acb7ceb656c06b533a516be95 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Mon, 28 Oct 2024 13:10:41 +0100 Subject: [PATCH 33/64] hook_info --- src/Rector/Convert/HookConvertRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index d48b3aef..f1ac1a4f 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -197,7 +197,8 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'schema', 'uninstall', 'update_last_removed', - 'module_implements_alter' + 'module_implements_alter', + 'hook_info', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From c37bef0ada457f2ae0307e8ed56e840d51333286 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Tue, 29 Oct 2024 11:46:33 +0100 Subject: [PATCH 34/64] Two system module functions --- src/Rector/Convert/HookConvertRector.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index f1ac1a4f..ecfbb1ba 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -120,8 +120,18 @@ public function refactor(Node $node): Node|int|null $node->setAttribute('comments', []); } - if ($node instanceof Function_ && $this->module && ($method = $this->createMethodFromFunction($node))) { - $this->hookClass->stmts[] = $method; + if ($node instanceof Function_) { + if ($node->name->toString() === 'system_theme') { + return null; + } + if ($this->module && ($method = $this->createMethodFromFunction($node))) { + $this->hookClass->stmts[] = $method; + if ($node->name->toString() === 'system_page_attachments') { + $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments')))]; + $node->name = new Node\Identifier('_system_page_attachments'); + return $node; + } + } return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); } From 1b3b6f58ae87ea62e7e7a9f0b5ad9d33f9d54119 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Wed, 30 Oct 2024 19:46:57 +0100 Subject: [PATCH 35/64] cache_flush --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index ecfbb1ba..7f10dd9a 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -209,6 +209,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'update_last_removed', 'module_implements_alter', 'hook_info', + 'cache_flush', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From 234b6346da77f56d7c9b32b0ce0e58110de76092 Mon Sep 17 00:00:00 2001 From: nicxvan Date: Wed, 30 Oct 2024 21:15:49 -0400 Subject: [PATCH 36/64] Move return back inside fsecond if --- src/Rector/Convert/HookConvertRector.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 7f10dd9a..b1e3cec9 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -131,9 +131,9 @@ public function refactor(Node $node): Node|int|null $node->name = new Node\Identifier('_system_page_attachments'); return $node; } - } - return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); + return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); + } } return null; From ec0708e6e916792588d7155113ac3ed2dff1c4f1 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Mon, 4 Nov 2024 22:12:26 +0100 Subject: [PATCH 37/64] _system_page_attachments arguments were missing --- src/Rector/Convert/HookConvertRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index b1e3cec9..e60d73d7 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -127,7 +127,8 @@ public function refactor(Node $node): Node|int|null if ($this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; if ($node->name->toString() === 'system_page_attachments') { - $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments')))]; + $args = array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); + $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments'), $args))]; $node->name = new Node\Identifier('_system_page_attachments'); return $node; } From 60e92e1aee79932d125480904ae91b62c1856e35 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Tue, 5 Nov 2024 01:13:26 +0100 Subject: [PATCH 38/64] factor param-arg conversion out --- src/Rector/Convert/HookConvertRector.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index e60d73d7..a47a058b 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -127,8 +127,7 @@ public function refactor(Node $node): Node|int|null if ($this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; if ($node->name->toString() === 'system_page_attachments') { - $args = array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); - $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments'), $args))]; + $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments'), self::convertParamsToArgs($node)))]; $node->name = new Node\Identifier('_system_page_attachments'); return $node; } @@ -317,8 +316,7 @@ protected function getMethodName(Function_ $node): string public function getLegacyHookFunction(Function_ $node): Function_ { - $args = array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); - $methodCall = new Node\Expr\MethodCall($this->drupalServiceCall, $this->getMethodName($node), $args); + $methodCall = new Node\Expr\MethodCall($this->drupalServiceCall, $this->getMethodName($node), self::convertParamsToArgs($node)); $hasReturn = (new NodeFinder())->findFirstInstanceOf([$node], Node\Stmt\Return_::class); $node->stmts = [$hasReturn ? new Node\Stmt\Return_($methodCall) : new Node\Stmt\Expression($methodCall)]; // Mark this function as a legacy hook. @@ -339,4 +337,14 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi file_put_contents($fileName, $services); } } + + /** + * @param \PhpParser\Node\Stmt\Function_ $node + * + * @return \PhpParser\Node\Arg[] + */ + protected static function convertParamsToArgs(Function_ $node): array + { + return array_map(fn(Node\Param $param) => new Node\Arg($param->var), $node->getParams()); + } } From dd1ede016633f642a0a43aa0b2c74da2979878f8 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Mon, 4 Nov 2024 19:52:18 -0500 Subject: [PATCH 39/64] Do not convert system_info_alter --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index a47a058b..81a940eb 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -210,6 +210,7 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'module_implements_alter', 'hook_info', 'cache_flush', + 'system_info_alter', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From ddaa6e9857731462570866b8b05ca23a94cc6775 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Tue, 5 Nov 2024 10:38:33 -0500 Subject: [PATCH 40/64] System info alter is fine --- src/Rector/Convert/HookConvertRector.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 81a940eb..a47a058b 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -210,7 +210,6 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'module_implements_alter', 'hook_info', 'cache_flush', - 'system_info_alter', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From c5b61c31af16921be479d80cb9e51498d2cdee39 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Mon, 11 Nov 2024 09:53:41 -0500 Subject: [PATCH 41/64] Remove installer hooks --- src/Rector/Convert/HookConvertRector.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index a47a058b..b1233f7b 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -199,17 +199,12 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod ['hook' => $hook, 'module' => $implementsModule] = $info; $procOnly = [ 'install', - 'module_preinstall', - 'module_preuninstall', - 'modules_installed', - 'modules_uninstalled', 'requirements', 'schema', 'uninstall', 'update_last_removed', 'module_implements_alter', 'hook_info', - 'cache_flush', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From 678a2ced119cbd730657e40f21d75ba819b1d198 Mon Sep 17 00:00:00 2001 From: nlighteneddesign Date: Wed, 20 Nov 2024 11:09:27 -0500 Subject: [PATCH 42/64] Fix duplicate use --- src/Rector/Convert/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index b1233f7b..324c1d14 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -116,8 +116,8 @@ public function refactor(Node $node): Node|int|null if ($node instanceof Use_) { // For some unknown reason some Use_ statements are passed twice // to this method. - $this->useStmts[$this->printer->prettyPrint([$node])] = $node; $node->setAttribute('comments', []); + $this->useStmts[$this->printer->prettyPrint([$node])] = $node; } if ($node instanceof Function_) { From 48c4c44038e99fb6ad576f1eeff82896a8ed181f Mon Sep 17 00:00:00 2001 From: nLightened Development LLC Date: Mon, 13 Jan 2025 10:05:10 -0500 Subject: [PATCH 43/64] Add install_tasks --- src/Rector/Convert/HookConvertRector.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 324c1d14..31ec262e 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -205,6 +205,8 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'update_last_removed', 'module_implements_alter', 'hook_info', + 'install_tasks', + 'install_tasks_alter', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From 3c7bbd94bc87375c13801359a95916404b7054a7 Mon Sep 17 00:00:00 2001 From: nLightened Development LLC Date: Wed, 15 Jan 2025 14:02:34 -0500 Subject: [PATCH 44/64] Update HookConvertRector.php Comments should stay for contrib --- src/Rector/Convert/HookConvertRector.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 31ec262e..e0c03e73 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -116,7 +116,6 @@ public function refactor(Node $node): Node|int|null if ($node instanceof Use_) { // For some unknown reason some Use_ statements are passed twice // to this method. - $node->setAttribute('comments', []); $this->useStmts[$this->printer->prettyPrint([$node])] = $node; } From b8dc5dd6406336171a6fee95c31be233e25bff7c Mon Sep 17 00:00:00 2001 From: nLightened Development LLC Date: Wed, 15 Jan 2025 15:33:05 -0500 Subject: [PATCH 45/64] Update HookConvertRector.php Fix comment and duplicate use statement --- src/Rector/Convert/HookConvertRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index e0c03e73..70ffc907 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -116,7 +116,8 @@ public function refactor(Node $node): Node|int|null if ($node instanceof Use_) { // For some unknown reason some Use_ statements are passed twice // to this method. - $this->useStmts[$this->printer->prettyPrint([$node])] = $node; + $newNode = new Use_($node->uses, $node->type, ['comments' => []] + $node->getAttributes()); + $this->useStmts[$this->printer->prettyPrint([$newNode])] = $newNode; } if ($node instanceof Function_) { From abf275e7ea3a17aa9f3b56d248537f445e4d3752 Mon Sep 17 00:00:00 2001 From: nLightened Development LLC Date: Wed, 15 Jan 2025 16:53:05 -0500 Subject: [PATCH 46/64] Ensure theme_suggestions are picked up --- src/Rector/Convert/HookConvertRector.php | 25 ++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 70ffc907..a559cc37 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -205,8 +205,6 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'update_last_removed', 'module_implements_alter', 'hook_info', - 'install_tasks', - 'install_tasks_alter', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; @@ -275,13 +273,24 @@ protected function getHookAndModuleName(Function_ $node): array // name. A difficulty here is "Implements hook_form_FORM_ID_alter". // Find these by looking for an optional part starting with an // uppercase letter. - if (preg_match('/^ \* Implements hook_([a-z0-9_]*)(?:[A-Z][A-Z0-9_]+(_[a-z0-9_]*))?/m', (string) $node->getDocComment()?->getReformattedText(), $matches)) { - $hookRegex = $matches[1]; - // If the optional part is present then replace the uppercase - // portions with an appropriate regex. - if (isset($matches[2])) { - $hookRegex .= '[a-z0-9_]+'.$matches[2]; + if (preg_match('/^ \* Implements hook_([a-zA-Z0-9_]+)/m', (string) $node->getDocComment()?->getReformattedText(), $matches)) { + $parts = explode('_', $matches[1]); + $isUppercase = FALSE; + foreach ($parts as &$part) { + if (!$part) { + continue; + } + if ($part === strtoupper($part)) { + if (!$isUppercase) { + $isUppercase = TRUE; + $part = '[a-z0-9_]+'; + } + } + else { + $isUpperCase = FALSE; + } } + $hookRegex = implode('_', $parts); $hookRegex = "_(?$hookRegex)"; $functionName = $node->name->toString(); // And now find the module and the hook. From c000df66477dcc534295c25113b909d09c9696b8 Mon Sep 17 00:00:00 2001 From: nLightened Development LLC Date: Wed, 15 Jan 2025 16:54:06 -0500 Subject: [PATCH 47/64] Install tasks --- src/Rector/Convert/HookConvertRector.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index a559cc37..f55b7055 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -205,6 +205,8 @@ protected function createMethodFromFunction(Function_ $node): ?ClassMethod 'update_last_removed', 'module_implements_alter', 'hook_info', + 'install_tasks', + 'install_tasks_alter', ]; if (in_array($hook, $procOnly) || str_starts_with($hook, 'preprocess') || str_starts_with($hook, 'process')) { return null; From 8c0e454bb1bdf24bf8310561e5e4315b6038f618 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 15:04:24 +0100 Subject: [PATCH 48/64] build: fix codestyle --- src/Drupal8/Rector/Deprecation/DBRector.php | 2 +- .../Deprecation/DrupalServiceRenameRector.php | 2 +- .../Rector/Deprecation/EntityLoadRector.php | 2 +- .../Deprecation/ExtensionPathRector.php | 2 +- .../UiHelperTraitDrupalPostFormRector.php | 2 +- src/Rector/AbstractDrupalCoreRector.php | 2 +- src/Rector/Convert/HookConvertRector.php | 30 +++++++++---------- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Drupal8/Rector/Deprecation/DBRector.php b/src/Drupal8/Rector/Deprecation/DBRector.php index 4963b5f7..d4c6df70 100644 --- a/src/Drupal8/Rector/Deprecation/DBRector.php +++ b/src/Drupal8/Rector/Deprecation/DBRector.php @@ -56,7 +56,7 @@ class DBRector extends AbstractRector implements ConfigurableRectorInterface protected $optionsArgumentPosition; /** - * @var \DrupalRector\Drupal8\Rector\ValueObject\DBConfiguration[] + * @var DBConfiguration[] */ private array $configuration; diff --git a/src/Drupal8/Rector/Deprecation/DrupalServiceRenameRector.php b/src/Drupal8/Rector/Deprecation/DrupalServiceRenameRector.php index 43726353..61b17330 100644 --- a/src/Drupal8/Rector/Deprecation/DrupalServiceRenameRector.php +++ b/src/Drupal8/Rector/Deprecation/DrupalServiceRenameRector.php @@ -14,7 +14,7 @@ class DrupalServiceRenameRector extends AbstractRector implements ConfigurableRectorInterface { /** - * @var \DrupalRector\Drupal8\Rector\ValueObject\DrupalServiceRenameConfiguration[] + * @var DrupalServiceRenameConfiguration[] */ protected array $staticArgumentRenameConfigs = []; diff --git a/src/Drupal8/Rector/Deprecation/EntityLoadRector.php b/src/Drupal8/Rector/Deprecation/EntityLoadRector.php index 7c3bb7fa..66b5e5b5 100644 --- a/src/Drupal8/Rector/Deprecation/EntityLoadRector.php +++ b/src/Drupal8/Rector/Deprecation/EntityLoadRector.php @@ -27,7 +27,7 @@ final class EntityLoadRector extends AbstractRector implements ConfigurableRectorInterface { /** - * @var \DrupalRector\Drupal8\Rector\ValueObject\EntityLoadConfiguration[] + * @var EntityLoadConfiguration[] */ protected array $entityTypes; diff --git a/src/Drupal9/Rector/Deprecation/ExtensionPathRector.php b/src/Drupal9/Rector/Deprecation/ExtensionPathRector.php index e0d9ca33..a8043d94 100644 --- a/src/Drupal9/Rector/Deprecation/ExtensionPathRector.php +++ b/src/Drupal9/Rector/Deprecation/ExtensionPathRector.php @@ -15,7 +15,7 @@ class ExtensionPathRector extends AbstractRector implements ConfigurableRectorInterface { /** - * @var \DrupalRector\Drupal9\Rector\ValueObject\ExtensionPathConfiguration[] + * @var ExtensionPathConfiguration[] */ private array $configuration; diff --git a/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php b/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php index 208f4ebf..9a3462ee 100644 --- a/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php +++ b/src/Drupal9/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php @@ -48,7 +48,7 @@ public function getNodeTypes(): array * * @throws ShouldNotHappenException * - * @return array + * @return array */ private function safeArgDestructure(Node\Expr\MethodCall $node): array { diff --git a/src/Rector/AbstractDrupalCoreRector.php b/src/Rector/AbstractDrupalCoreRector.php index 89ac7c5b..8edfb174 100644 --- a/src/Rector/AbstractDrupalCoreRector.php +++ b/src/Rector/AbstractDrupalCoreRector.php @@ -16,7 +16,7 @@ abstract class AbstractDrupalCoreRector extends AbstractRector implements ConfigurableRectorInterface { /** - * @var array|\DrupalRector\Contract\VersionedConfigurationInterface[] + * @var array|VersionedConfigurationInterface[] */ protected array $configuration = []; diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index f55b7055..23ef029e 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -129,6 +129,7 @@ public function refactor(Node $node): Node|int|null if ($node->name->toString() === 'system_page_attachments') { $method->stmts = [new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name('_system_page_attachments'), self::convertParamsToArgs($node)))]; $node->name = new Node\Identifier('_system_page_attachments'); + return $node; } @@ -277,20 +278,19 @@ protected function getHookAndModuleName(Function_ $node): array // uppercase letter. if (preg_match('/^ \* Implements hook_([a-zA-Z0-9_]+)/m', (string) $node->getDocComment()?->getReformattedText(), $matches)) { $parts = explode('_', $matches[1]); - $isUppercase = FALSE; + $isUppercase = false; foreach ($parts as &$part) { if (!$part) { continue; } if ($part === strtoupper($part)) { if (!$isUppercase) { - $isUppercase = TRUE; + $isUppercase = true; $part = '[a-z0-9_]+'; } - } - else { - $isUpperCase = FALSE; - } + } else { + $isUpperCase = false; + } } $hookRegex = implode('_', $parts); $hookRegex = "_(?$hookRegex)"; @@ -346,13 +346,13 @@ protected static function writeServicesYml(string $fileName, string $fullyClassi } } - /** - * @param \PhpParser\Node\Stmt\Function_ $node - * - * @return \PhpParser\Node\Arg[] - */ - protected static function convertParamsToArgs(Function_ $node): array - { - return array_map(fn(Node\Param $param) => new Node\Arg($param->var), $node->getParams()); - } + /** + * @param Function_ $node + * + * @return Node\Arg[] + */ + protected static function convertParamsToArgs(Function_ $node): array + { + return array_map(fn (Node\Param $param) => new Node\Arg($param->var), $node->getParams()); + } } From 81ce5f3d754bd46dc0aab425be0037b33d902a73 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:06:59 +0100 Subject: [PATCH 49/64] fix: handle dry-run properly --- src/Rector/Convert/HookConvertRector.php | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 23ef029e..18e97c8b 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -15,6 +15,7 @@ use PhpParser\NodeFinder; use PhpParser\NodeTraverser; use PhpParser\NodeVisitorAbstract; +use Rector\Configuration\Option; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; @@ -50,8 +51,12 @@ class HookConvertRector extends AbstractRector */ private BetterStandardPrinter $printer; + private bool $isDryRun; + public function __construct(BetterStandardPrinter $printer) { + $this->isDryRun = !in_array(['--'.Option::DRY_RUN, '-'.Option::DRY_RUN_SHORT], $_SERVER['argv'] ?? []); + if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { $this->printer = new class extends BetterStandardPrinter { protected bool $multilineArray = false; @@ -183,12 +188,15 @@ public function __destruct() $this->hookClass, ]; $this->hookClass->setDocComment(new \PhpParser\Comment\Doc("/**\n * Hook implementations for $this->module.\n */")); - // Write it out. - @mkdir("$this->moduleDir/src"); - @mkdir("$this->moduleDir/src/Hook"); - file_put_contents("$this->moduleDir/src/Hook/$className.php", $this->printer->prettyPrintFile($hookClassStmts)); - if (!str_starts_with($this->moduleDir, $this->drupalCorePath)) { - static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); + // Write it out if not a dry run + if ($this->isDryRun === false) { + @mkdir("$this->moduleDir/src"); + @mkdir("$this->moduleDir/src/Hook"); + + file_put_contents("$this->moduleDir/src/Hook/$className.php", $this->printer->prettyPrintFile($hookClassStmts)); + if (!str_starts_with($this->moduleDir, $this->drupalCorePath)) { + static::writeServicesYml("$this->moduleDir/$this->module.services.yml", "$namespace\\$className"); + } } } $this->module = ''; From ba7790657dc2d069c1670ca929c56c92eb9aea12 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:08:22 +0100 Subject: [PATCH 50/64] fix: rework tests to work with a module and depending files --- .../functional_test__single_rectors.yml | 14 +++++++----- .../fixture/module/module.module | 7 ------ .../fixture/module/module.info.yml | 0 .../fixture/module/module.module | 10 +++++++++ .../module_updated/Hook/ModuleHooks.php | 22 +++++++++++++++++++ .../fixture/module_updated/module.info.yml | 0 .../fixture/module_updated/module.module | 0 .../module_updated/module.services.yml | 5 +++++ .../rector.php | 0 9 files changed, 45 insertions(+), 13 deletions(-) delete mode 100644 tests/functional/HookConverRector/fixture/module/module.module create mode 100644 tests/functional/HookConvertRector/fixture/module/module.info.yml create mode 100644 tests/functional/HookConvertRector/fixture/module/module.module create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/module.info.yml rename tests/functional/{HookConverRector => HookConvertRector}/fixture/module_updated/module.module (100%) create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/module.services.yml rename tests/functional/{HookConverRector => HookConvertRector}/rector.php (100%) diff --git a/.github/workflows/functional_test__single_rectors.yml b/.github/workflows/functional_test__single_rectors.yml index c88a0951..aca1fa89 100644 --- a/.github/workflows/functional_test__single_rectors.yml +++ b/.github/workflows/functional_test__single_rectors.yml @@ -13,7 +13,7 @@ on: jobs: run_functional_test_single: - name: Functional Test | single rectors" + name: Functional Test | single rectors runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -36,7 +36,7 @@ jobs: run: | cd ~/drupal mkdir -p web/modules/custom - cp -R vendor/palantirnet/drupal-rector/tests/functional/* web/modules/custom + cp -R vendor/palantirnet/drupal-rector/tests/functional/HookConvertRector/fixture/module web/modules/custom/HookConvertRector # dry-run is expected to return exit code 2 if there are changes, which we are expecting to happen, here. # an error code of 1 represents other errors. # @see \Rector\Core\Console\ExitCode::CHANGED_CODE @@ -45,8 +45,9 @@ jobs: cd ~/drupal for d in web/modules/custom/*; do if [ -d "$d" ]; then - cp ${d}/rector.php . - vendor/bin/rector process $d/fixture/module --dry-run --debug || if (($? == 2)); then true; else exit 1; fi + echo "Processing $d" + cp vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/rector.php . + vendor/bin/rector process $d/fixture/module -vvv --dry-run --debug || if (($? == 2)); then true; else exit 1; fi fi done - name: Run rector against Drupal @@ -54,7 +55,8 @@ jobs: cd ~/drupal for d in web/modules/custom/*; do if [ -d "$d" ]; then - cp ${d}/rector.php . + echo "Processing $d" + cp vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/rector.php . vendor/bin/rector process $d/fixture/module --debug fi done @@ -68,6 +70,6 @@ jobs: cd ~/drupal for d in web/modules/custom/*; do if [ -d "$d" ]; then - diff -rubB "$d/fixture/module" "$d/fixture/module_updated" + diff -rubB "$d" "vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/fixture/module_updated" fi done diff --git a/tests/functional/HookConverRector/fixture/module/module.module b/tests/functional/HookConverRector/fixture/module/module.module deleted file mode 100644 index 68de33b3..00000000 --- a/tests/functional/HookConverRector/fixture/module/module.module +++ /dev/null @@ -1,7 +0,0 @@ - diff --git a/tests/functional/HookConvertRector/fixture/module/module.info.yml b/tests/functional/HookConvertRector/fixture/module/module.info.yml new file mode 100644 index 00000000..e69de29b diff --git a/tests/functional/HookConvertRector/fixture/module/module.module b/tests/functional/HookConvertRector/fixture/module/module.module new file mode 100644 index 00000000..0249627c --- /dev/null +++ b/tests/functional/HookConvertRector/fixture/module/module.module @@ -0,0 +1,10 @@ + diff --git a/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php b/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php new file mode 100644 index 00000000..2dbbe0a8 --- /dev/null +++ b/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php @@ -0,0 +1,22 @@ + Date: Fri, 17 Jan 2025 16:24:21 +0100 Subject: [PATCH 51/64] fix: invert condition --- src/Rector/Convert/HookConvertRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 18e97c8b..44f4c57b 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -55,7 +55,8 @@ class HookConvertRector extends AbstractRector public function __construct(BetterStandardPrinter $printer) { - $this->isDryRun = !in_array(['--'.Option::DRY_RUN, '-'.Option::DRY_RUN_SHORT], $_SERVER['argv'] ?? []); + $options = ['--' . Option::DRY_RUN, '-' . Option::DRY_RUN_SHORT]; + $this->isDryRun = in_array($options, $_SERVER['argv'] ?? []); if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { $this->printer = new class extends BetterStandardPrinter { From 8aa9fa8dcc601e7dda778f604a7a70748ba95b87 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:24:28 +0100 Subject: [PATCH 52/64] chore: rename module --- .../module/{module.info.yml => hookconvertrector.info.yml} | 0 .../fixture/module/{module.module => hookconvertrector.module} | 2 -- 2 files changed, 2 deletions(-) rename tests/functional/HookConvertRector/fixture/module/{module.info.yml => hookconvertrector.info.yml} (100%) rename tests/functional/HookConvertRector/fixture/module/{module.module => hookconvertrector.module} (97%) diff --git a/tests/functional/HookConvertRector/fixture/module/module.info.yml b/tests/functional/HookConvertRector/fixture/module/hookconvertrector.info.yml similarity index 100% rename from tests/functional/HookConvertRector/fixture/module/module.info.yml rename to tests/functional/HookConvertRector/fixture/module/hookconvertrector.info.yml diff --git a/tests/functional/HookConvertRector/fixture/module/module.module b/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module similarity index 97% rename from tests/functional/HookConvertRector/fixture/module/module.module rename to tests/functional/HookConvertRector/fixture/module/hookconvertrector.module index 0249627c..8e8aaf81 100644 --- a/tests/functional/HookConvertRector/fixture/module/module.module +++ b/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module @@ -6,5 +6,3 @@ function module_user_cancel($edit, UserInterface $account, $method) { $red = 'red'; } - -?> From 006d1582e6f7e8cdf81204cbe58276b41ad15013 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:26:16 +0100 Subject: [PATCH 53/64] fix: rename module to something more uniwue --- .../module_updated/Hook/ModuleHooks.php | 22 ------------------- ...le.info.yml => hookconvertrector.info.yml} | 0 .../module_updated/hookconvertrector.module | 12 ++++++++++ .../hookconvertrector.services.yml | 5 +++++ .../fixture/module_updated/module.module | 9 -------- .../module_updated/module.services.yml | 5 ----- .../src/Hook/HookconvertrectorHooks.php | 19 ++++++++++++++++ 7 files changed, 36 insertions(+), 36 deletions(-) delete mode 100644 tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php rename tests/functional/HookConvertRector/fixture/module_updated/{module.info.yml => hookconvertrector.info.yml} (100%) create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml delete mode 100644 tests/functional/HookConvertRector/fixture/module_updated/module.module delete mode 100644 tests/functional/HookConvertRector/fixture/module_updated/module.services.yml create mode 100644 tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php diff --git a/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php b/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php deleted file mode 100644 index 2dbbe0a8..00000000 --- a/tests/functional/HookConvertRector/fixture/module_updated/Hook/ModuleHooks.php +++ /dev/null @@ -1,22 +0,0 @@ -moduleUserCancel($edit, $account, $method); +} diff --git a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml b/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml new file mode 100644 index 00000000..3be2c759 --- /dev/null +++ b/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml @@ -0,0 +1,5 @@ + +services: + Drupal\hookconvertrector\Hook\HookconvertrectorHooks: + class: Drupal\hookconvertrector\Hook\HookconvertrectorHooks + autowire: true diff --git a/tests/functional/HookConvertRector/fixture/module_updated/module.module b/tests/functional/HookConvertRector/fixture/module_updated/module.module deleted file mode 100644 index b5958e7d..00000000 --- a/tests/functional/HookConvertRector/fixture/module_updated/module.module +++ /dev/null @@ -1,9 +0,0 @@ - diff --git a/tests/functional/HookConvertRector/fixture/module_updated/module.services.yml b/tests/functional/HookConvertRector/fixture/module_updated/module.services.yml deleted file mode 100644 index f98ecdeb..00000000 --- a/tests/functional/HookConvertRector/fixture/module_updated/module.services.yml +++ /dev/null @@ -1,5 +0,0 @@ - -services: - Drupal\module\Hook\ModuleHooks: - class: Drupal\module\Hook\ModuleHooks - autowire: true diff --git a/tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php b/tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php new file mode 100644 index 00000000..1e3ce41a --- /dev/null +++ b/tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php @@ -0,0 +1,19 @@ + Date: Fri, 17 Jan 2025 16:26:54 +0100 Subject: [PATCH 54/64] build: fix stle --- src/Rector/Convert/HookConvertRector.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 44f4c57b..c9b3aeab 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -55,7 +55,7 @@ class HookConvertRector extends AbstractRector public function __construct(BetterStandardPrinter $printer) { - $options = ['--' . Option::DRY_RUN, '-' . Option::DRY_RUN_SHORT]; + $options = ['--'.Option::DRY_RUN, '-'.Option::DRY_RUN_SHORT]; $this->isDryRun = in_array($options, $_SERVER['argv'] ?? []); if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { From 0bcce1bb86327efcb65d7124ecf29c197442e09d Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:38:02 +0100 Subject: [PATCH 55/64] fix: rename hook function name --- .../HookConvertRector/fixture/module/hookconvertrector.module | 2 +- .../fixture/module_updated/hookconvertrector.module | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module b/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module index 8e8aaf81..cf23b018 100644 --- a/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module +++ b/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module @@ -3,6 +3,6 @@ /** * Implements hook_user_cancel(). */ -function module_user_cancel($edit, UserInterface $account, $method) { +function hookconvertrector_user_cancel($edit, UserInterface $account, $method) { $red = 'red'; } diff --git a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module b/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module index 52f00844..19491f24 100644 --- a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module +++ b/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module @@ -7,6 +7,6 @@ use Drupal\hookconvertrector\Hook\HookconvertrectorHooks; * Implements hook_user_cancel(). */ #[LegacyHook] -function module_user_cancel($edit, UserInterface $account, $method) { +function hookconvertrector_user_cancel($edit, UserInterface $account, $method) { \Drupal::service(HookconvertrectorHooks::class)->moduleUserCancel($edit, $account, $method); } From 17ba33cdb1414bf943da95980a4fa8888332f5e7 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:45:17 +0100 Subject: [PATCH 56/64] build: fix codestyle --- .php-cs-fixer.dist.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index e045696e..0ac9d1c3 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -6,6 +6,9 @@ __DIR__.'/tests', __DIR__.'/config/drupal-*', ]) + ->exclude([ + 'functional/HookConvertRector/fixture/module_updated', + ]) ; return (new PhpCsFixer\Config()) From b453e4993b4e6b7c95330730c96601c843bd4e26 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 16:58:22 +0100 Subject: [PATCH 57/64] fix: try and fix tests directory stuff --- .github/workflows/functional_test__single_rectors.yml | 8 ++++---- src/Rector/Convert/HookConvertRector.php | 3 +-- .../fixture/hookconvertrector}/hookconvertrector.info.yml | 0 .../fixture/hookconvertrector}/hookconvertrector.module | 0 .../hookconvertrector_updated}/hookconvertrector.info.yml | 0 .../hookconvertrector_updated}/hookconvertrector.module | 0 .../hookconvertrector.services.yml | 0 .../src/Hook/HookconvertrectorHooks.php | 0 .../{HookConvertRector => hookconvertrector}/rector.php | 0 9 files changed, 5 insertions(+), 6 deletions(-) rename tests/functional/{HookConvertRector/fixture/module => hookconvertrector/fixture/hookconvertrector}/hookconvertrector.info.yml (100%) rename tests/functional/{HookConvertRector/fixture/module => hookconvertrector/fixture/hookconvertrector}/hookconvertrector.module (100%) rename tests/functional/{HookConvertRector/fixture/module_updated => hookconvertrector/fixture/hookconvertrector_updated}/hookconvertrector.info.yml (100%) rename tests/functional/{HookConvertRector/fixture/module_updated => hookconvertrector/fixture/hookconvertrector_updated}/hookconvertrector.module (100%) rename tests/functional/{HookConvertRector/fixture/module_updated => hookconvertrector/fixture/hookconvertrector_updated}/hookconvertrector.services.yml (100%) rename tests/functional/{HookConvertRector/fixture/module_updated => hookconvertrector/fixture/hookconvertrector_updated}/src/Hook/HookconvertrectorHooks.php (100%) rename tests/functional/{HookConvertRector => hookconvertrector}/rector.php (100%) diff --git a/.github/workflows/functional_test__single_rectors.yml b/.github/workflows/functional_test__single_rectors.yml index aca1fa89..85082e9d 100644 --- a/.github/workflows/functional_test__single_rectors.yml +++ b/.github/workflows/functional_test__single_rectors.yml @@ -36,7 +36,7 @@ jobs: run: | cd ~/drupal mkdir -p web/modules/custom - cp -R vendor/palantirnet/drupal-rector/tests/functional/HookConvertRector/fixture/module web/modules/custom/HookConvertRector + cp -R vendor/palantirnet/drupal-rector/tests/functional/hookconvertrector/fixture/hookconvertrector web/modules/custom/hookconvertrector # dry-run is expected to return exit code 2 if there are changes, which we are expecting to happen, here. # an error code of 1 represents other errors. # @see \Rector\Core\Console\ExitCode::CHANGED_CODE @@ -47,7 +47,7 @@ jobs: if [ -d "$d" ]; then echo "Processing $d" cp vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/rector.php . - vendor/bin/rector process $d/fixture/module -vvv --dry-run --debug || if (($? == 2)); then true; else exit 1; fi + vendor/bin/rector process $d -vvv --dry-run --debug || if (($? == 2)); then true; else exit 1; fi fi done - name: Run rector against Drupal @@ -57,7 +57,7 @@ jobs: if [ -d "$d" ]; then echo "Processing $d" cp vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/rector.php . - vendor/bin/rector process $d/fixture/module --debug + vendor/bin/rector process $d --debug fi done # diff options: @@ -70,6 +70,6 @@ jobs: cd ~/drupal for d in web/modules/custom/*; do if [ -d "$d" ]; then - diff -rubB "$d" "vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/fixture/module_updated" + diff -rubB "$d" "vendor/palantirnet/drupal-rector/tests/functional/$(basename ${d})/fixture/$(basename ${d})_updated" fi done diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index c9b3aeab..18b27b22 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -55,8 +55,7 @@ class HookConvertRector extends AbstractRector public function __construct(BetterStandardPrinter $printer) { - $options = ['--'.Option::DRY_RUN, '-'.Option::DRY_RUN_SHORT]; - $this->isDryRun = in_array($options, $_SERVER['argv'] ?? []); + $this->isDryRun = in_array('--'.Option::DRY_RUN, $_SERVER['argv'] ?? []) || in_array('-'.Option::DRY_RUN_SHORT, $_SERVER['argv'] ?? []); if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { $this->printer = new class extends BetterStandardPrinter { diff --git a/tests/functional/HookConvertRector/fixture/module/hookconvertrector.info.yml b/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.info.yml similarity index 100% rename from tests/functional/HookConvertRector/fixture/module/hookconvertrector.info.yml rename to tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.info.yml diff --git a/tests/functional/HookConvertRector/fixture/module/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module similarity index 100% rename from tests/functional/HookConvertRector/fixture/module/hookconvertrector.module rename to tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module diff --git a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.info.yml b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.info.yml similarity index 100% rename from tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.info.yml rename to tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.info.yml diff --git a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module similarity index 100% rename from tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.module rename to tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module diff --git a/tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.services.yml similarity index 100% rename from tests/functional/HookConvertRector/fixture/module_updated/hookconvertrector.services.yml rename to tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.services.yml diff --git a/tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php similarity index 100% rename from tests/functional/HookConvertRector/fixture/module_updated/src/Hook/HookconvertrectorHooks.php rename to tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php diff --git a/tests/functional/HookConvertRector/rector.php b/tests/functional/hookconvertrector/rector.php similarity index 100% rename from tests/functional/HookConvertRector/rector.php rename to tests/functional/hookconvertrector/rector.php From 895c55bb44837d97721493f9edb1386525dc8ec0 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 17:00:59 +0100 Subject: [PATCH 58/64] build: update php cs config --- .php-cs-fixer.dist.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 0ac9d1c3..64f7498a 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -7,7 +7,7 @@ __DIR__.'/config/drupal-*', ]) ->exclude([ - 'functional/HookConvertRector/fixture/module_updated', + 'functional/hookconvertrector/fixture', ]) ; From 620b8dc43b2bf746020909423e9005960fd04c91 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 17:01:06 +0100 Subject: [PATCH 59/64] fix: update expected --- .../hookconvertrector_updated/hookconvertrector.module | 2 +- .../src/Hook/HookconvertrectorHooks.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module index 19491f24..865909f1 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module @@ -8,5 +8,5 @@ use Drupal\hookconvertrector\Hook\HookconvertrectorHooks; */ #[LegacyHook] function hookconvertrector_user_cancel($edit, UserInterface $account, $method) { - \Drupal::service(HookconvertrectorHooks::class)->moduleUserCancel($edit, $account, $method); + \Drupal::service(HookconvertrectorHooks::class)->userCancel($edit, $account, $method); } diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php index 1e3ce41a..8401ea21 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php @@ -11,8 +11,8 @@ class HookconvertrectorHooks /** * Implements hook_user_cancel(). */ - #[Hook('user_cancel', module: 'module')] - public function moduleUserCancel($edit, \UserInterface $account, $method) + #[Hook('user_cancel')] + public function userCancel($edit, \UserInterface $account, $method) { $red = 'red'; } From 29323fc39b885234da5a1435a1f536a985012fcd Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 17:12:21 +0100 Subject: [PATCH 60/64] fix: fix deprecations --- src/Rector/Convert/HookConvertRector.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 18b27b22..a86ab496 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -5,6 +5,7 @@ namespace DrupalRector\Rector\Convert; use Composer\InstalledVersions; +use PhpParser\Modifiers; use PhpParser\Node; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Scalar\String_; @@ -14,6 +15,7 @@ use PhpParser\Node\Stmt\Use_; use PhpParser\NodeFinder; use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor; use PhpParser\NodeVisitorAbstract; use Rector\Configuration\Option; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; @@ -138,7 +140,7 @@ public function refactor(Node $node): Node|int|null return $node; } - return str_starts_with($filePath, $this->drupalCorePath) ? NodeTraverser::REMOVE_NODE : $this->getLegacyHookFunction($node); + return str_starts_with($filePath, $this->drupalCorePath) ? NodeVisitor::REMOVE_NODE : $this->getLegacyHookFunction($node); } } @@ -246,7 +248,7 @@ public function leaveNode(Node $node) $traverser->traverse([$node]); // Convert the function to a method. $method = new ClassMethod($this->getMethodName($node), get_object_vars($node), $node->getAttributes()); - $method->flags = Class_::MODIFIER_PUBLIC; + $method->flags = Modifiers::PUBLIC; // Assemble the arguments for the #[Hook] attribute. $arguments = [new Node\Arg(new String_($hook))]; if ($implementsModule !== $this->module) { @@ -274,7 +276,7 @@ public function leaveNode(Node $node) * @param Function_ $node * A function node * - * @return array + * @return array * If a match was found then an associative array with keys hook and module * with corresponding values. Otherwise, the array is empty. */ From 0321be7360aba763c92fa3c2b0590a183e8b2059 Mon Sep 17 00:00:00 2001 From: bjorn Date: Fri, 17 Jan 2025 17:13:21 +0100 Subject: [PATCH 61/64] build: fix style --- src/Rector/Convert/HookConvertRector.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index a86ab496..128d0938 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -277,8 +277,8 @@ public function leaveNode(Node $node) * A function node * * @return array - * If a match was found then an associative array with keys hook and module - * with corresponding values. Otherwise, the array is empty. + * If a match was found then an associative array with keys hook and module + * with corresponding values. Otherwise, the array is empty. */ protected function getHookAndModuleName(Function_ $node): array { From 7dc67e0bff1f973a3af5d1482db2ac94a0f57a33 Mon Sep 17 00:00:00 2001 From: bjorn Date: Sat, 18 Jan 2025 21:54:52 +0100 Subject: [PATCH 62/64] Add more tests and remove the need for the custom printer. --- src/Rector/Convert/HookConvertRector.php | 36 +++++----- .../hookconvertrector.module | 69 +++++++++++++++++++ .../hookconvertrector.module | 27 +++++++- .../src/Hook/HookconvertrectorHooks.php | 61 ++++++++++++++++ 4 files changed, 172 insertions(+), 21 deletions(-) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index 128d0938..cc37a174 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -58,27 +58,8 @@ class HookConvertRector extends AbstractRector public function __construct(BetterStandardPrinter $printer) { $this->isDryRun = in_array('--'.Option::DRY_RUN, $_SERVER['argv'] ?? []) || in_array('-'.Option::DRY_RUN_SHORT, $_SERVER['argv'] ?? []); + $this->printer = $printer; - if (!(new \ReflectionClass(BetterStandardPrinter::class))->isFinal()) { - $this->printer = new class extends BetterStandardPrinter { - protected bool $multilineArray = false; - - protected function pExpr_Array(Node\Expr\Array_ $array): string - { - $result = $this->multilineArray ? '' : $this->pCommaSeparated($array->items); - if ($this->multilineArray || strlen($result) > 80) { - $multilineArray = $this->multilineArray; - $this->multilineArray = true; - $result = $this->pCommaSeparatedMultiline($array->items, true).$this->nl; - $this->multilineArray = $multilineArray; - } - - return '['.$result.']'; - } - }; - } else { - $this->printer = $printer; - } try { if (class_exists(InstalledVersions::class) && ($corePath = InstalledVersions::getInstallPath('drupal/core'))) { $this->drupalCorePath = realpath($corePath); @@ -131,6 +112,17 @@ public function refactor(Node $node): Node|int|null if ($node->name->toString() === 'system_theme') { return null; } + /* + @todo Something like this should fix the issue with the legacy hooks + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attribute) { + if (str_ends_with($this->nodeNameResolver->getName($attribute->name), 'LegacyHook')) { + return null; + } + } + } + */ + if ($this->module && ($method = $this->createMethodFromFunction($node))) { $this->hookClass->stmts[] = $method; if ($node->name->toString() === 'system_page_attachments') { @@ -240,6 +232,10 @@ public function leaveNode(Node $node) } } + if ($node instanceof Node\Expr\Array_) { + $node->setAttribute(AttributeKey::NEWLINED_ARRAY_PRINT, true); + } + return $node instanceof Node\Scalar\MagicConst\Function_ ? $this->functionName : parent::leaveNode($node); } }; diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module index cf23b018..cc0e1f98 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector/hookconvertrector.module @@ -5,4 +5,73 @@ */ function hookconvertrector_user_cancel($edit, UserInterface $account, $method) { $red = 'red'; + $method = ['red', 'green', 'blue']; + $edit = [ + 'red' => 'red', + 'green' => 'green', + 'blue' => 'blue', + ]; +} + + +/** + * Implements hook_user_add(). + */ +#[LegacyHook] +function hookconvertrector_user_add($edit, UserInterface $account, $method) { + $red = 'red'; + $method = ['red', 'green', 'blue']; + $edit = [ + 'red' => 'red', + 'green' => 'green', + 'blue' => 'blue', + ]; +} + +/** + * Implements hook_page_attachments(). + */ +function hookconvertrector_page_attachments(array &$page) { + // Routes that don't use BigPipe also don't need no-JS detection. + if (\Drupal::routeMatch()->getRouteObject()->getOption('_no_big_pipe')) { + return; + } + + $request = \Drupal::request(); + // BigPipe is only used when there is an actual session, so only add the no-JS + // detection when there actually is a session. + // @see \Drupal\big_pipe\Render\Placeholder\BigPipeStrategy. + $session_exists = \Drupal::service('session_configuration')->hasSession($request); + $page['#cache']['contexts'][] = 'session.exists'; + // Only do the no-JS detection while we don't know if there's no JS support: + // avoid endless redirect loops. + $has_big_pipe_nojs_cookie = $request->cookies->has(BigPipeStrategy::NOJS_COOKIE); + $page['#cache']['contexts'][] = 'cookies:' . BigPipeStrategy::NOJS_COOKIE; + if ($session_exists) { + if (!$has_big_pipe_nojs_cookie) { + // Let server set the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + // Redirect through a 'Refresh' meta tag if JavaScript is disabled. + '#tag' => 'meta', + '#noscript' => TRUE, + '#attributes' => [ + 'http-equiv' => 'Refresh', + 'content' => '0; URL=' . Url::fromRoute('big_pipe.nojs', [], ['query' => \Drupal::service('redirect.destination')->getAsArray()])->toString(), + ], + ], + 'big_pipe_detect_nojs', + ]; + } + else { + // Let client delete the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + '#tag' => 'script', + '#value' => 'document.cookie = "' . BigPipeStrategy::NOJS_COOKIE . '=1; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"', + ], + 'big_pipe_detect_js', + ]; + } + } } diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module index 865909f1..0ad00848 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module @@ -7,6 +7,31 @@ use Drupal\hookconvertrector\Hook\HookconvertrectorHooks; * Implements hook_user_cancel(). */ #[LegacyHook] -function hookconvertrector_user_cancel($edit, UserInterface $account, $method) { +function hookconvertrector_user_cancel($edit, UserInterface $account, $method) +{ \Drupal::service(HookconvertrectorHooks::class)->userCancel($edit, $account, $method); } + +/** + * Implements hook_user_add(). + */ +#[LegacyHook] +function hookconvertrector_user_add($edit, UserInterface $account, $method) +{ + $red = 'red'; + $method = ['red', 'green', 'blue']; + $edit = [ + 'red' => 'red', + 'green' => 'green', + 'blue' => 'blue', + ]; +} + +/** + * Implements hook_page_attachments(). + */ +#[LegacyHook] +function hookconvertrector_page_attachments(array &$page) +{ + \Drupal::service(HookconvertrectorHooks::class)->pageAttachments($page); +} diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php index 8401ea21..28b36722 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/src/Hook/HookconvertrectorHooks.php @@ -15,5 +15,66 @@ class HookconvertrectorHooks public function userCancel($edit, \UserInterface $account, $method) { $red = 'red'; + $method = [ + 'red', + 'green', + 'blue', + ]; + $edit = [ + 'red' => 'red', + 'green' => 'green', + 'blue' => 'blue', + ]; + } + + /** + * Implements hook_page_attachments(). + */ + #[Hook('page_attachments')] + public function pageAttachments(array &$page) + { + // Routes that don't use BigPipe also don't need no-JS detection. + if (\Drupal::routeMatch()->getRouteObject()->getOption('_no_big_pipe')) { + return; + } + $request = \Drupal::request(); + // BigPipe is only used when there is an actual session, so only add the no-JS + // detection when there actually is a session. + // @see \Drupal\big_pipe\Render\Placeholder\BigPipeStrategy. + $session_exists = \Drupal::service('session_configuration')->hasSession($request); + $page['#cache']['contexts'][] = 'session.exists'; + // Only do the no-JS detection while we don't know if there's no JS support: + // avoid endless redirect loops. + $has_big_pipe_nojs_cookie = $request->cookies->has(\BigPipeStrategy::NOJS_COOKIE); + $page['#cache']['contexts'][] = 'cookies:' . \BigPipeStrategy::NOJS_COOKIE; + if ($session_exists) { + if (!$has_big_pipe_nojs_cookie) { + // Let server set the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + // Redirect through a 'Refresh' meta tag if JavaScript is disabled. + '#tag' => 'meta', + '#noscript' => TRUE, + '#attributes' => [ + 'http-equiv' => 'Refresh', + 'content' => '0; URL=' . \Url::fromRoute('big_pipe.nojs', [ + ], [ + 'query' => \Drupal::service('redirect.destination')->getAsArray(), + ])->toString(), + ], + ], + 'big_pipe_detect_nojs', + ]; + } else { + // Let client delete the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + '#tag' => 'script', + '#value' => 'document.cookie = "' . \BigPipeStrategy::NOJS_COOKIE . '=1; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"', + ], + 'big_pipe_detect_js', + ]; + } + } } } From 2ac861d8205de0281633343ec0db624ecc1be06a Mon Sep 17 00:00:00 2001 From: bjorn Date: Sat, 18 Jan 2025 21:57:48 +0100 Subject: [PATCH 63/64] Add missing class use statement --- src/Rector/Convert/HookConvertRector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rector/Convert/HookConvertRector.php b/src/Rector/Convert/HookConvertRector.php index cc37a174..3c04ca09 100644 --- a/src/Rector/Convert/HookConvertRector.php +++ b/src/Rector/Convert/HookConvertRector.php @@ -19,6 +19,7 @@ use PhpParser\NodeVisitorAbstract; use Rector\Configuration\Option; use Rector\Doctrine\CodeQuality\Utils\CaseStringHelper; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; From 370833d953e7495253bad8cecd5c9393d7dcfd97 Mon Sep 17 00:00:00 2001 From: bjorn Date: Sat, 18 Jan 2025 21:59:45 +0100 Subject: [PATCH 64/64] return is logical, it doesnt know what the return might be. --- .../fixture/hookconvertrector_updated/hookconvertrector.module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module index 0ad00848..5d58c340 100644 --- a/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module +++ b/tests/functional/hookconvertrector/fixture/hookconvertrector_updated/hookconvertrector.module @@ -33,5 +33,5 @@ function hookconvertrector_user_add($edit, UserInterface $account, $method) #[LegacyHook] function hookconvertrector_page_attachments(array &$page) { - \Drupal::service(HookconvertrectorHooks::class)->pageAttachments($page); + return \Drupal::service(HookconvertrectorHooks::class)->pageAttachments($page); }