Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule for node_load_multiple #148

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions rector_examples/node_load_multiple.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

/**
* This demonstrates the deprecated static calls that might be called from procedural code like `.module` files.
*/

/**
* A simple example.
*/
function simple_example() {
$nodes = node_load_multiple([123, 456]);
}

/**
* An example using all of the arguments.
*/
function all_arguments() {
$nodes = node_load_multiple([123, 456], TRUE);
}

/**
* An example using a variable for the argument.
*/
function all_arguments_as_variables() {
$node_ids = [123, 456];
$reset = TRUE;
$nodes = node_load_multiple($node_ids, $reset);
}
32 changes: 32 additions & 0 deletions rector_examples_updated/node_load_multiple.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

/**
* This demonstrates the deprecated static calls that might be called from procedural code like `.module` files.
*/

/**
* A simple example.
*/
function simple_example() {
$nodes = \Drupal::service('entity_type.manager')->getStorage('node')->loadMultiple([123, 456]);
}

/**
* An example using all of the arguments.
*/
function all_arguments() {
// TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
// A ternary operator is used here to keep the conditional contained within this part of the expression. Consider wrapping this statement in an `if / else` statement.
$nodes = TRUE ? \Drupal::service('entity_type.manager')->getStorage('node')->resetCache([123, 456])->loadMultiple([123, 456]) : \Drupal::service('entity_type.manager')->getStorage('node')->loadMultiple([123, 456]);
}

/**
* An example using a variable for the argument.
*/
function all_arguments_as_variables() {
$node_ids = [123, 456];
$reset = TRUE;
// TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
// A ternary operator is used here to keep the conditional contained within this part of the expression. Consider wrapping this statement in an `if / else` statement.
$nodes = $reset ? \Drupal::service('entity_type.manager')->getStorage('node')->resetCache($node_ids)->loadMultiple($node_ids) : \Drupal::service('entity_type.manager')->getStorage('node')->loadMultiple($node_ids);
}
23 changes: 13 additions & 10 deletions src/Rector/Deprecation/Base/EntityLoadBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ public function refactor(Node $node): ?Node
// This will work for node_load, etc.
$method_name = $this->entityType . '_load';
}
$method_name_multiple = $method_name . '_multiple';

/** @var Node\Expr\FuncCall $node */
if ($this->getName($node->name) === $method_name) {
if (in_array($this->getName($node->name), [$method_name, $method_name_multiple])) {
$is_multiple = $this->getName($node->name) === $method_name_multiple;

// We are doing this here, because we know we have access to arguments since we have already checked the method name.
if ($is_rector_rule_entity_load) {
// Since we have one more argument, all the array keys are one greater.
Expand All @@ -76,8 +79,8 @@ public function refactor(Node $node): ?Node
$entity_type = new Node\Arg(new Node\Scalar\String_($this->entityType));
}

/* @var Node\Arg $entity_id. */
$entity_id = $node->args[0 + $argument_offset];
/* @var Node\Arg $entity_ids. */
$entity_ids = $node->args[0 + $argument_offset];

$name = new Node\Name\FullyQualified('Drupal');

Expand All @@ -95,9 +98,9 @@ public function refactor(Node $node): ?Node
$getStorage_node = new Node\Expr\MethodCall($var, $getStorage_method_name, [$entity_type]);

// Create the simple version of the entity load.
$load_method_name = new Node\Identifier('load');
$load_method_name = new Node\Identifier($is_multiple ? 'loadMultiple' : 'load');

$new_node = new Node\Expr\MethodCall($getStorage_node, $load_method_name, [$entity_id]);
$new_node = new Node\Expr\MethodCall($getStorage_node, $load_method_name, [$entity_ids]);

// We need to account for the `reset` option which adds a method to the chain.
// We will replace the original method with a ternary to evaluate and provide both options.
Expand All @@ -109,14 +112,14 @@ public function refactor(Node $node): ?Node

$resetCache_method_name = new Node\Identifier('resetCache');

$reset_args = [
$reset_args = $is_multiple ?
$entity_ids :
// This creates a new argument that wraps the entity ID in an array.
new Node\Arg(new Node\Expr\Array_([new Node\Expr\ArrayItem($entity_id->value)])),
];
new Node\Arg(new Node\Expr\Array_([new Node\Expr\ArrayItem($entity_ids->value)]));

$entity_load_reset_node = new Node\Expr\MethodCall($getStorage_node, $resetCache_method_name, $reset_args);
$entity_load_reset_node = new Node\Expr\MethodCall($getStorage_node, $resetCache_method_name, [$reset_args]);

$entity_load_reset_node = new Node\Expr\MethodCall($entity_load_reset_node, $load_method_name, [$entity_id]);
$entity_load_reset_node = new Node\Expr\MethodCall($entity_load_reset_node, $load_method_name, [$entity_ids]);

// Replace the new_node with a ternary.
$new_node = new Node\Expr\Ternary($reset_flag->value, $entity_load_reset_node, $new_node);
Expand Down
42 changes: 42 additions & 0 deletions src/Rector/Deprecation/NodeLoadMultipleRector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

namespace DrupalRector\Rector\Deprecation;

use DrupalRector\Rector\Deprecation\Base\EntityLoadBase;
use Rector\Core\RectorDefinition\CodeSample;
use Rector\Core\RectorDefinition\RectorDefinition;

/**
* Replaced deprecated node_load_multiple() calls.
*
* See https://www.drupal.org/node/2266845 for change record.
*
* What is covered:
* - See EntityLoadBase.php
*
* Improvement opportunities
* - See EntityLoadBase.php
*/
final class NodeLoadMultipleRector extends EntityLoadBase
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it will duplicate / be redundant to the implementation of https://github.com/palantirnet/drupal-rector/blob/master/src/Rector/Deprecation/NodeLoadRector.php.

{
protected $entityType = 'node';

/**
* @inheritdoc
*/
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Fixes deprecated node_load_multiple() use',[
new CodeSample(
<<<'CODE_BEFORE'
$nodes = node_load_multiple([123, 456]);
CODE_BEFORE
,
<<<'CODE_AFTER'
$nodes = \Drupal::entityManager()->getStorage('node')->loadMultiple([123, 456]);
CODE_AFTER
)
]);
}

}