Problem/Motivation

Plugin definitions for all plugin types in core are either arrays or objects of type PluginDefinitionInterface. There are several places in documentation with this expectation, and Drupal\Component\Plugin\Factory\DefaultFactory::getPluginClass() throws an exception on any plugin definition is that is an object that is not an instance of PluginDefinitionInterface.

However, the return type of Drupal\Component\Plugin\Attribute\AttributeInterface::get() is mixed and Drupal\Component\Plugin\Attribute\AttributeBase::get() is array|object. Also the logic in DefaultPluginManager::extractProviderFromDefinition() and DefaultPluginManagerTest::testProviderExists() accounts for plugin definitions that are other types of objects.

But since practically, only array and PluginDefinitionInterface plugin definitions are supported and used, so this issue to make that consistent across the plugin system.

Steps to reproduce

Proposed resolution

Deprecate return type of AttributeInterface and AttributeBase and prepare for change to array|PluginDefinitionInterface
Remove code such as in DefaultPluginManager::extractProviderFromDefinition() and DefaultPluginManagerTest::testProviderExists(), and wherever else, that tries to account for other types of plugin definitions.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3584799

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate’s picture

godotislate
he/him
commented
Title: Restrict plugin definitions to be array or PluginDefinitionInterface objects » Restrict plugin definitions to be arrays or PluginDefinitionInterface objects
ishani patel’s picture

ishani patel commented
Assigned: Unassigned » ishani patel

ishani patel opened merge request !15458

ishani patel’s picture

ishani patel commented
Assigned: ishani patel » Unassigned
Status: Active » Needs review

Hello @godotislate,
I've updated the changes and raised an MR.
Kindly review it.

Thank you!

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appears to have pipeline issues

godotislate’s picture

godotislate
he/him
commented

@ishani patel couple general thoughts:

We can't change return type declarations right away, because these methods are considered API, and doing so breaks any implementations in contrib or custom code. We'd have to deprecate and announce an upcoming change in the next major (D13). I don't know that we have a mechanism in place yet to announce return type changes. An alternative might be to identify everywhere AttributeInterface::get() is called, check the type of the returned value, and trigger a deprecation for an invalid type, not sure.

Also part of the scope of the issue is to find all relevant code in core that needs updating. The places listed in the IS are not necessarily comprehensive.

I'm not sure this issue is workable without investigation first.