Problem/Motivation
In #2946122: Deprecate ConfigurablePluginInterface and replace with an interface that doesn't extend DependentPluginInterface We added an isConfigurable() method to the PluginBase class as a helper while we were converting things from ConfigurablePluginInterface to ConfigurableInterface, along with a static PluginHelper class to do the same thing. Since ConfigurablePluginInterface is long dead now, all this method does is an instanceOf check against ConfigurableInterface.
The method on the base class is not part of any interface, meaning if core actually used this to check if a plugin was configurable, it would break tests trying to use mocks or prophecies based on plugin interfaces (which is why the static helper was introduced there as well). Fortunately core does not use this.
Looking back at 2946122, I'm not quite sure why we even included it. I'm wondering if it should have been removed when we decided to go with the static helper, and doing so was just missed (by me... :-( ). We deprecated the static helper in 10.1 (#3105685: Deprecate PluginHelper::isConfigurable()) and removed it from Drupal 11. We should deprecate this method and remove it from D12 in favor of having any calling code just do an `instanceof ConfigurableInterface` check
Steps to reproduce
Proposed resolution
Deprecate PluginBase->isConfigurable in D11.1 and remove from D12.0
Remaining tasks
Review and commit
User interface changes
API changes
PluginBase->isConfigurable() is deprecated
Data model changes
Release notes snippet
\Drupal\Component\Plugin\PluginBase->isConfigurable() is deprecated. Use instanceof to check if a plugin implements ConfigurableInterface instead.
Issue fork drupal-3459533
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:
Comments
Comment #3
mikelutzComment #4
mikelutzComment #5
smustgrave commentedDeprecation versions seem correct. Has simple test case for it. CR is there. Believe this is good
Comment #7
catchWent ahead and committed this without the test coverage:
- we generally don't add deprecation testing unless there's an actual bc layer involved, just triggering the deprecation is enough.
- getMockForAbstractClass() is deprecated in phpunit 10 and we only don't need to add it to the baseline because it's being called in a @legacy test. It would mean having to remove or refactor this test before we can update to phpunit 11 - another reason to skip test coverage.
Committed/pushed to 11.x, thanks!