Part of #2571965: [meta] Fix coding standards in core.

Here we should target Drupal.WhiteSpace.ScopeClosingBrace and Generic.Functions.OpeningFunctionBraceKernighanRitchie as Squiz.Functions.MultiLineFunctionDeclaration is fixed in #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard.

Approach

We are testing coding standards with PHP CodeSniffer, using the Drupal coding standards from the Coder module. Both of these packages are not installed in Drupal core. We need to do a couple of steps in order to download and configure them so we can run a coding standards check.

Step 1: Add the coding standard to the whitelist

Every coding standard is identified by a "sniff". For example, an imaginary coding standard that would require all llamas to be placed inside a square bracket fence would be called the "Drupal.AnimalControlStructure.BracketedFence sniff". There are dozens of such coding standards, and to make the work easier we have started by only whitelisting the sniffs that pass. For the moment all coding standards that are not yet fixed are simply skipped during the test.

Open the file core/phpcs.xml.dist and add a line for the sniff of this ticket. The sniff name is in the issue title. Make sure your patch will include the addition of this line.

Step 2: Install PHP CodeSniffer and the ruleset from the Coder module

Both of these packages are not installed by default in Drupal core, so we need to download them. This can be done with Composer, from the root folder of your Drupal installation:

$ composer require drupal/coder squizlabs/php_codesniffer
$ ./vendor/bin/phpcs --config-set installed_paths ../../drupal/coder/coder_sniffer

Once you have installed the phpcs package, you can list all the sniffs available to you like this:

$ ./vendor/bin/phpcs --standard=Drupal -e

This will give you a big list of sniffs, and the Drupal-based ones should be present.

Step 3: Prepare the phpcs.xml file

To speed up the testing you should make a copy of the file phpcs.xml.dist (in the core/ folder) and save it as phpcs.xml. This is the configuration file for PHP CodeSniffer.

We only want this phpcs.xml file to specify the sniff we're interested in. So we need to remove all the rule items, and add only our own sniff's rule. Rule items look like this:

<rule ref="Drupal.Classes.UnusedUseStatement"/>

Remove all of them, and add only the sniff from this issue title. This will make sure that our tests run quickly, and are not going to contain any output from unrelated sniffs.

Step 4: Run the test

Now you are ready to run the test! From within the core/ folder, run the following command to launch the test:

$ cd core/
$ ../vendor/bin/phpcs -p

This takes a couple of minutes. The -p flag shows the progress, so you have a bunch of nice dots to look at while it is running.

Step 5: Fix the failures

When the test is complete it will present you a list of all the files that contain violations of your sniff, and the line numbers where the violations occur. You could fix all of these manually, but thankfully phpcbf can fix many of them. You can call phpcbf like this:

$ ../vendor/bin/phpcbf

This will fix the errors in place. You can then make a diff of the changes using git. You can also re-run the test with phpcs and determine if that fixed all of them.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

attiks created an issue. See original summary.

DuaelFr’s picture

Issue tags: -Novice

As agreed between the mentors at Drupalcon, according to issues to avoid for novices, I am untagging this issue as "Beginner". This issue contains changes across a very wide range of files and might create too many other patches to need to be rerolled at this particular time. This patch has an automated way to be rerolled later so better to implement it after Drupalcon.

pfrenssen’s picture

Issue summary: View changes

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

Status: Needs review » Needs work

Needs reworking our phpcs.xml.dist is now inclusinve so the rule will have to be added rather than removed.

