Problem/Motivation

Noticed here: #3560397: Deprecate node.admin.inc

We should fix all instances : grep -rniE 'deprecated.*project/drupal/issues' core/

Steps to reproduce

Run grep

Proposed resolution

Confirm these are incorrect, some deprecations do reference the issue if there is no CR
If they are incorrect, update them to the correct CR link.

  1. core/modules/block_content/src/Plugin/Block/BlockContentBlock.php:55: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $entityRepository argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3175985', E_USER_DEPRECATED);
  2. core/modules/content_translation/content_translation.admin.inc:12:@trigger_error(__FILE__ . ' is deprecated in drupal:11.4.0 and is removed from drupal:12.0.0. Functions have been deprecated and moved to content_translation.module. See https://www.drupal.org/project/drupal/issues/3548573', E_USER_DEPRECATED);
  3. core/modules/system/src/Plugin/Block/SystemBrandingBlock.php:39: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $themeSettingsProvider argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3035289', E_USER_DEPRECATED);
  4. core/modules/comment/src/CommentManager.php:199: @trigger_error(__FUNCTION__ . '() is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. Use \Drupal\history\HistoryManager::getCountNewComments() instead. See https://www.drupal.org/project/drupal/issues/3551729', 'E_USER_DEPRECATED');
  5. core/modules/user/src/Theme/AdminNegotiator.php:61: @trigger_error("Passing the $deprecated_service_name (entity_type.manager service) to AdminNegotiator is deprecated in drupal:11.2.0 and will be removed in drupal:12.0.0. There is no replacement for this service, as it is not used. See https://www.drupal.org/project/drupal/issues/3501727", E_USER_DEPRECATED);
  6. core/modules/node/node.admin.inc:14:@trigger_error(__FILE__ . ' is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. Functions have been deprecated. See https://www.drupal.org/project/drupal/issues/3533315', E_USER_DEPRECATED);
  7. core/modules/node/src/Controller/NodePreviewController.php:79: @trigger_error(__METHOD__ . ' is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/project/drupal/issues/3024386', E_USER_DEPRECATED);
  8. core/modules/language/src/ConfigurableLanguageManager.php:130: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $cacheBackend argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3497341', E_USER_DEPRECATED);
  9. core/modules/editor/src/Plugin/EditorManager.php:40: @trigger_error('Calling ' . __METHOD__ . '() without the $entityTypeManager argument is deprecated in drupal:11.2.0 and will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3447794', E_USER_DEPRECATED);
  10. core/modules/workspaces/src/WorkspacePublisher.php:39: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $time argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3531037', E_USER_DEPRECATED);
  11. core/modules/workspaces/src/WorkspaceOperationFactory.php:33: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $time argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3531037', E_USER_DEPRECATED);
  12. core/tests/Drupal/KernelTests/Core/ClassLoader/BackwardsCompatibilityClassLoaderTest.php:48: $this->expectDeprecation('Class ' . Foo::class . ' is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0, use Drupal\Component\Utility\Random instead. See https://www.drupal.org/project/drupal/issues/3502882');
  13. core/lib/Drupal/Core/Access/RouteProcessorCsrf.php:32: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $requestStack argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3485174', E_USER_DEPRECATED);
  14. core/lib/Drupal/Core/Render/BareHtmlPageRenderer.php:45: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $themeSettingsProvider argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3035289', E_USER_DEPRECATED);
  15. core/lib/Drupal/Core/Asset/AssetResolver.php:89: @trigger_error('Calling ' . __METHOD__ . ' without the $theme_handler argument is deprecated in drupal:11.1.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3451667', E_USER_DEPRECATED);
  16. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:44 @trigger_error('Not passing the language.default_values parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');
  17. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:48 @trigger_error('Not passing the language.translate_english parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');

