WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 8 days ago

#44225 closed defect (bug) (fixed)

PHPCS: Assignment must be the first block of code on a line

Reported by: subrataemfluence Owned by: desrosj
Milestone: 5.1 Priority: normal
Severity: normal Version: trunk
Component: General Keywords: has-patch commit
Focuses: coding-standards Cc:

Description

Although WordPress-Core standard is reporting this as an error, I would like to ask if the following block of code was overlooked and kept intentionally.

File: wp-admin/includes/plugin.php Line number: 146

The code block being reported is:

<?php
$allowed_tags = $allowed_tags_in_links = array(
    'abbr'    => array( 'title' => true ),
    'acronym' => array( 'title' => true ),
    'code'    => true,
    'em'      => true,
    'strong'  => true,
);

While according to the standards it should be:

<?php
$allowed_tags_in_links = array(
    'abbr'    => array( 'title' => true ),
    'acronym' => array( 'title' => true ),
    'code'    => true,
    'em'      => true,
    'strong'  => true,
);

$allowed_tags      = $allowed_tags_in_links;

Does this create additional overhead if the declaration and assignment are done in two lines? I have uploaded a patch to meet WordPress-Core standard but will love to know if this is really a necessity! And if not, why.

Attachments (1)

44225.diff (636 bytes) - added by subrataemfluence 8 months ago.

Download all attachments as: .zip

Change History (9)

#1 @GaryJ
8 months ago

Patch looks good to me :-)

#2 @jrf
8 months ago

  • Keywords 2nd-opinion removed

I would like to ask if the following block of code was overlooked and kept intentionally.

@subrataemfluence No, this block wasn't kept intentionally, but these kind of errors can not be auto-fixed by the tooling we used to clean up the code as they generally need a developer to check what is the best course of action.

So far, mostly the errors and warnings which can be auto-fixed have made it into core. Most manual fixes still need to be done.

Does this create additional overhead if the declaration and assignment are done in two lines?

It creates additional overhead in the head of developers who can't find the assignment ;-)

For PHP it makes no difference at all. This rule is about code readability.

I have uploaded a patch to meet WordPress-Core standard

Thanks for that and I concur with @GaryJ that the patch looks good. I unfortunately can't add the commit keyword.

#3 @desrosj
8 months ago

  • Keywords commit added

#4 @desrosj
8 months ago

  • Milestone changed from Awaiting Review to 4.9.7

#5 @netweb
8 months ago

  • Milestone changed from 4.9.7 to 5.0

Coding standards tickets/commits need to only go into the 5.0 (or greater) milestone as #41057 was only committed to /trunk aka 5.0

#6 @pento
3 months ago

  • Milestone changed from 5.0 to 5.1

#7 @desrosj
8 days ago

  • Owner set to desrosj
  • Status changed from new to reviewing

#8 @desrosj
8 days ago

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

In 44593:

Coding Standards: Assignments must be the first block of code on a line.

Props subrataemfluence.
Fixes #44225.

Note: See TracTickets for help on using tickets.