Problem/Motivation

Passing NULL as plugin ID to the ::createInstance() of a plugin manager with fallback plugin worked until now and the fallback plugin was instantiated. With a fallback plugin manager, passing NULL as plugin ID should not be so uncommon. Designing a fallback plugin manager is often made for such cases. Just imagine the plugin ID is stored in nullable config property. If it was not configured you want to get the fallback plugin.

But with PHP 8.5 a deprecation error is emitted:

Deprecated function: Using null as an array offset is deprecated, use an empty string instead in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 45 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php)

Steps to reproduce

Call ::createInstance(NULL) on a plugin manager with fallback.

Proposed resolution

Fix.

Remaining tasks

None.

User interface changes

None.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Issue fork drupal-3581628

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

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes

andypost made their first commit to this issue’s fork.

andypost opened merge request !15221

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review

Added quick-fix and test

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3546535: [meta] Deal with NULL as array key/offset
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs review » Reviewed & tested by the community

I still think this is a bug, as without a plugin ID we should get the fallback plugin. At least, for me, this was the obvious usage of the fallback interface.

The fix is simple and tested, RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

I'm not sure. There's no code in core calling with a NULL and for me the fallback is about handling the case when you have have configured plugin that disappears for some reason so plugin ID would not be NULL in this case. Do we have a concrete example of this happening in contrib? Otherwise we're just adding code that goes against the documentation... ie. 

  /**
   * Creates a plugin instance based on the provided ID and configuration.
   *
   * @param string $plugin_id
   *   The ID of the plugin being instantiated.
   * @param array $configuration
   *   An array of configuration relevant to the plugin instance.
   *
   * @return object
   *   A fully configured plugin instance.
   *
   * @throws \Drupal\Component\Plugin\Exception\PluginException
   *   If the instance cannot be created, such as if the ID is invalid.
   */
  public function createInstance($plugin_id, array $configuration = []);

Our docs say $plugin_id is a string - so don't pass a NULL to it.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

@alexpott, a plugin that disappears is a use case. My understanding was that a plugin not configured somewhere in a config is other case. Why only disappearing plugins are covered? My first thought when a saw the fallback interface was that it addresses this scenario: "if you don't have a plugin we're covered, we give you the fallback".

But I'm OK to solve the problem in the caller, not a big deal, and for now on to be careful with the fallback mechanism. Maybe FallbackPluginManagerInterface should implement a method ::createInstanceOrFallback() to cover the nullable case?

smustgrave’s picture

smustgrave commented

@alexpott what do you think of the response above?