Fix subclassing and stop overriding constructors [#3111730]

Problem/Motivation

Add object that uses the FactoryInterface should have dependencies inject via ::create and not ::__construct(). This allows parent classes to change their constructor with breaking any code.
@see https://events.drupal.org/seattle2019/sessions/drupal-9-coming-getting-y...

Example: \Drupal\config_sync\Controller\ConfigSyncController::create
@see https://www.youtube.com/watch?time_continue=1554&v=hN9KjaBvAUk
@see https://www.previousnext.com.au/blog/safely-extending-drupal-8-plugin-cl...

Proposed resolution

Fix subclassing and stop overriding constructors.

Comment	File	Size	Author
#7	pathauto-Fix_subclassing_and_stop_overriding_constructors-3111730-7-D8.patch	5.08 KB	thalles
#3	pathauto-Fix_subclassing_and_stop_overriding_constructors-3111730-3-D8.patch	5.08 KB	thalles
#2	pathauto-Fix_subclassing_and_stop_overriding_constructors-3111730-2-D8.patch	3.58 KB	thalles

Comments

Comment #1

6 February 2020 at 18:49

thalles created an issue. See original summary.

Comment #2

thalles

Portuguese, Brazil

Teófilo Otoni - MG

commented 6 February 2020 at 18:58

Status:

Needs work

» Needs review

Status	File	Size
new	pathauto-Fix_subclassing_and_stop_overriding_constructors-3111730-2-D8.patch	3.58 KB

Follow a patch.

Comment #3

thalles

Portuguese, Brazil

Teófilo Otoni - MG

commented 27 April 2020 at 21:53

Status	File	Size
new	pathauto-Fix_subclassing_and_stop_overriding_constructors-3111730-3-D8.patch	5.08 KB

Follow a new patch

Comment #4

berdir

German

Switzerland

commented 28 April 2020 at 18:15

Status:

Needs review

» Needs work

+++ b/src/Plugin/migrate/source/PathautoPattern.php
@@ -27,27 +27,15 @@ class PathautoPattern extends DrupalSqlBase {
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration = NULL) {
-    return new static(
-      $configuration,
-      $plugin_id,
-      $plugin_definition,
-      $migration,
-      $container->get('state'),
-      $container->get('entity_type.manager'),
-      $container->get('entity_type.bundle.info')
-    );
+    /** @var \Drupal\pathauto\Plugin\migrate\source\PathautoPattern */
+    $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition, $migration);
+    $instance->entityTypeBundleInfo = $container->get('entity_type.bundle.info');
+
+    return $instance;

per discussion in the other issue, /** @var static $instance */ usually works fine for me, and this is definitely not valid because it's missing the variable name which is required for an inline definition.