alexpott’s picture

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/NativeUpsert.php
@@ -93,7 +93,8 @@ public function __toString() {
-    $insert_fields = array_map(function($f) { return $this->connection->escapeField($f); }, $insert_fields);
+    $insert_fields = array_map(function($f) { return $this->connection->escapeField($f); ¶
+    }, $insert_fields);

Also anonymous functions look a mess.

pfrenssen’s picture

Issue summary: View changes
pfrenssen’s picture

Issue summary: View changes
andypost’s picture

Version: 8.1.x-dev » 8.2.x-dev
Status: Needs work » Needs review
FileSize
56.62 KB

Inline functions still broken, reroll

pfrenssen’s picture

Issue summary: View changes
alexpott’s picture

We need problem discuss anonymous function coding standards. I don't think there are any. In the interim I think we need to fix the rule to not mess with anonymous functions.

alexpott’s picture

Status: Needs review » Needs work
dawehner’s picture

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

This is apparently currently postponed on: #2795669: Incorrect autofix of anonymous function body in Drupal.WhiteSpace.ScopeClosingBrace

We got around the overly aggressive fixer in #2676540: Fixer should not delete elaborate @file doc comments for 8.1.x, but IIRC we did that by patching core wherever the fixer would have over-fixed it. While the scope of that rule's implementation was very big, it was also straightforward since it was always at the top of a file. This rule is way more fussy, so I can see postponing it on making the fixer better.

pfrenssen’s picture

Status: Postponed » Needs work

This can be reopened. #2795669: Incorrect autofix of anonymous function body in Drupal.WhiteSpace.ScopeClosingBrace has been closed with "works as designed". @klausi explains how we should tackle it:

You need to run the fixer for closing curly braces AND opening curly braces, then you get the desired autofix (the last code block in the issue summary). The problem is that core only enables part of the Drupal standard, so the fixer can only fix part of the problems.

If you execute the following command then ConfigManager.php will be auto-fixed just fine:

phpcbf --standard=Drupal core/lib/Drupal/Core/Config/ConfigManager.php

xjm’s picture

If you execute the following command then ConfigManager.php will be auto-fixed just fine:

phpcbf --standard=Drupal core/lib/Drupal/Core/Config/ConfigManager.php

Is it possible to provide a more scoped command to auto-fix? This will fix every standards violation, which is not feasible or in scope to deploy the new closure standards I think.

pfrenssen’s picture

Status: Needs work » Postponed

I've been looking into this. It's not possible to fix this one standard (whitespace around the scope closing brace) on its own without making the code base look a lot worse. If the closing brace is not correctly placed, this usually means that the opening brace is also incorrect.

As commented before this mainly affects closures. I suggest to expand the scope of this issue to fix the coding standards for closures.

There is also a new version of Coder released which adds a bunch of new sniffs. Those need to be fixed or excluded from the ruleset before we can continue with this.

Postponing on #2803779: Update Coder to 8.2.10 and fix errors from improved sniffs.

pfrenssen’s picture

@xjm asks in comment 17:

Is it possible to provide a more scoped command to auto-fix? This will fix every standards violation, which is not feasible or in scope to deploy the new closure standards I think.

Here is a patch to the ruleset that will autofix the closures. We can't run it yet though before #2803779: Update Coder to 8.2.10 and fix errors from improved sniffs is in.

pfrenssen’s picture

Title: Fix 'Drupal.WhiteSpace.ScopeClosingBrace' coding standard » Fix coding standard for closures

Updating title to match the new scope. If anyone does not agree with this, please feel free to revert this.

We need a couple of rules to fix the matching opening and closing braces, and just adding the OpeningFunctionBrace sniff will make this autofix closures entirely. I think the additional work to add this one single sniff is quite small in comparison to the benefit of being able to fix all closures at once in core and match the new coding standard.

pfrenssen’s picture

This is blocked by #2803779: Update Coder to 8.2.10 and fix errors from improved sniffs which is in turn blocked by #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core. In that last issue Coder is added as a dependency of core. This is probably not something that is acceptable so late in the release cycle for 8.2.x, so this means that it is unfortunately unlikely to get the coding standards for closures fixed in 8.2.x :(

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

Version: 8.4.x-dev » 8.3.x-dev
Status: Postponed » Active
Issue tags: +rc target

Coder is updated now, so this would be acceptable as an 8.3.x RC target.

Mile23’s picture

Using phpcbf and the changed config in #19, you sometimes get changes like this:

-        array_map(function ($entity) { return $entity->getConfigDependencyName(); }, $affected_dependencies[$dependency_type]),
+        array_map(function ($entity) {
+          return $entity->getConfigDependencyName(); 
+        }, $affected_dependencies[$dependency_type]),

Note that there's a stray space at the end of the return line.

I tried this with 8.4.x (which requires a different version of Coder) for the same result.

pfrenssen’s picture

Running phpcbf a second time will fix the stray spaces I think :)

hgunicamp’s picture

I posted a new patch fixing the rejected changes in '2572795-19.patch' one.
I'm keeping 8.3.x branch but it also worked in 8.4.x one.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mfernea’s picture

Squiz.Functions.MultiLineFunctionDeclaration is also fixed in #2901726: Fix 'Squiz.Functions.MultiLineFunctionDeclaration' coding standard. Either we close that one or we don't target Squiz.Functions.MultiLineFunctionDeclaration in this issue. I think we should also mention in the issue title what sniffs we are targeting here.

