Remove manual create() method from plugins

Adding related issue for controllers

Not sure the best way to find these, but this finds all files in src/Plugin dirs with the same signature:
$ find core -type f -path "*/src/Plugin/*" -exec grep -rH 'public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition)' {} \;

Note this doesn't include classes extending DeriverBase, ConstraintValidator or any Migration plugins with a ?MigrationInterface param. There are potentially others.

Ideally this is a job for rector as there are 172 exact matches.

EDIT: Sorry for multiple edits, I messed up the find command a couple times

Not sure what the best way is either, but PHPStorm located overrides of PluginBase::create() for me and I'm going with that.

@mstrelan I'm still working on it, finding issues caused by removing too much.

Had a quick attempt at profiling this, not spotting any major differences, variation is quite high, but I've had less than 5% time/memory differences.

Tested with disabled render caches on umami. 644 calls to ContainerFactory::createInstance(), in total 20ms/5%. \Drupal\Core\DependencyInjection\AutowiredInstanceTrait::createInstanceAutowired doesn't show up at all, I guess it's because it's a trait and split among many different classes, so doesn't registered as the same thing. ContainerFactory::createInstance() reports as 80% own cost, but I think that's just blackfire being unable to attribute it to anything else.

If anything, just spotted unrelated issues:

* ViewsBlock is by far the most expensive plugin to create, 10% of the total time with just 4 calls. This doesn't even use autowire yet due to the entity storage being injected and it does too much in in __construct().
* 100 calls to Drupal\Core\ImageToolkit\ImageToolkitManager::createInstance(). less than 1% of total time, but still, seems like a waste. It's actually all coming from 100 calls to isAvifSupported, I'm not sure we can statically cache the default instance as it has state (fibers..) but we can definitely statically cache if we support avif or not.

I responded to the feedback that was already given.

Any reason why Drupal\image\ImageEffectBase is not converted?

Any reason why Drupal\image\ImageEffectBase is not converted?

I checked for logger channels that are defined and not in use by the plugins, but missed that one. Thanks for catching it.

Tagging for release priority - having done #3452852: Add create() factory method with autowired parameters to PluginBase this is needed if we want to have contrib starting conversions. If core plugins still have the create() method, contrib extending from those cannot benefit from the changes (unless they override the method, but that will mean touching the code twice)

Appears to need a manual rebase.

I reverted the changes to core/modules/filter/src/Plugin/migrate/process/FilterID.php which was recently deprecated. That deprecation causes PHPStan issues in core/modules/filter/src/Plugin/migrate/process/FilterID.php when FilterID::create() is removed. I couldn't stop the errors. If someone has an idea, then I'll try to fix it. But for now I decided to revert the changes since the class is deprecated anyway. So I'm not sure it matters.

Reverted the revert (would that fit?), and just adjusted the baseline removing the entry that is no longer matched.

Ugh, thank you. I could not figure out where that was coming from. I checked the output multiple times, but never saw any indication. IDK if I just missed it or didn't understand what it was saying.

When PHPStan reports Ignored error pattern […] was not matched in reported errors., it means that an ignored error, either in baseline or in the neon file, is no longer there. The way to solve is to remove the pattern entry from the baseline or the file. Or to regenerate the baseline.

Thank you for instructing me on that. I'm going to go back and look for it when I'm home later.

Hello,

For #3422958: Error: Call to a member function transformRelative() , I removed the create from core/modules/image/src/Plugin/Field/FieldFormatter/ImageUrlFormatter.php.

The problem is that it was an entity storage that was injected. So I did minimal change to only switch to EntityTypeManagerInterface.

Regarding https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal, the storage should be obtained only at the moment of usage, not in the constructor, but I guess this should be a separated issue.

I've made the changes and answered the questions.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rebased. There was a conflict in the workspaces_ui block.

One question on the MR, leaving RTBC because I'm hoping it's a bad question and we can leave as-is.

Committed/pushed to main and cherry-picked to 11.x, thanks!

No unfortunately not - MR doesn't apply, committed it when it did, then another issue landed, then I couldn't rebase main to push.

Rebased.

Committed/pushed to main and 11.x, thanks!