Change the above URLS as follows

  1. core/modules/block_content/src/Plugin/Block/BlockContentBlock.php:55: no change, use link to issue
  2. core/modules/content_translation/content_translation.admin.inc:12: https://www.drupal.org/node/3548573
  3. core/modules/system/src/Plugin/Block/SystemBrandingBlock.php:39: https://www.drupal.org/node/3035289
  4. core/modules/comment/src/CommentManager.php:199: https://www.drupal.org/node/3551729
  5. core/modules/user/src/Theme/AdminNegotiator.php:61: no change, use link to issue
  6. core/modules/node/node.admin.inc:14: https://www.drupal.org/node/3533315
  7. core/modules/node/src/Controller/NodePreviewController.php:79: no change, use link to issue
  8. core/modules/language/src/ConfigurableLanguageManager.php:130: no change, use link to issue
  9. core/modules/editor/src/Plugin/EditorManager.php:40: https://www.drupal.org/node/3509245
  10. core/modules/workspaces/src/WorkspacePublisher.php:39: https://www.drupal.org/node/3531039
  11. core/modules/workspaces/src/WorkspaceOperationFactory.php:33: https://www.drupal.org/node/3531039
  12. core/tests/Drupal/KernelTests/Core/ClassLoader/BackwardsCompatibilityClassLoaderTest.php:48: https://example.com/change_record
  13. core/lib/Drupal/Core/Access/RouteProcessorCsrf.php:32: no change, use link to issue
  14. core/lib/Drupal/Core/Render/BareHtmlPageRenderer.php:45: https://www.drupal.org/node/3035289
  15. core/lib/Drupal/Core/Asset/AssetResolver.php:89: no change, use link to issue
  16. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:44 no change, use link to issue
  17. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:48 no change, use link to issue

Remaining tasks

Validate MR
Create

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3560560

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

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Issue tags: +Novice
quietone’s picture

quietone commented
Issue summary: View changes

formatting

quietone’s picture

quietone commented
Issue tags: +11.3.0 release target

This should be cleaned up before the release.

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

quietone opened merge request !14066

quietone’s picture

quietone commented
Issue tags: -Novice

Due to the amount of git research needed on this issue I am removing the Novice tag.

I think this requires a more git research than is suitable for a novice issue, so removing the tag.

I categorized each URL and fixed those where the issue had a change record.

No change record
1, 5, 7, 8, 13, 15, And a new one, \Drupal\language\Config\LanguageConfigFactoryOverride

Further investigation is needed to determine if these need a change record.

404
2, 3, 4, 6, 14

Fixed
9, 12
10, 11 Changed URL and added more detail to the change record.

quietone’s picture

quietone commented
Issue summary: View changes

Fixed these 404s by using git blame to find the issue, which then had a CR; 2, 3, 4, 14. That makes 8 fixes and 7 more to figure out.

No change record
1, 5, 7, 8, 13, 15, And a new one, \Drupal\language\Config\LanguageConfigFactoryOverride

404
6

Fixed
2, 3, 4, 9, 12, 14
10, 11 Changed URL and added more detail to the change record.

quietone’s picture

quietone commented
Issue summary: View changes

And the last 404, number 6. There is a change record for that, https://www.drupal.org/node/3533315

No change record
1, 5, 7, 8, 13, 15, And a new one, \Drupal\language\Config\LanguageConfigFactoryOverride

Fixed
2, 3, 4, 6, 9, 12, 14
10, 11 Changed URL and added more detail to the change record.

quietone’s picture

quietone commented
Issue summary: View changes

Add LanguageConfigFactoryOverride to the issue summary

  1. core/modules/block_content/src/Plugin/Block/BlockContentBlock.php:55: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $entityRepository argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3175985', E_USER_DEPRECATED);
  2. core/modules/user/src/Theme/AdminNegotiator.php:61: @trigger_error("Passing the $deprecated_service_name (entity_type.manager service) to AdminNegotiator is deprecated in drupal:11.2.0 and will be removed in drupal:12.0.0. There is no replacement for this service, as it is not used. See https://www.drupal.org/project/drupal/issues/3501727", E_USER_DEPRECATED);
  3. core/modules/node/src/Controller/NodePreviewController.php:79: @trigger_error(__METHOD__ . ' is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/project/drupal/issues/3024386', E_USER_DEPRECATED);
  4. core/modules/language/src/ConfigurableLanguageManager.php:130: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $cacheBackend argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3497341', E_USER_DEPRECATED);
  5. core/lib/Drupal/Core/Access/RouteProcessorCsrf.php:32: @trigger_error('Calling ' . __CLASS__ . ' constructor without the $requestStack argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3485174', E_USER_DEPRECATED);
  6. core/lib/Drupal/Core/Asset/AssetResolver.php:89: @trigger_error('Calling ' . __METHOD__ . ' without the $theme_handler argument is deprecated in drupal:11.1.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3451667', E_USER_DEPRECATED);
  7. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:44 @trigger_error('Not passing the language.default_values parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');
  8. core/modules/language/src/Config/LanguageConfigFactoryOverride.php:48 @trigger_error('Not passing the language.translate_english parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');
quietone’s picture

quietone commented
Issue summary: View changes
Status: Active » Needs review

The remaining error messages are about changes to the arguments in a constructor. Typically, a change record doesn't add much information for that situation. So, having it link to the issue instead is reasonable. That means the changes are as follows,

No change record - Use the link to the issue
1, 5, 7, 8, 13, 15, And a new one, \Drupal\language\Config\LanguageConfigFactoryOverride

