10.2: EntityReferenceTestTrait to EntityReferenceFieldCreationTrait rector using RenameClassRector

[bbrala]

Description

Fixes: https://www.drupal.org/node/3401941

When renaming it does not remove the old use statement. This should not error, but is ugly.

Can only be avoided if we start applying codestyle to the files. Which seem overkill. Although something like this could work maybe? rector-src/vendor/symplify/easy-coding-standard/config/set/common/namespaces.php

To Test

Functional tests

Drupal.org issue

Provide a link to the issue from https://www.drupal.org/project/rector/issues. If no issue exists, please create one and link to this PR.

[bbrala]

I've been looking into how i could use two traits, but the ways to do it are very very very very ugly.

The sad thing is, the method could have been static...

  /**
   * Creates a field of an entity reference field storage on the specified bundle.
   *
   * @param string $entity_type
   *   The type of entity the field will be attached to.
   * @param string $bundle
   *   The bundle name of the entity the field will be attached to.
   * @param string $field_name
   *   The name of the field; if it already exists, a new instance of the existing
   *   field will be created.
   * @param string $field_label
   *   The label of the field.
   * @param string $target_entity_type
   *   The type of the referenced entity.
   * @param string $selection_handler
   *   The selection handler used by this field.
   * @param array $selection_handler_settings
   *   An array of settings supported by the selection handler specified above.
   *   (e.g. 'target_bundles', 'sort', 'auto_create', etc).
   * @param int $cardinality
   *   The cardinality of the field.
   *
   * @see \Drupal\Core\Entity\Plugin\EntityReferenceSelection\SelectionBase::buildConfigurationForm()
   */
  protected function createEntityReferenceField($entity_type, $bundle, $field_name, $field_label, $target_entity_type, $selection_handler = 'default', $selection_handler_settings = [], $cardinality = 1) {
    // Look for or add the specified field to the requested entity bundle.
    if (!FieldStorageConfig::loadByName($entity_type, $field_name)) {
      FieldStorageConfig::create([
        'field_name' => $field_name,
        'type' => 'entity_reference',
        'entity_type' => $entity_type,
        'cardinality' => $cardinality,
        'settings' => [
          'target_type' => $target_entity_type,
        ],
      ])->save();
    }
    if (!FieldConfig::loadByName($entity_type, $bundle, $field_name)) {
      FieldConfig::create([
        'field_name' => $field_name,
        'entity_type' => $entity_type,
        'bundle' => $bundle,
        'label' => $field_label,
        'settings' => [
          'handler' => $selection_handler,
          'handler_settings' => $selection_handler_settings,
        ],
      ])->save();
    }
  }

We could do some magic with an added method and an anonimous class, which loads based on which class exists. Especially since that trait is not using the object context at all.

Then it could be even backwards compatible. It just uses ugly code then.

Somthings like

protected function createEntityReferenceField($entity_type, $bundle, $field_name, $field_label, $target_entity_type, $selection_handler = 'default', $selection_handler_settings = [], $cardinality = 1) {
    if(trait_exists('Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait')){
      $caller = new class() { use \Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait; };
    } else {
       $caller = new class() { use \OldTrait; };
     }

    
      $caller->createEntityReferenceField($entity_type, $bundle, $field_name, $field_label, $target_entity_type, $selection_handler, $selection_handler_settings, $cardinality);
}

This might work, but the code is not very pretty.

[mglaman]

It'd probably be easier to add a class alias at the top of the class for the trait.

[bbrala]

I don't understand what you mean.

[bbrala]

Ok something like

<?php 
trait Foo {}
class_alias("Foo","Bar");
echo trait_exists("Bar") ? 'yes' : 'no'; 
?>
//yes

But wrapped more sanely

[bbrala]

Ok, something like this should work.

/**
 * @description In order to support Drupal versions before 10.2 we need to alias the EntityReferenceFieldCreationTrait to EntityReferenceTestTrait.
 */
if (!class_exists(\Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait::class)) {
  class_alias(\Drupal\Tests\field\Traits\EntityReferenceTestTrait::class, \Drupal\Tests\field\Traits\EntityReferenceFieldCreationTrait::class);
}

class Test {

  use EntityReferenceFieldCreationTrait;

}

$ref = new ReflectionClass(Test::class);

var_dump($ref->getTraits());

Would probably not use the RenameClassRector but should use the service. That way renaming is easy, but we are targetting a trait right now, so might reduce the scope initially to keep it simple.

Probably:

1. Only target classes
2. Check their trait use for the deprecated trait
3. If found add the class alias stuff (hopefully we can while at a class node)
4. Use RenamedClassesDataCollector to rename the class, hopefully.

Few things im not sure about yet:

1. When should the class_alias be removed when using upgrade bot. And does this mean min version will be upped to 10.2 even though this could also work in earlier version.
2. Adding the code above the class might not work.
3. Getting down into the class and renaming the use statement might be a hassle.

Also important: 10.2 was fixed in regards to keeping the old class see https://www.drupal.org/node/3403491

[mglaman]

Yeah, like that. But only in tests using the trait – I have no idea how hard that makes it. Maybe target Class and then use PHPStan scope/tools to detect traits used.

When should the class_alias be removed when using upgrade bot. And does this mean min version will be upped to 10.2 even though this could also work in earlier version.

Good Q. Maybe when D10 is EOL.

Adding the code above the class might not work.

It should. See this abomination https://git.drupalcode.org/project/acquia_connector/-/blob/4.x/src/Event/EventBase.php?ref_type=heads

[bbrala]

Haha yeah I understand the code will work. But mostly if I can get rector to do it :)

But I'll have a look, and perhaps we shouldn't care too much about forcing ^10.1 || ^11 in the modules. Although unneeded.

[bbrala]

All code that will be for 10.x deprecation will have that anyways, since deprecationhelper is only available since then.

[agentrickard]

@mglaman I fixed the merge conflict here and tests pass locally. Could use another review.

[agentrickard]

@bbrala @mglaman This PR is green again. Are we good to merge?