Problem/Motivation

As per #3440075: Add rule for multi-line function declarations we now allow multi line function declarations. This sniff was released in coder 8.3.24. Issues: #3440075: Add rule for multi-line function declarations and #3440603: Validate trailing comma in multi-line function declaration. We need to enable the correct config for that sniff and make sure all is fine.

Steps to reproduce

Proposed resolution

Update coder
Update phpcs.xml.dist

Remaining tasks

  1. Update coder
  2. Update phpcs.xml.dist
  3. Validate output
  4. Fix issues

User interface changes

n.a.

API changes

n.a.

Data model changes

n.a.

Release notes snippet

The coding standards rule Drupal.Functions.MultiLineFunctionDeclaration has been enabled.

Issue fork drupal-3443117

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:

Comments

bbrala created an issue. See original summary.

bbrala’s picture

Checked against the meta issue, seems nothing is related/adjecent.

bbrala’s picture

After running locally, there are quite a few issues in core regarding this coding standard.

bbrala’s picture

Ran with:

gitlab-ci-local --variable CI_PIPELINE_SOURCE=merge_event "<0001f9f9> PHP Coding standards (PHPCS)" --variable COMPOSER_ROOT_VERSION=10.3.x-dev --needs

Most can be automaticly fixed it seems.

bbrala’s picture

Issue summary: View changes
Status: Active » Needs review

Added extra ignore to DrupalKernel because that had a different failure after update.

Fixed all issues automaticly, yay. Cleaning up a lot of code.

longwave’s picture

Status: Needs review » Needs work

Thanks, MR looks good for 10.3.x, we will also need another MR to apply against 11.x.

quietone’s picture

Title: Coding standard: Allow multi-line function declarations » Fix Drupal.Functions.MultiLineFunctionDeclaration coding standard
Parent issue: #3295249: Allow multi-line function declarations » #2571965: [meta] Fix PHP coding standards in core, stage 1
Related issues: -#2571965: [meta] Fix PHP coding standards in core, stage 1 +#3295249: Allow multi-line function declarations

Yay indeed!

Making the title the same style as other coding standard issues in the core queue. And switching the parent and related.

longwave’s picture

Also DemoUmamiProfileTest has failed twice, I thought it was random so I re-ran the job but it failed again.

longwave’s picture

Opened MR for 11.x.

bbrala’s picture

The drupal 11 MR looks good and is all green. I'll see if I can find out what is failing in 10.3

bbrala’s picture

There is no reason afaik that that test should fail. Rebased, trying again. Also looked if perhaps php version was an issue, but nothing notable... Arg.

bbrala’s picture

Seems there are more failures on that test in other places. Suspecting its a HEAD issue in 10.3.x

bbrala’s picture

The core issue has been fixed. Mr for 10.3 should pass again.

https://www.drupal.org/project/drupal/issues/3443248

bbrala’s picture

Status: Needs work » Needs review
smustgrave’s picture

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 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.

longwave’s picture

Status: Needs work » Needs review
bbrala’s picture

Rebased and fixed 2 remaining (new) issues in 11.x

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 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

Status: Needs work » Needs review

Bot is lying. It applies.

nod_’s picture

Issue tags: +no-needs-review-bot
longwave’s picture

smustgrave’s picture

Version: 10.3.x-dev » 11.x-dev
Status: Needs review » Needs work

MR 7693 appears to need manual rebase.

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

bbrala’s picture

bbrala’s picture

Status: Needs work » Needs review

Updated new code to standard again and fixed a conflict with 11.x

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Yay, this is really nice cleanup, let's land this before the betas.

  • catch committed 4ca16dc1 on 10.3.x
    Issue #3443117 by bbrala, longwave, quietone: Fix Drupal.Functions....

  • catch committed b4ff0795 on 10.4.x
    Issue #3443117 by bbrala, longwave, quietone: Fix Drupal.Functions....

  • catch committed 58ee3dd2 on 11.0.x
    Issue #3443117 by bbrala, longwave, quietone: Fix Drupal.Functions....

  • catch committed b41dfccb on 11.x
    Issue #3443117 by bbrala, longwave, quietone: Fix Drupal.Functions....
catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks great. Might be the quickest we've been compliant with a coding standard after it's agreed? Committed/pushed to 11.x/11.0.x/10.4.x/10.3.x respectively.

bbrala’s picture

Thanks catch. And maybe it is ;)

Status: Fixed » Closed (fixed)

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

teknocat’s picture

In reviewing my current codebase, I can't seem to get drupal/coder 8.3.24 due to the following dependency chain:

  • drush/drush requires chi-teck/drupal-code-generator:^3.0, which installs v3.2 (at the time of this writing)
  • chi-teck/drupal-code-generator 3.2 requires drupal/coder:3.2.23 (a fixed version number, won't be updated)

I have Drush 12.5.2 installed which, at the time of this writing, is the latest version. I'm currently running Drupal 10.3.

Can somebody advise me on how to update? Alternatively I could update my phpcs.dist.xml file to reference globally installed drupal/coder sniffs as well, where I have 8.3.24 installed. However, that doesn't seem like the best approach and is less portable if other devs working on the same project don't also have the sniffs installed globally in the same location as me.

xjm’s picture

We don't release-notes-ify CS rules anymore unless they're functionally disruptive somehow (e.g. requiring typehints or whatnot). Thanks!