Problem/Motivation

When looking at depreactaion messages for rector/update bot i saw the following:

  @trigger_error(__FUNCTION__ . 'is deprecated in drupal:11.4.0 and is removed from drupal:12.0.0. Use \Drupal\ckeditor5\Hook\Ckeditor5Hooks::updateCkeditor5HtmlFilter() instead. See https://www.drupal.org/node/3566774', E_USER_DEPRECATED);

That is actually missing a space which makes the message a bit weird. There are 3 places where this is the case right now. This MR fixes that.

Since this is not released yet, we could fix this quickly. Setting major since its time sensitive.

Working on this also showed that some deprecation messages for __FUNCTION__ and __METHOD__ are missing the "()" characters.

Steps to reproduce

na

Proposed resolution

Add a space when needed
Add parenthesis where they are missing in deprecation messages for __FUNCTION__ and __METHOD__.
Remove parenthesis where they are in deprecation messages for __CLASS__

Remaining tasks

Review
Commit

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3591520-nr-bot_30qm93ht.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3591520

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

bbrala created an issue. See original summary.

bbrala’s picture

bbrala
Location Netherlands
commented
Title: Deprecation message in ckeditor is missing a space » Multiple 11.4.x deprecation messages missing a space

bbrala opened merge request !15838

bbrala’s picture

bbrala
Location Netherlands
commented
Version: main » 11.x-dev
Issue summary: View changes

bbrala opened merge request !15839

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

FWIW we can fix deprecation messages like this even after release - for example I'm pretty sure the links have been wrong before and we only noticed post-release.

nicxvan’s picture

nicxvan commented
Status: Active » Reviewed & tested by the community

I don't have access to the 11.x failures but I expect they are random.

Both branches look good!

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3591520-nr-bot_30qm93ht.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs work » Needs review

Hmm, if that is the case there is way more. A quick search for __FUNCTION__ . ' is deprecated shows quite a few hits. Do we include those here?

Some are 11.4, but also quite a few 11.3

quietone’s picture

quietone commented
Component: ckeditor5.module » other
Status: Needs review » Needs work

Oh rats. Yea, we really should fix them all to conform to policy. I found 8 more on 11.x and 3 on main. Since this can be fixed via the command line, let's do that.

I'll add to my list to check the current RTBC issues for this particular error.

Changing the component to other since this now affects more than one system.

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs work » Needs review
dcam’s picture

dcam commented

Neither of the following commands turned up any additional instances of the problem in the branch for main:

  • grep -r "__FUNCTION__ . 'is" core
  • grep -r "__FUNCTION__ . ' is" core

But the command grep -r "__FUNCTION__ . ' is" core turned up 5 instances in the branch for 11.x. They were located in the following files:

  • core/modules/field_ui/field_ui.module
  • core/modules/node/node.module
  • core/modules/file/file.module
  • core/modules/comment/src/Controller/CommentController.php

Were these omitted intentionally? I don't see anything about them that makes it look like they should be exceptions. Let me know if I missed something.

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Needs work

Hmm no they weren't, good catch. Not sure how i missed those.

quietone’s picture

quietone commented
Title: Multiple 11.4.x deprecation messages missing a space » Fix format of some deprecation messages
Issue summary: View changes
Issue tags: +Coding standards, +11.4.0 release priority

I was hoping to commit this but instead I have updated the MR for #12.

I then noticed more deprecation messages are missing the "()", so I fixed those and updated the issue summary for the changed scope.

quietone’s picture

quietone commented
Status: Needs work » Needs review
godotislate’s picture

godotislate
he/him
commented

Changes on both MRs both look good to me. I am AFK though and have not checked existing branches to make sure there aren't any other instances that need fixing, but either way, these are improvements.

I'm guessing there are no additional instances that need fixing in 11.4.x?

Otherwise, RTBC +1.

dcam’s picture

dcam commented
Status: Needs review » Needs work

I expanded my grep checks to cover the additional cases that were added. I didn't find any more in either branch. But I think I did find a mistake in MR 15838. Please check it.

quietone’s picture

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

Thanks for the prompt reviews! I have updated to include searching for CLASS as well. I have updated the MR accordingly. Tests are passing and I have run the following wee script and didn't find any other problems. 

$ ./check.sh 
+ git checkout 3591520-fix-for-main
Switched to branch '3591520-fix-for-main'
Your branch is up to date with 'drupal-3591520/3591520-fix-for-main'.
+ git grep '__FUNCTION__ . '\''is' core
+ git grep '__FUNCTION__ . '\'' is' core
+ git grep '__METHOD__ . '\''is' core
+ git grep '__METHOD__ . '\'' is' core
+ git grep '__CLASS__ . '\''is' core
+ git grep '__CLASS__ . '\''()' core
+ git checkout 3591520-deprecation-message-in
Switched to branch '3591520-deprecation-message-in'
Your branch is up to date with 'drupal-3591520/3591520-deprecation-message-in'.
+ git grep '__FUNCTION__ . '\''is' core
+ git grep '__FUNCTION__ . '\'' is' core
+ git grep '__METHOD__ . '\''is' core
+ git grep '__METHOD__ . '\'' is' core
+ git grep '__CLASS__ . '\''is' core
+ git grep '__CLASS__ . '\''()' core
dcam’s picture

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

I didn't find any additional issues. This one looks good to me.

godotislate closed merge request !15839

  • godotislate committed 6a004b1f on main 
    fix: #3591520 Fix format of some deprecation messages

By: bbrala
By:...

godotislate closed merge request !15838

  • godotislate committed 87d9861a on 11.x 
    fix: #3591520 Fix format of some deprecation messages

By: bbrala
By:...

  • godotislate committed af90abb9 on 11.4.x 
    fix: #3591520 Fix format of some deprecation messages

By: bbrala
By:...
godotislate’s picture

godotislate
he/him
commented
Version: 11.x-dev » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 6a004b1f main, 87d9861 to 11.x, and af90abb to 11.4.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.

Status: Fixed » Closed (fixed)

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