Fix "Drupal\system\Plugin\views\field\BulkForm is deprecated in Drupal 8.5.x, will be removed before Drupal 9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead." deprecation error

Hmmm... as a plugin we should move the deprecation into the constructor because marking this test as a legacy doesn't fell right.

Plugin deprecations are a bit special see https://www.drupal.org/core/deprecation#how-plugin

Patch applied successfully. Fix also complies with Plugin Deprecation. Marking RTBC.

A legacy test means that it should be removed - I don’t think that is true in this case. I think we need to allow the test to ignore depreciations in a different way.

+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -73,16 +83,48 @@ public function testPluginData() {
    * Tests creating instances of every views plugin.
    *
    * This will iterate through all plugins from _views_fetch_plugin_data().
+   *
+   * @group legacy
+   *
+   * @expectedDeprecation Drupal\system\Plugin\views\field\BulkForm is deprecated in Drupal 8.5.x, will be removed before Drupal 9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.
+   */
+  public function testPluginInstancesIncludingDeprecated() {
+    foreach ($this->definitions as $type => $plugins) {

the new docblock of the existing function (the way git diff sees it) was updated to mention it excludes deprecated but this is still the original that should now instead explicitly mention that it tests deprecated plugins or something.

Needed a reroll so the interdiff looks a bit weird for DeprecationListenerTrait.

Updated the @legacy test to only test the deprecated plugins, before it was testing all the plugins which seems a little excessive. Updated the doc blocks to match that.

This is looking good.

+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -69,20 +79,56 @@ public function testPluginData() {
+        // Get a reflection class for this plugin.
+        // We only want to test true plugins, i.e. They extend PluginBase.
+        $reflection = new \ReflectionClass($definition['class']);
+        if ($reflection->isSubclassOf(PluginBase::class)) {
+          // Create a plugin instance and check what it is. This is not just
+          // good to check they can be created but for throwing any notices for
+          // method signatures etc. too.
+          $instance = $manager->createInstance($id);
+          $this->assertTrue($instance instanceof $definition['class'], format_string('Instance of @type:@id created', ['@type' => $type, '@id' => $id]));
+        }
...
         // Get a reflection class for this plugin.
         // We only want to test true plugins, i.e. They extend PluginBase.
         $reflection = new \ReflectionClass($definition['class']);
-        if ($reflection->isSubclassOf('Drupal\views\Plugin\views\PluginBase')) {
+        if ($reflection->isSubclassOf(PluginBase::class)) {
           // Create a plugin instance and check what it is. This is not just
           // good to check they can be created but for throwing any notices for
           // method signatures etc. too.

Let's combine the duplicated code into a protected method - that way if add to the generic plugin testing both deprecated and not deprecated plugins get the same tests without having to think too much.

Here we go, also removed a deprecated format_string.

+++ b/core/modules/system/src/Plugin/views/field/BulkForm.php
@@ -18,4 +19,12 @@
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityManagerInterface $entity_manager, LanguageManagerInterface $language_manager, MessengerInterface $messenger) {
+    @trigger_error(__NAMESPACE__ . '\BulkForm is deprecated in Drupal 8.5.x, will be removed before Drupal 9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.', E_USER_DEPRECATED);
+    parent::__construct($configuration, $plugin_id, $plugin_definition, $entity_manager, $language_manager, $messenger);

afaik part of the new format is also how the version is specified, drupal:8.5.0 and so on, but that isn't actually validated with the rule.

If we change that, the @deprecated docblock should be updated too. Still setting to RTBC, maybe that can be fixed on commit, or if that is indeed enough to hold up a commit, put back to needs work.

The test argument is a bit weird, but I agree it's nice to avoid the code duplication and possible differences.

Needed a reroll and an update to the deprecated class to use the EntityTypeManagerInterface, also updated the version notation per #15

+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -87,7 +87,7 @@
    *
-   * @expectedDeprecation Drupal\system\Plugin\views\field\BulkForm is deprecated in Drupal 8.5.x, will be removed before Drupal 9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.
+   * @expectedDeprecation Drupal\system\Plugin\views\field\BulkForm is deprecated in drupal:8.5.x, will be removed before drupal:9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.
    */

Should be drupal:8.5.0 I think, not x

I considered whether

+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -38,6 +39,15 @@ class PluginInstanceTest extends ViewsKernelTestBase {
+  /**
+   * List of deprecated plugin classes.
+   *
+   * @var string[]
+   */
+  protected $deprecatedPlugins = [
+    'Drupal\system\Plugin\views\field\BulkForm',
+  ];

is fragile to being not updated. I've pondered about detecting whether something is deprecated by doing something like

  /**
   * Determines if a class is deprecated.
   *
   * @param \ReflectionClass $class
   *   The reflected class to determine if it is deprecated.
   *
   * @return bool
   *   TRUE if the class is deprecated, FALSE if not.
   */
  protected function isClassDeprecated(\ReflectionClass $class) {
    return strpos($class->getDocComment(), '* @deprecated ') !== FALSE;
  }

But this would have problems the other way around - ie that you'd forget to add the @expectedDeprecation to the test. So I think we're good to go here.

Committed 397cd30 and pushed to 8.8.x. Thanks!

diff --git a/core/modules/system/src/Plugin/views/field/BulkForm.php b/core/modules/system/src/Plugin/views/field/BulkForm.php
index 4a0328002e..486e1be88c 100644
--- a/core/modules/system/src/Plugin/views/field/BulkForm.php
+++ b/core/modules/system/src/Plugin/views/field/BulkForm.php
@@ -12,7 +12,7 @@
  *
  * @ViewsField("legacy_bulk_form")
  *
- * @deprecated in drupal:8.5.x, will be removed before drupal:9.0.0. Use
+ * @deprecated in drupal:8.5.0, will be removed before drupal:9.0.0. Use
  *   \Drupal\views\Plugin\views\field\BulkForm instead.
  *
  * @see https://www.drupal.org/node/2916716
@@ -23,7 +23,7 @@ class BulkForm extends ViewsBulkForm {
    * {@inheritdoc}
    */
   public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, LanguageManagerInterface $language_manager, MessengerInterface $messenger) {
-    @trigger_error(__NAMESPACE__ . '\BulkForm is deprecated in drupal:8.5.x, will be removed before drupal:9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.', E_USER_DEPRECATED);
+    @trigger_error(__NAMESPACE__ . '\BulkForm is deprecated in drupal:8.5.0, will be removed before drupal:9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.', E_USER_DEPRECATED);
     parent::__construct($configuration, $plugin_id, $plugin_definition, $entity_type_manager, $language_manager, $messenger);
   }
 
diff --git a/core/modules/views/tests/src/Kernel/PluginInstanceTest.php b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
index 6b397ab935..90c4e4a158 100644
--- a/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -87,7 +87,7 @@ public function testPluginData() {
    *
    * @group legacy
    *
-   * @expectedDeprecation Drupal\system\Plugin\views\field\BulkForm is deprecated in drupal:8.5.x, will be removed before drupal:9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.
+   * @expectedDeprecation Drupal\system\Plugin\views\field\BulkForm is deprecated in drupal:8.5.0, will be removed before drupal:9.0.0. Use \Drupal\views\Plugin\views\field\BulkForm instead. See https://www.drupal.org/node/2916716.
    */
   public function testDeprecatedPluginInstances() {
     $this->assertPluginInstances(TRUE);

diff --git a/core/modules/views/tests/src/Kernel/PluginInstanceTest.php b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
index 90c4e4a158..1584b11b49 100644
--- a/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
+++ b/core/modules/views/tests/src/Kernel/PluginInstanceTest.php
@@ -106,7 +106,7 @@ public function testPluginInstances() {
   /**
    * Asserts that instances of every views plugin can be created.
    *
-   * @param bool $deprecated
+   * @param bool $test_deprecated
    *   Indicates if deprecated plugins should be tested or skipped.
    */
   protected function assertPluginInstances($test_deprecated) {

Fixed on commit.