Problem/Motivation

We added phpcs.dist.xml in #2573377: Add our own phpcs.xml file we chose to exclude rules. This was because we thought it was better to get improvements from drupal/coder and phpcs automatically. But this is problematic because HEAD could just start failing completely out of our control - for example #2666112: phpcs has a new sniff: Generic.PHP.LowerCaseKeyword

Proposed resolution

I discussed this with @xjm and we agreed that (obviously) core can't and doesn't want to hold back development on drupal/coder or phpcs, therefore, we have swap the file around to only detail what sniffs are currently passing.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
5.28 KB

Here's a patch that swaps this around. We need #2620576: fnmatch() is not available on all environments (i.e QNAP QTS) to land so that Drupal.Functions.DiscouragedFunctions passes on the 8.2.6 release of coder.

Output from running test:

8.1.x  alex: ~/dev/sites/drupal8alt.dev/core > phpcs -ps ./
............................................................   60 / 6252 (1%)
............................................................  120 / 6252 (2%)
............................................................  180 / 6252 (3%)
............................................................  240 / 6252 (4%)
............................................................  300 / 6252 (5%)
............................................................  360 / 6252 (6%)
............................................................  420 / 6252 (7%)
............................................................  480 / 6252 (8%)
............................................................  540 / 6252 (9%)
............................................................  600 / 6252 (10%)
.............W.....W........................................  660 / 6252 (11%)
............................................................  720 / 6252 (12%)
............................................................  780 / 6252 (12%)
............................................................  840 / 6252 (13%)
............................................................  900 / 6252 (14%)
............................................................  960 / 6252 (15%)
............................................................ 1020 / 6252 (16%)
............................................................ 1080 / 6252 (17%)
............................................................ 1140 / 6252 (18%)
............................................................ 1200 / 6252 (19%)
............................................................ 1260 / 6252 (20%)
............................................................ 1320 / 6252 (21%)
............................................................ 1380 / 6252 (22%)
............................................................ 1440 / 6252 (23%)
............................................................ 1500 / 6252 (24%)
............................................................ 1560 / 6252 (25%)
............................................................ 1620 / 6252 (26%)
............................................................ 1680 / 6252 (27%)
............................................................ 1740 / 6252 (28%)
............................................................ 1800 / 6252 (29%)
............................................................ 1860 / 6252 (30%)
............................................................ 1920 / 6252 (31%)
............................................................ 1980 / 6252 (32%)
......................................W..................... 2040 / 6252 (33%)
............................................................ 2100 / 6252 (34%)
............................................................ 2160 / 6252 (35%)
............................................................ 2220 / 6252 (36%)
............................................................ 2280 / 6252 (36%)
............................................................ 2340 / 6252 (37%)
............................................................ 2400 / 6252 (38%)
............................................................ 2460 / 6252 (39%)
............................................................ 2520 / 6252 (40%)
............................................................ 2580 / 6252 (41%)
............................................................ 2640 / 6252 (42%)
............................................................ 2700 / 6252 (43%)
............................................................ 2760 / 6252 (44%)
............................................................ 2820 / 6252 (45%)
............................................................ 2880 / 6252 (46%)
............................................................ 2940 / 6252 (47%)
............................................................ 3000 / 6252 (48%)
............................................................ 3060 / 6252 (49%)
............................................................ 3120 / 6252 (50%)
............................................................ 3180 / 6252 (51%)
............................................................ 3240 / 6252 (52%)
............................................................ 3300 / 6252 (53%)
.W.......................................................... 3360 / 6252 (54%)
............................................................ 3420 / 6252 (55%)
............................................................ 3480 / 6252 (56%)
............................................................ 3540 / 6252 (57%)
............................................................ 3600 / 6252 (58%)
............................................................ 3660 / 6252 (59%)
............................................................ 3720 / 6252 (60%)
............................................................ 3780 / 6252 (60%)
............................................................ 3840 / 6252 (61%)
............................................................ 3900 / 6252 (62%)
............................................................ 3960 / 6252 (63%)
............................................................ 4020 / 6252 (64%)
............................................................ 4080 / 6252 (65%)
............................................................ 4140 / 6252 (66%)
............................................................ 4200 / 6252 (67%)
............................................................ 4260 / 6252 (68%)
............................................................ 4320 / 6252 (69%)
............................................................ 4380 / 6252 (70%)
............................................................ 4440 / 6252 (71%)
............................................................ 4500 / 6252 (72%)
............................................................ 4560 / 6252 (73%)
............................................................ 4620 / 6252 (74%)
............................................................ 4680 / 6252 (75%)
............................................................ 4740 / 6252 (76%)
............................................................ 4800 / 6252 (77%)
............................................................ 4860 / 6252 (78%)
............................................................ 4920 / 6252 (79%)
............................................................ 4980 / 6252 (80%)
............................................................ 5040 / 6252 (81%)
............................................................ 5100 / 6252 (82%)
............................................................ 5160 / 6252 (83%)
............................................................ 5220 / 6252 (83%)
............................................................ 5280 / 6252 (84%)
............................................................ 5340 / 6252 (85%)
............................................................ 5400 / 6252 (86%)
............................................................ 5460 / 6252 (87%)
............................................................ 5520 / 6252 (88%)
............................................................ 5580 / 6252 (89%)
............................................................ 5640 / 6252 (90%)
............................................................ 5700 / 6252 (91%)
............................................................ 5760 / 6252 (92%)
............................................................ 5820 / 6252 (93%)
............................................................ 5880 / 6252 (94%)
............................................................ 5940 / 6252 (95%)
............................................................ 6000 / 6252 (96%)
............................................................ 6060 / 6252 (97%)
............................................................ 6120 / 6252 (98%)
............................................................ 6180 / 6252 (99%)
............................................................ 6240 / 6252 (100%)
............