Fixed
2, 3, 4, 6, 9, 12, 14
10, 11 Changed URL and added more detail to the change record.

Most importantly is that all the 404s have been fixed. So, time for a review.

quietone’s picture

quietone commented
Issue summary: View changes

There was a test failure for BackwardsCompatibilityClassLoaderTest. And then I realized that that test should be using a test URL not a real link to an existing issue or change record. I have changed that URL to use example.com.

quietone’s picture

quietone commented
Parent issue: » #3553512: [meta] Ensure documentation URLs are valid
dww’s picture

dww
we/he/they
commented

The MR itself looks good. Some of the linked CRs did not. 😅 I did some edits as I was confirming that each linked CR matched the deprecation message. Sometimes it was cosmetic formatting, but in some cases, the CR didn't mention things like constructor params changing. For the record, here are all the edits I made:

Meanwhile, checking out the issue fork and running the grep from the summary locally, I'm still seeing:

core/lib/Drupal/Core/Asset/AssetResolver.php:89:      @trigger_error('Calling ' . __METHOD__ . ' without the $theme_handler argument is deprecated in drupal:11.1.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3451667', E_USER_DEPRECATED);
core/lib/Drupal/Core/Access/RouteProcessorCsrf.php:32:      @trigger_error('Calling ' . __CLASS__ . ' constructor without the $requestStack argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3485174', E_USER_DEPRECATED);
core/modules/language/src/Config/LanguageConfigFactoryOverride.php:44:      @trigger_error('Not passing the language.default_values parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');
core/modules/language/src/Config/LanguageConfigFactoryOverride.php:48:      @trigger_error('Not passing the language.translate_english parameter to LanguageConfigFactoryOverride::__construct() is deprecated in drupal:11.3.0 and will be removed in drupal::12.0.0. See https://www.drupal.org/project/drupal/issues/3518992');
core/modules/language/src/ConfigurableLanguageManager.php:130:      @trigger_error('Calling ' . __CLASS__ . ' constructor without the $cacheBackend argument is deprecated in drupal:11.2.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3497341', E_USER_DEPRECATED);
core/modules/user/src/Theme/AdminNegotiator.php:61:      @trigger_error("Passing the $deprecated_service_name (entity_type.manager service) to AdminNegotiator is deprecated in drupal:11.2.0 and will be removed in drupal:12.0.0. There is no replacement for this service, as it is not used. See https://www.drupal.org/project/drupal/issues/3501727", E_USER_DEPRECATED);
core/modules/block_content/src/Plugin/Block/BlockContentBlock.php:55:      @trigger_error('Calling ' . __CLASS__ . ' constructor without the $entityRepository argument is deprecated in drupal:11.3.0 and it will be required in drupal:12.0.0. See https://www.drupal.org/project/drupal/issues/3175985', E_USER_DEPRECATED);
core/modules/node/src/Controller/NodePreviewController.php:79:    @trigger_error(__METHOD__ . ' is deprecated in drupal:11.2.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/project/drupal/issues/3024386', E_USER_DEPRECATED);

It's a little hard to follow all the #s in the summary and comments. I'll have to figure out if all these remaining hits are expected...

nicxvan’s picture

nicxvan commented

Ok I also went through this as well.

My approach was a little different, I just went through the edited ones and made sure they linked to the appropriate CR.

I checked @dww's edits and they looked good.

There are 10 changes, but only 9 marked as changed in the list in the issue summary.

I also confirmed each of the ones marked as no CR do not have a CR and reference the right issue.

I think this is ready if we figure out why 9 are listed as changed but 10 are actually changed.

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

It's core/modules/system/tests/modules/module_autoload_test/module_autoload_test.services.yml

The ones in 15 are accounted for in the IS, there are no CRs for those issues.

I think this is ready, let's get these updated.

quietone’s picture

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

With this applied I ran the script to check URLs that is in the parent issue. That found another 404 related to these changes. It is in the doc block for a deprecation, not in the trigger_error message. The script found others but they are not related to deprecations, so those will be done in sibling issue.

quietone’s picture

quietone commented
Title: Fix deprecated messages that reference issue instead of change record. » Fix deprecated messages that reference issue instead of change record
dww’s picture

dww
we/he/they
commented
Related issues: +#3563876: TypeError: trigger_error(): Argument #2 ($error_level) must be of type int, string given in trigger_error()

Merge conflicts from #3563876: TypeError: trigger_error(): Argument #2 ($error_level) must be of type int, string given in trigger_error(). Resolved locally and pushed.

Also, marked #3563885: Broken link in trigger_error() as duplicate.

