Problem/Motivation

Running vendor/bin/phpcs -ps --parallel=$(nproc) --standard="core/phpcs.xml.dist" against Drupal 10 produces the following warning:

FILE: /Volumes/dev/drupal/core/modules/locale/locale.module
----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 1063 | WARNING | Unused variable $strings.
      |         | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
----------------------------------------------------------------------------------------------------------

On Drupal 9 the errors are:

FILE: /Volumes/dev/drupal/core/modules/ckeditor5/tests/src/Functional/Update/CKEditor5UpdateImageToolbarItemTest.php
--------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------
 165 | ERROR | [x] Array indentation error, expected 14 spaces but found 12
     |       |     (Drupal.Arrays.Array.ArrayIndentation)
 166 | ERROR | [x] Array indentation error, expected 14 spaces but found 12
     |       |     (Drupal.Arrays.Array.ArrayIndentation)
--------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------


FILE: /Volumes/dev/drupal/core/modules/locale/locale.module
----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 1059 | WARNING | Unused variable $strings.
      |         | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
----------------------------------------------------------------------------------------------------------


FILE: /Volumes/dev/drupal/core/includes/install.inc
-----------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------
 636 | WARNING | Unused variable $pos.
     |         | (DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable)
-----------------------------------------------------------------------------------------------------

Time: 18.32 secs; Memory: 10MB

Not sure we're not caught this...

Steps to reproduce

Run vendor/bin/phpcs -ps --parallel=$(nproc) --standard="core/phpcs.xml.dist"

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3324540-10.x-3.patch3.72 KBalexpott
#4 10.x-2-3-interdiff.txt1.59 KBalexpott
#2 3324540-9.x-2.patch3.72 KBalexpott
#2 3324540-10.x-2.patch2.13 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Active » Needs review
FileSize
3324540-10.x-2.patch2.13 KB
3324540-9.x-2.patch3.72 KB

Also now that PHPCS does parallel running we can leverage that to always run a complete check on DrupalCI.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Component: locale.module » base system
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
FileSize
10.x-2-3-interdiff.txt1.59 KB
3324540-10.x-3.patch3.72 KB

So the two 9.5.x only issues have been fix in 10.x for some reason but not fixed in 9.5.x - I think we should fix them in the same way so here's a 10.x patch to make things consistent.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented

Note we should not backport this to 9.4.x as we don't have the parallel phpcs changes there.

+++ b/core/includes/install.inc
@@ -633,7 +633,7 @@ function drupal_install_system($install_state) {
-    if (($pos = strpos($autoload, 'src/Driver/Database/')) !== FALSE) {
+    if (str_contains($autoload, 'src/Driver/Database/')) {

We have the Symfony PHP 8 polyfill so this works fine on PHP 7.3 and 7.4...

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Discussed in Slack with @alexpott. Root cause appears to be upgrading Coder (at different times on different branches) without a full run of PHPCS. The change to commit-code-check.sh that means we do a full PHPCS run every time, which only takes 10 seconds there now it is done in parallel, should prevent this from happening again in the future.

  • catch committed 619c625 on 10.0.x 
    Issue #3324540 by alexpott, longwave: PHPCS fails on Drupal 10...
  • catch committed f8fad5f on 10.1.x
    Issue #3324540 by alexpott, longwave: PHPCS fails on Drupal 10 & 9

  • catch committed 909046f on 9.5.x 
    Issue #3324540 by alexpott, longwave: PHPCS fails on Drupal 10 & 9
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Commmitted/pushed to 10.1.x/10.0.x and 9.5.x respectively, thanks!

Status: Fixed » Closed (fixed)

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