FILE: ...sites/drupal8alt.dev/core/lib/Drupal/Core/Config/FileStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 211 | WARNING | The use of function fnmatch() is discouraged
     |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
 312 | WARNING | The use of function fnmatch() is discouraged
     |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
----------------------------------------------------------------------


FILE: ...es/drupal8alt.dev/core/lib/Drupal/Core/Config/InstallStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 206 | WARNING | The use of function fnmatch() is discouraged
     |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
 233 | WARNING | The use of function fnmatch() is discouraged
     |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
----------------------------------------------------------------------


FILE: ...pal8alt.dev/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 338 | WARNING | The use of function fnmatch() is discouraged
     |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
----------------------------------------------------------------------


FILE: ...upal8alt.dev/core/modules/migrate/src/MigrateTemplateStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 77 | WARNING | The use of function fnmatch() is discouraged
    |         | (Drupal.Functions.DiscouragedFunctions.Discouraged)
----------------------------------------------------------------------

Time: 2 mins, 42.63 secs; Memory: 540Mb
pfrenssen’s picture

Status: Needs review » Postponed

Alex, I guess you have symlinked the Drupal coding standards inside the PHPCS standards folder. I have a local installation of PHPCS and the Coder module managed by Composer. I had to change the file to the following:

  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/CSS/ClassDefinitionNameSpacingSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/CSS/ColourDefinitionSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentStarSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/ControlStructures/ElseIfSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Files/TxtFileLineLengthSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Functions/DiscouragedFunctionsSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/InfoFiles/AutoAddedKeysSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/InfoFiles/ClassFilesSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/InfoFiles/DuplicateEntrySniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/InfoFiles/RequiredSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/EmptyInstallSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/FunctionWatchdogSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/InstallHooksSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/LStringTranslatableSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/PregSecuritySniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/TInHookMenuSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Semantics/TInHookSchemaSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/WhiteSpace/CommaSniff.php"/>
  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/WhiteSpace/ObjectOperatorIndentSniff.php"/>

Then I'm getting the same results:

............................................................   60 / 6224 (1%)
............................................................  120 / 6224 (2%)
............................................................  180 / 6224 (3%)
............................................................  240 / 6224 (4%)
............................................................  300 / 6224 (5%)
............................................................  360 / 6224 (6%)
............................................................  420 / 6224 (7%)
............................................................  480 / 6224 (8%)
............................................................  540 / 6224 (9%)
............................................................  600 / 6224 (10%)
............................................................  660 / 6224 (11%)
............................................................  720 / 6224 (12%)
............................................................  780 / 6224 (13%)
............................................................  840 / 6224 (13%)
............................................................  900 / 6224 (14%)
............................................................  960 / 6224 (15%)
............................................W............... 1020 / 6224 (16%)
............................................................ 1080 / 6224 (17%)
............................................................ 1140 / 6224 (18%)
............................................................ 1200 / 6224 (19%)
............................................................ 1260 / 6224 (20%)
............................................................ 1320 / 6224 (21%)
............................................................ 1380 / 6224 (22%)
............................................................ 1440 / 6224 (23%)
............................................................ 1500 / 6224 (24%)
............................................................ 1560 / 6224 (25%)
............................................................ 1620 / 6224 (26%)
............................................................ 1680 / 6224 (27%)
............................................................ 1740 / 6224 (28%)
............................................................ 1800 / 6224 (29%)
............................................................ 1860 / 6224 (30%)
............................................................ 1920 / 6224 (31%)
............................................................ 1980 / 6224 (32%)
............................................................ 2040 / 6224 (33%)
............................................................ 2100 / 6224 (34%)
............................................................ 2160 / 6224 (35%)
............................................................ 2220 / 6224 (36%)
............................................................ 2280 / 6224 (37%)
............................................................ 2340 / 6224 (38%)
............................................................ 2400 / 6224 (39%)
............................................................ 2460 / 6224 (40%)
............................................................ 2520 / 6224 (40%)
............................................................ 2580 / 6224 (41%)
............................................................ 2640 / 6224 (42%)
............................................................ 2700 / 6224 (43%)
............................................................ 2760 / 6224 (44%)
............................................................ 2820 / 6224 (45%)
............................................................ 2880 / 6224 (46%)
............................................................ 2940 / 6224 (47%)
............................................................ 3000 / 6224 (48%)
.............W.............................................. 3060 / 6224 (49%)
............................................................ 3120 / 6224 (50%)
............................................................ 3180 / 6224 (51%)
............................................................ 3240 / 6224 (52%)
............................................................ 3300 / 6224 (53%)
............................................................ 3360 / 6224 (54%)
............................................................ 3420 / 6224 (55%)
............................................................ 3480 / 6224 (56%)
............................................................ 3540 / 6224 (57%)
............................................................ 3600 / 6224 (58%)
............................................................ 3660 / 6224 (59%)
............................................................ 3720 / 6224 (60%)
............................................................ 3780 / 6224 (61%)
............................................................ 3840 / 6224 (62%)
............................................................ 3900 / 6224 (63%)
............................................................ 3960 / 6224 (64%)
............................................................ 4020 / 6224 (65%)
............................................................ 4080 / 6224 (66%)
............................................................ 4140 / 6224 (67%)
............................................................ 4200 / 6224 (67%)
............................................................ 4260 / 6224 (68%)
............................................................ 4320 / 6224 (69%)
............................................................ 4380 / 6224 (70%)
............................................................ 4440 / 6224 (71%)
............................................................ 4500 / 6224 (72%)
............................................................ 4560 / 6224 (73%)
............................................................ 4620 / 6224 (74%)
............................................................ 4680 / 6224 (75%)
............W............................................... 4740 / 6224 (76%)
.......................W.................................... 4800 / 6224 (77%)
............................................................ 4860 / 6224 (78%)
............................................................ 4920 / 6224 (79%)
............................................................ 4980 / 6224 (80%)
............................................................ 5040 / 6224 (81%)
............................................................ 5100 / 6224 (82%)
............................................................ 5160 / 6224 (83%)
............................................................ 5220 / 6224 (84%)
............................................................ 5280 / 6224 (85%)
............................................................ 5340 / 6224 (86%)
............................................................ 5400 / 6224 (87%)
............................................................ 5460 / 6224 (88%)
............................................................ 5520 / 6224 (89%)
............................................................ 5580 / 6224 (90%)
............................................................ 5640 / 6224 (91%)
............................................................ 5700 / 6224 (92%)
............................................................ 5760 / 6224 (93%)
............................................................ 5820 / 6224 (94%)
............................................................ 5880 / 6224 (94%)
............................................................ 5940 / 6224 (95%)
............................................................ 6000 / 6224 (96%)
............................................................ 6060 / 6224 (97%)
............................................................ 6120 / 6224 (98%)
............................................................ 6180 / 6224 (99%)
............................................