This is probably RTBC.

dww’s picture

dww
we/he/they
commented

I re-confirmed that everything I listed in #15 is indeed in the IS and that none of those issues have CRs. So that's all cool.

Pushed commit 1f657dc5 to change the deprecation messages in LanguageConfigFactoryOverride to say the new constructor params will be required in D12, not removed.

The only other minor nit in the messages touched by this issue is here:

core/modules/user/src/Theme/AdminNegotiator.php:61:      @trigger_error("Passing the $deprecated_service_name (entity_type.manager service) to AdminNegotiator is deprecated in drupal:11.2.0 and will be removed in drupal:12.0.0. There is no replacement for this service, as it is not used. See https://www.drupal.org/project/drupal/issues/3501727", E_USER_DEPRECATED);

I had to open up AdminNegotiator.php to see that $deprecated_service_name is a real variable holding a real class name, so that message should print out something real. 😅 However, we're passing the service to the AdminNegotiator class's __construct() method. Generally, we say something like:

Calling ' . __CLASS__ . ' constructor ...

Should we do the same here?

dww’s picture

dww
we/he/they
commented

Latest pipeline hit this random fail: #3551373: [random test failure] ManageDisplayTest testFormatterUI() and testWidgetUI() Re-running that test (and will be opening an MR in that issue to fix the random fail).

quietone’s picture

quietone commented

I agree the message discussed in #21 isn't following the standard message style. Changing that is out of scope for this issue. But since this didn't get into 11.3.0 let's tidy that up. I have made a change to that one message.

dww’s picture

dww
we/he/they
commented

Re: #23 - okay, cool. Glad we can just fix that here as part of cleaning all these up. I pushed another commit to reword the message to be more like the original message, just including the ::_constructor() stuff I was talking about. Hope that's agreeable. 😅

Should we move the "11.3.0 release target" tag?

ivnish’s picture

ivnish
he/him
commented

@dww

https://git.drupalcode.org/project/drupal/-/merge_requests/14066/diffs#d...

I see 'E_USER_DEPRECATED' with quotes again. Should be without

dww’s picture

dww
we/he/they
commented

Re: #25: Totally agree. The version of that merge conflict I had pushed to the MR removed the quotes:

550f2970

Apparently, @quietone didn't pull my changes when they worked on #23, re-resolved the conflict themselves, and re-added the quotes:

2249869e

Then they added 179b8f29

They didn't mention any of this in their comments, and I didn't think to re-check the merge conflict I had already resolved, and only noticed some concerns with 179b8f29 and pushed 7605cf7f to address those.

Regardless, thanks for catching the error! It would have been really sad had we committed this and re-broken #3563876: TypeError: trigger_error(): Argument #2 ($error_level) must be of type int, string given in trigger_error().

Generally, I'm glad we no longer require "deprecation tests" that only trigger deprecations, but at this point, I'm sort of wishing we had a test that was calling getCountNewComments() and expecting a deprecation, not a fatal error. 😅

dww’s picture

dww
we/he/they
commented

I realized the sequence of events described in #26 means we also lost commit 1f657dc5 from comment #21. Just cherry-picked that and pushed.

I just re-reviewed the current MR changes. I think we're RTBC, but I'm not qualified to formally move the status now that I've been so involved in the code. 😅

dww’s picture

dww
we/he/they
commented

Needed a rebase for #3557585: Update to Composer 2.9.2

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

I verified the last couple of changes since this issue was set to RTBC. The new changes look appropriate to me, including the ones that are technically out of scope. I agree that we should just fix them here while we're working on these messages.

I'm sorry about the added CommentManager one since that was something I should have caught on review of that issue.

  • catch committed 2b4f7fb6 on 11.x 
    fix: #3560560 Fix deprecated messages that reference issue instead of...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Strange that a couple of these were linking to the right node ID but the wrong URL, maybe some kind of manual copypasta.

The MR itself looks good. Some of the linked CRs did not. 😅 I did some edits as I was confirming that each linked CR matched the deprecation message. Sometimes it was cosmetic formatting, but in some cases, the CR didn't mention things like constructor params changing.

So for most constructor deprecations, we don't require a CR at all, because the deprecation message has as much information as it's really possible to write about it, and because constructor deprecations are 'best effort' since they're supposed to be @internal. I think in these cases what's happened is a wider change has a resulted in a change record, and the change also introduced a constructor deprecation, but that's not really the important thing in the CR. It's fine to add the information, but we shouldn't introduce an expectation that it's in there, because there isn't one.

Committed/pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !14066

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

quietone commented
Issue tags: -11.3.0 release target +11.3.0 release priority