mfernea’s picture

Title: Fix coding standard for closures » Fix coding standard for closures - Drupal.WhiteSpace.ScopeClosingBrace and Generic.Functions.OpeningFunctionBraceKernighanRitchie
Issue summary: View changes
Status: Needs review » Needs work

I updated the title to reflect the scope of the issue. I also updated the summary.

mfernea’s picture

Version: 8.4.x-dev » 8.5.x-dev
Status: Needs work » Needs review
FileSize
18.07 KB

Here is the patch.

borisson_’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/tests/src/Kernel/Plugin/StyleGridTest.php
@@ -69,11 +69,16 @@ protected function assertGrid(ViewExecutable $view, $alignment, $columns) {
+      case 5: $width = '20'; ¶
+        break;
+      case 4: $width = '25'; ¶
+        break;
+      case 3: $width = '33.3333'; ¶
+        break;
+      case 2: $width = '50'; ¶
+        break;
+      case 1: $width = '100'; ¶
+        break;

This leaves whitespace at the end of those lines, that looks off.

mfernea’s picture

Status: Needs work » Needs review
FileSize
18.06 KB
778 bytes

Good spot, I didn't noticed. Here is the patch and interdiff.

osab’s picture

Assigned: Unassigned » osab
Issue tags: +kharkiv2017
osab’s picture

Assigned: osab » Unassigned
Status: Needs review » Reviewed & tested by the community

tested, only warnings about empty files :

PHP CODE SNIFFER REPORT SUMMARY
----------------------------------------------------------------------
FILE ERRORS WARNINGS
----------------------------------------------------------------------
...m/tests/fixtures/HtaccessTest/access_test.install 0 1
...ystem/tests/fixtures/HtaccessTest/access_test.inc 0 1
...em/tests/fixtures/HtaccessTest/access_test.module 0 1
...m/tests/fixtures/HtaccessTest/access_test.tpl.php 0 1
...m/tests/fixtures/HtaccessTest/access_test.profile 0 1
...tem/tests/fixtures/HtaccessTest/access_test.theme 0 1
----------------------------------------------------------------------
A TOTAL OF 0 ERRORS AND 6 WARNINGS WERE FOUND IN 6 FILES
----------------------------------------------------------------------

The last submitted patch, 19: 2572795-19.patch, failed testing. View results

mfernea’s picture

@osab According to your comment this should not have RTBC status. We should not get any errors or warnings. But, are you sure you've tested with the full phpcs.xml.dist?

Mile23’s picture

These commands lead me to believe this patch should be RTBC:

$ git fetch
$ git checkout 8.5.x
$ git pull origin 8.5.x
$ git apply [the patch]
$ composer install
$ cd core/
$ ../vendor/bin/phpcs -ps

The patch looks good, so +1.

catch’s picture

Status: Reviewed & tested by the community » Needs review
  1. +++ b/core/lib/Drupal/Component/DependencyInjection/Container.php
    @@ -586,8 +586,6 @@ public function getServiceIds() {
    +  private function __clone() {}
    

    If we do this.

  2. +++ b/core/modules/aggregator/tests/src/Unit/Plugin/AggregatorPluginSettingsBaseTest.php
    @@ -109,5 +109,6 @@ public function testSettingsForm() {
     
     if (!function_exists('drupal_set_message')) {
    -  function drupal_set_message() {}
    +  function drupal_set_message() {
    +  }
     }
    

    Why do we do this?

    Do we have explicit coding standards that say empty procedural functions need the newline, or is it the CS rule enforcing it?

mfernea’s picture

{} is allowed only in classes. Not procedural code nor traits. This is what current implementation of Drupal CS does.
In classes, we can also use the style of procedural code. Drupal CS doesn't complain.

I can't find anything related to functions with no body in https://www.drupal.org/docs/develop/standards.

alexpott’s picture

Status: Needs review » Postponed

The trait vs class difference is really odd here. As a developer that rule would be impossible to learn. I think before we proceed here we should determine what the standard should be and get the coder rules adjusted accordingly. I've opened #2915420: Empty functions coding standard to discuss and come up with a solution. I think this issue needs to be postponed on that one.

mfernea’s picture

As per my comment at https://www.drupal.org/node/2915420#comment-12296196, both styles are allowed for classes. So, I would adjust the patch to make it consistent and also pass the sniffs.

mfernea’s picture

mfernea’s picture

alexpott’s picture

Status: Postponed » Needs review

@mfernea ah okay my bad I misunderstood that classes are allowed the closing brace on the next line too. The current patch looks good and this does not need to be postponed on #2915420: Empty functions coding standard

zaporylie’s picture

Status: Needs review » Reviewed & tested by the community

IMO #43 is good to go. It applies cleanly and raises no CS errors, and given #41 and #44 does not need to be postponed.

Thanks!

  • xjm committed c422bdf on 8.5.x
    Issue #2572795 by mfernea, pfrenssen, attiks, andypost, alexpott, xjm,...
xjm’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed to 8.5.x. Thanks!

We can also backport the cleanups to 8.4.x since they're basically just whitespace changes, but first we need to remove the changes to phpcs.xml.dist as per #2919983: Restore the 8.4.0 phpcs.xml.dist on 8.4.x.

  • xjm committed edf57d8 on 8.5.x
    Revert "Issue #2572795 by mfernea, pfrenssen, attiks, andypost, alexpott...
xjm’s picture

Version: 8.4.x-dev » 8.5.x-dev
Status: Patch (to be ported) » Needs work

Hm, actually I'm not totally sure what happened because I was sure I checked all of core before committing this, but this does introduce a number of fails:

FILE: ...rupal/Core/Http/Exception/CacheableAccessDeniedHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: .../Drupal/Core/Http/Exception/CacheableBadRequestHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ib/Drupal/Core/Http/Exception/CacheableConflictHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...re/lib/Drupal/Core/Http/Exception/CacheableGoneHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...r/core/lib/Drupal/Core/Http/Exception/CacheableHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...pal/Core/Http/Exception/CacheableLengthRequiredHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...l/Core/Http/Exception/CacheableMethodNotAllowedHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...upal/Core/Http/Exception/CacheableNotAcceptableHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ib/Drupal/Core/Http/Exception/CacheableNotFoundHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...Core/Http/Exception/CacheablePreconditionFailedHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...re/Http/Exception/CacheablePreconditionRequiredHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...Core/Http/Exception/CacheableServiceUnavailableHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...al/Core/Http/Exception/CacheableTooManyRequestsHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...rupal/Core/Http/Exception/CacheableUnauthorizedHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ore/Http/Exception/CacheableUnprocessableEntityHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...re/Http/Exception/CacheableUnsupportedMediaTypeHttpException.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 19 | ERROR | [x] Expected 1 space before opening brace; found 2
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 1 mins, 47.41 secs; Memory: 94Mb

So it looks like there's more to fix before we commit this and I've reverted it for now. Thanks!

zaporylie’s picture

Hmm... same for me, and I actually have tried #43 before changing to RTBC so not sure what have happened here. I'll give it a try.

zaporylie’s picture

Seems like all of them were introduced in #2920001: Add cacheable HTTP exceptions: Symfony HTTP exceptions + Drupal cacheability metadata committed earlier today.

zaporylie’s picture

Status: Needs work » Needs review
FileSize
13.75 KB
29.27 KB

phpcbf for the rescue 💪

andypost’s picture

xjm’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

That explains it; thanks @zaporylie.

Committed and pushed to 8.5.x (again) and moving back to 8.4.x for the backport without the rule change. Thanks!

  • xjm committed 91e668f on 8.5.x
    Issue #2572795 by mfernea, zaporylie, pfrenssen, andypost, attiks,...
zaporylie’s picture

Status: Patch (to be ported) » Needs review
FileSize
14.33 KB
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php"/>
  <rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie">
    <properties>
      <property name="checkClosures" value="true"/>
    </properties>
  </rule>
  1. Added rule above to phpcs.xml.dist
  2. phpcbf
  3. Fixed #31
  4. phpcs
  5. Removed rule from phpcs.xml.dist
andypost’s picture

Status: Needs review » Reviewed & tested by the community

Changes are the same but without rule like #54 said

  • xjm committed f34441b on 8.4.x
    Issue #2572795 by mfernea, zaporylie, pfrenssen, andypost, attiks,...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @zaporylie. I confirmed that all the changes are within the scope of the rule, reviewed with git diff --staged --color-words to ensure there were no other incidental changes, and ran the test suite against core again just to be sure.

Committed and pushed to 8.4.x. Thanks!

Status: Fixed » Closed (fixed)

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