FILE: ...upal/drupal/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 338 | WARNING | The use of function fnmatch() is discouraged
----------------------------------------------------------------------


FILE: ...rupal/drupal/core/modules/migrate/src/MigrateTemplateStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 77 | WARNING | The use of function fnmatch() is discouraged
----------------------------------------------------------------------


FILE: ...eter/v/drupal/drupal/core/lib/Drupal/Core/Config/FileStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 211 | WARNING | The use of function fnmatch() is discouraged
 312 | WARNING | The use of function fnmatch() is discouraged
----------------------------------------------------------------------


FILE: ...r/v/drupal/drupal/core/lib/Drupal/Core/Config/InstallStorage.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 206 | WARNING | The use of function fnmatch() is discouraged
 233 | WARNING | The use of function fnmatch() is discouraged
----------------------------------------------------------------------

Time: 2 mins, 22.45 secs; Memory: 430.01Mb

We can't add Coder to the core Composer dependencies since it is packaged as a Drupal module. As our Composer integration improves this is bound to end up in the modules/ folder instead of the vendor/ folder at some point in the future. The best solution would be to abstract out the code sniffer ruleset into a separate PHP project, so we can add it along with PHPCS as dev-dependencies in our composer file. We would have full control over the exact version of the coding standards that is used to test a specific version of core too. That's way out of scope for this issue though. We should discuss this with Klausi.

So I guess on the testbot PHPCS is also installed with a symlink to the Coder ruleset in its standards folder. This is fine for now. What is most important is that we can run the coding standards checks automatically without breakage due to upstream changes in the coding standards.

Going to mark this as postponed on #2620576: fnmatch() is not available on all environments (i.e QNAP QTS). Once that is in we can run a retest of this before merging.

pfrenssen’s picture

I just checked and Coder is no longer packaged as a Drupal module, it already declares itself as a standard PHP library. That would make it easy to add it as a dev-dependency to core.

alexpott’s picture

Status: Postponed » Needs review

@pfrenssen but then you'd have to make changes to use the current phpcs.xml.dist. phpcs needs access to the Drupal ruleset regardless whether we include or exclude. Therefore, I think we can proceed with this issue because with your current set up we wouldn't have committed the phpcs.xml.dist in the first place.

alexpott’s picture

Rerolled and fixing the things we've inadvertently broken. No interdiff possible cause reroll.

pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community
pfrenssen’s picture

By the way I have discussed the problem I mentioned in comment #3 about the location of the coding standard in our phpcs.xml.dist with the PHP CodeSniffer maintainers. The best solution for the moment is to run the tests with the --runtime-set installed_paths option pointing to the directory that contains the coding standard.

For example if you have installed Coder with composer in the root of the Drupal site:

$ cd core
$ ../vendor/bin/phpcs --runtime-set installed_paths ../../drupal/coder/coder_sniffer/ -p

In the next release of PHP CodeSniffer it will even be easier. This configuration setting can now be used in custom rulesets (ref. PR #899) so anyone that has the coding standards committed in their project can provide their own phpcs.xml that includes core's phpcs.xml.dist and set the path of the coding standards:

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="Drupal">
  <config name="installed_paths" value="../../drupal/coder/coder_sniffer/" />
  <rule ref="./core/phpcs.xml.dist" />
  <arg value="p" />
</ruleset>

Running the coding standards is then as simple as:

$ ./vendor/bin/phpcs

This will be 100% reliable since you can control the exact versions of Coder and PHP CodeSniffer with composer. No unexpected failures when either of the projects releases an update.

  • catch committed 3a18d59 on 8.1.x
    Issue #2666172 by alexpott: PHPCS configuration should include rules and...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

Status: Fixed » Closed (fixed)

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