Problem/Motivation

This is a followup to #2935897: [policy, then docs] Change how we deprecate classes.

The deprecation policy for classes now include a specific policy for concrete classes. All existing class deprecations in core need to be updated to the new policy.

This may not find all of the ones that need to change, but it is a start.
git grep "@trigger_error(" | grep NAMESPACE | awk -F: '{print $1}' | sort -u | nl

Steps to reproduce

<code>$ git grep -A3 '^namespace' | grep trigger
core/lib/Drupal/Component/Utility/DeprecatedArray.php- * An array that triggers a deprecation warning when accessed.
core/lib/Drupal/Core/FileTransfer/ChmodInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\ChmodInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/lib/Drupal/Core/Updater/Updater.php-@trigger_error('The ' . __NAMESPACE__ . '\Updater base class is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/lib/Drupal/Core/Updater/UpdaterInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\UpdaterInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\DrupalStandardsListenerDeprecatedClass is deprecated.', E_USER_DEPRECATED);
core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\FixtureDeprecatedClass is deprecated.', E_USER_DEPRECATED);

ChmodInterface.php and UpdaterInterface.php are interfaces and Updater.php is an abstract class so they have implemented the deprecation policy correctly.

That leaves looking at the tests to determine if they need to be changed.

  • core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php
  • core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php

Proposed resolution

Convert existing class deprecations to use the method in Concrete, instantiated classes.

Remaining tasks

Determine if these tests need to change. And are there tests for deprecating concrete classes with the deprecation in the constructor.

  • core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php
  • core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3335966

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

quietone created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

lucienchalom opened merge request !4272

lucienchalom’s picture

lucienchalom commented
Status: Active » Needs review

I moved the deprecation error triger in classes:
3 core/lib/Drupal/Core/Asset/JsCollectionOptimizer.php
2 core/lib/Drupal/Core/Asset/CssCollectionOptimizer.php

and deleted the redundant trigger from
4 core/lib/Drupal/Core/Http/RequestStack.php

I am not really sure how to proceed with
5 core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php
6 core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php
because they are "arbitrarily deprecated in order to test the deprecation error handling properties of DrupalStandardsListener."

and
1 core/lib/Drupal/Component/Plugin/PluginHelper.php
does not accept a contruct.

I tried to look for other classes deprecated but could not find any concrete, instantiated classes.

please help and review

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

I think this is good but not sure the policy for moving existing deprecation triggers.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work

@lucienchalom, thanks for working on this! This is a good start. For PluginHelper and RequestStack add a constructor (stated in policy) and then the test will need to be changed to instantiate the class. And the same for the test modules because we should be using the standard set in policy and well as testing that.

@smustgrave, There is no policy for moving deprecations. There is a policy for concrete classes and core needs to change to met the current standard.

The issue summary has a sample grep to find the instances that need to be changed. Has anyone worked to improve on that or verify that it is accurate?

quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs work » Active
Issue tags: +Novice

Rebased and updated the issue summary.

I think that this is suitable as a novice level issue.

sanket.tale made their first commit to this issue’s fork.

sanket.tale’s picture

sanket.tale commented
Status: Active » Needs review

Hi Updated both Updater.php and I18nQueryTrait.php. Please review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

We are adding a trait to deprecate it? That doesn’t seem right,

Deprecations should probably get a CR too

sanket.tale’s picture

sanket.tale commented
Status: Needs work » Needs review

Thank you for flagging this! After verifying that the I18nQueryTrait is not used anywhere, I’ve removed it entirely. I’ve also updated the existing Change Record to reflect this change. You can check it out here: Change Record - https://www.drupal.org/node/3439256.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

This MR seems wrong.

Whatever changes were done in #6 are all gone.

#13 is also worrisome. "After verifying that the I18nQueryTrait is not used anywhere, I’ve removed it entirely." 😬 That's not how this works. 😅We mark something deprecated and *leave it where it is* so that contrib and custom code isn't immediately broken. That's the point of deprecation. So yes, it's unused in Core. But it needs to remain until D12 when we told people we'd remove it from its current location. I'm afraid of whatever edits were made to a live CR about this. Those probably need to be reverted, too. Although thankfully, https://www.drupal.org/node/3439256/revisions/view/13881188/13973995 doesn't look so bad (other than using markdown syntax which isn't yet supported on d.o CRs and docs pages).

I'm not sure this issue is really suitable for "novice" contributors.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes

p.s. I'm unclear why core/lib/Drupal/Core/Updater/Updater.php is being touched, since that is in fact an abstract class, so the existing deprecation is following the documented procedure.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs work » Active
Issue tags: -Novice

@dww, thanks for commenting.

This work has not followed the directions given in the issue summary. So, I started over with a new search

$ git grep -A3 '^namespace' | grep trigger
core/lib/Drupal/Component/Utility/DeprecatedArray.php- * An array that triggers a deprecation warning when accessed.
core/lib/Drupal/Core/FileTransfer/ChmodInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\ChmodInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/lib/Drupal/Core/Updater/Updater.php-@trigger_error('The ' . __NAMESPACE__ . '\Updater base class is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/lib/Drupal/Core/Updater/UpdaterInterface.php-@trigger_error('The ' . __NAMESPACE__ . '\UpdaterInterface is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. Use composer to manage the code for your site. See https://www.drupal.org/node/3512364', E_USER_DEPRECATED);
core/modules/system/tests/modules/deprecation_test/src/Deprecation/DrupalStandardsListenerDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\DrupalStandardsListenerDeprecatedClass is deprecated.', E_USER_DEPRECATED);
core/modules/system/tests/modules/deprecation_test/src/Deprecation/FixtureDeprecatedClass.php-@trigger_error(__NAMESPACE__ . '\FixtureDeprecatedClass is deprecated.', E_USER_DEPRECATED);

The remaining deprecations just after the namespace are correct. ChmodInterface.php and UpdaterInterface.php are interfaces and Updater.php is an abstract class. Thus, they have implemented the deprecation policy correctly.

The test classes, however are not using the correct style, they need to be looked at to make sure that there is test coverage for the deprecation of concrete classes in the constructor.

I am removing the novice tag. Removed this issue from the change record, https://www.drupal.org/node/3439256

Active because this is starting over.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.