Make WordPress Core

#56754 closed defect (bug) (fixed)

Use consistent, coding standard compliant naming for category ID variables

Reported by: hilayt24's profile hilayt24 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch
Focuses: coding-standards Cc:

Description

In "app/public/wp-admin/includes/taxonomy.php" while scanning I found the following errors and warnings.

189 | ERROR   | Variable "$cat_ID" is not in valid snake_case format, try "$cat_i_d"
 191 | ERROR   | Variable "$cat_ID" is not in valid snake_case format, try "$cat_i_d"
 191 | WARNING | Found: ==. Use strict comparisons (=== or !==).
 196 | ERROR   | Variable "$cat_ID" is not in valid snake_case format, try "$cat_i_d"

WordPress Version : 6.0.2
PHPCS Standard : WordPress-Core

Attachments (2)

#56754.patch (944 bytes) - added by hilayt24 22 months ago.
I fixed the PHPCS errors and checked the changes in the site also.
56754.diff (2.1 KB) - added by viralsampat 22 months ago.
I have added condition and resolved PHPCS errors.

Download all attachments as: .zip

Change History (18)

@hilayt24
22 months ago

I fixed the PHPCS errors and checked the changes in the site also.

@viralsampat
22 months ago

I have added condition and resolved PHPCS errors.

#1 @desrosj
21 months ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 6.2

#2 @desrosj
21 months ago

  • Summary changed from While scanning through the files for the standards I found errors and warnings. to Use consistent, coding standard compliant naming for category ID variables

Related: [52958].

#3 @robinwpdeveloper
18 months ago

Thanks everyone who added patch.

Patch: https://core.trac.wordpress.org/attachment/ticket/56754/56754.diff
I am not sure if we need the empty check or not.
Also !empty should be ! empty to maintain the coding standard if I am not wrong.


Patch: https://core.trac.wordpress.org/attachment/ticket/56754/%2356754.patch
Looks straight forward to me and good.

#4 @tanazmasaba
18 months ago

Hi, I have searched $cat_ID throughout the full project in my local setup and I have found this in 3 different files, and total 13 occurrences.

Here are the file locations:

  1. wordpress-develop/src/wp-admin/includes/taxonomy.php
  2. wordpress-develop/src/wp-includes/deprecated.php
  3. wordpress-develop/src/wp-includes/taxonomy.php

I think it would be better if we could fix in all 3 files.

Screenshot: https://d.pr/i/gf7c6l

#5 @robinwpdeveloper
18 months ago

Hi @costdev can you help us moving this ticket forward?

#7 follow-up: @costdev
18 months ago

Hi @robinwpdeveloper, thanks for the ping!

I agree that #56754.patch is more straightforward, as the other patch contains a condition and unrelated inline comments.

Thanks for the PR which verified no tests were broken by the changes!

I've left a review noting that we can't change function parameter casing due to named parameters in PHP 8, which are case-sensitive.

Can you update the PR to remove the changes in src/wp-includes/deprecated.php and src/wp-includes/taxonomy.php?

Other than that, I think the PR is good to go.

@robinwpdeveloper commented on PR #3961:


18 months ago
#8

Thanks @costdev
Changes are made as per feedback.

#9 @robinwpdeveloper
18 months ago

Changes are made as per feedback.
I think we are good to go now.

Last edited 18 months ago by robinwpdeveloper (previous) (diff)

#10 @SergeyBiryukov
18 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#11 in reply to: ↑ 7 @SergeyBiryukov
18 months ago

Replying to costdev:

I've left a review noting that we can't change function parameter casing due to named parameters in PHP 8, which are case-sensitive.

That's correct, but as noted in the PHP 8.0 dev note, WordPress does not support named parameters at this time:

Using named parameters when calling WordPress functions and class methods is explicitly not supported and highly discouraged until this audit can be completed, as during the audit, parameter names are subject to change without notice. When this audit has been completed, it will be announced in a future developer note.

See tickets #51553, #55327, #55650, #56788 for continued work on this.

So I think we can include those parameter renamings here too.

#12 @SergeyBiryukov
18 months ago

In 55190:

Coding Standards: Rename the $cat_ID variable to $cat_id in wp_update_category().

This resolves a few WPCS warnings:

Variable "$cat_ID" is not in valid snake_case format, try "$cat_i_d"

Follow-up to [2695], [4490], [52958].

Props hilayt24, viralsampat, desrosj, robinwpdeveloper, tanazmasaba, costdev, SergeyBiryukov.
See #56754.

@SergeyBiryukov commented on PR #3961:


18 months ago
#13

Thanks for the PR! Merged in r55190.

#14 @SergeyBiryukov
18 months ago

  • Component changed from General to Taxonomy

#15 @costdev
18 months ago

@SergeyBiryukov I wasn't aware of that! Thanks for the info!

#16 @SergeyBiryukov
17 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 55334:

Coding Standards: Rename the remaining $cat_ID variables to $cat_id.

This resolves a few WPCS warnings:

Variable "$cat_ID" is not in valid snake_case format, try "$cat_i_d"

Affected functions:

  • wp_delete_category()
  • get_category_rss_link()
  • get_catname()

Follow-up to [836], [2068], [2551], [2695], [6365], [10959], [52958], [55190].

Fixes #56754.

Note: See TracTickets for help on using tickets.