Update the stable version of drupal/coder to ˆ8.3.1

phpcs doesn't run properly with #8 as some of the sniffs have changed names.

$ ../vendor/bin/phpcs
ERROR: Referenced sniff "Drupal.Array.Array" does not exist

Run "phpcs --help" for usage information

The attached patch renames these sniffs in phpcs.xml.dist and it seems to work after this, though running the full set against core reports a lot of "Opening statement of multi-line function call not indented correctly; expected X spaces but found Y", not sure if this is expected.

This patch ignores the "PEAR.Functions.FunctionCallSignature.OpeningIndent" sniff which is new in PHP CodeSniffer 3.1.2, but is similar to PEAR.Functions.FunctionCallSignature.Indent in that we do not comply with it yet.

It also fixes a number of other new coding standards issues identified by the updated sniffs included in 3.1.2.

With this patch you should get clean output from a PHPCS run across all of core.

Looks good to me. Pretty minor changes to match the sniff adjustments so should be okay despite being slightly broader scope than ideal I think

+1 to being on the latest version of coder.

As a dependency upgrade that does more than just bug fix upgrades this will only be able to be part of 8.7.x - we need a reroll to apply to that branch.

One concern I had was that phpcs might have a higher minimum PHP dependency than Drupal 8s (current PHP 5.5) however looking at their composer.json file it shows that they are targeting PHP 5.4 so that's great.

I think we should have a change record because projects do track Drupal's dev dependencies. See https://github.com/webflo/drupal-core-require-dev

+++ b/core/composer.json
@@ -54,7 +54,7 @@
-        "drupal/coder": "^8.2.12",
+        "drupal/coder": "^8.3.1",

One thought is are we truly incompatible with coder 8.2? I don't think so if a project is stuck on 8.2 for whatever reason we could be kind and change the dependency to ^8.2.12|^8.3.1

I don't think we can be immediately compatible with both because of the renamed sniffs mentioned in #10, I assume the older phpcs will throw the opposite error with the updated phpcs.xml.dist file. It is possible there is some config or other changes we can do to work around this I guess.

There's no re-roll necessary - the patch doesn't apply after composer run drupal-phpunit-upgrade

@longwave you're right - we can't be compatible with both because Drupal.Array.Array => Drupal.Arrays.Array - that's debatable whether that should be in a minor version change - but semver is hard for contrib anyhow.

Still I think it is fine for us to upgrade our dev dependencies like this in a minor version but tagging for needs release manager review for a second opinion.

I think a change record that documents the renames would be great.

Draft Change Record

Oops I created a duplicate change record at https://www.drupal.org/node/3003759 but the other one is probably better.

The CR looks good. I removed the extra one. We need an rtbc and a release manager +1.

Reroll after #2787871: Properly deprecate getUserName() and use getAccountName() instead. Setting it to RTBC as it is just a reroll.

I ran composer run phpcs -- ./core -ps locally after running composer install. All looks good.

I tested running this patch on PHP5.5 for what it is worth and it runs just fine.

Would slightly prefer the code-style fixes and the update to be in separate issues, although there's only a handful of changes so it's not too bad.

This doesn't apply to 8.7.x any more though.

3 way merge FTW.

Needs a re-roll. Fine with the small code-style clean-ups happening here.

Conflicts because of #2809503: Convert AJAX part of \Drupal\file\Tests\FileFieldWidgetTest to JavascriptTestBase.

#30 needs an update. It fails for the following reasons:

FILE: /Users/alex/dev/sites/drupal8alt.dev/core/tests/Drupal/Tests/Component/Utility/CryptRandomFallbackTest.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 19 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Database/Database.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
 43 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
 50 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
 57 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
 64 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
 78 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Render/Element/HtmlTag.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 41 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/lib/Drupal/Component/Utility/Timer.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 12 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/serialization/tests/serialization_test/src/SerializationTestEncoder.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 14 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/serialization/tests/serialization_test/src/SerializationTestNormalizer.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 14 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/serialization/src/Encoder/XmlEncoder.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 30 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/Timezone/TimezoneTest.php
---------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------
 57 | ERROR | [x] Expected 1 space after USE keyword; found 0
    |       |     (Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterUse)
---------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Kernel/PathHooksTest.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 18 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/alex/dev/sites/drupal8alt.dev/core/modules/system/tests/src/Functional/Theme/ThemeTokenTest.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 19 | ERROR | [x] The static declaration must come after the visibility declaration
    |       |     (PSR2.Classes.PropertyDeclaration.StaticBeforeVisibility)
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------

Re-rolled and fixed all the things reported by PHPCS using PHPCBF - this seems to be happening because #30 doesn't fully update PHPCS to latest version - which I think we should do here.

Since this is an automated tool setting back to rtbc.

Nice one my phpcs -p --standard=core/phpcs.xml.dist core was still running when I posted the patch :D

Committed and pushed c67761090d to 8.8.x and b5a52c0814 to 8.7.x. Thanks!

This seems to have broken some contrib project's support for coder in drupalci:

See https://dispatcher.drupalci.org/job/drupal8_contrib_patches/53298/console

This is a disruptive change -- it also broke my local toolchain and prevented me from committing patches cleanly. :)

I resolved the issue with:

composer global remove drupal/coder
composer global remove phpcs
composer install

Do we have another issue for #37?

Also, any dependency update should be mentioned in the release notes. We should include as much information as possible about the problems sites might encounter and how to fix them.

Do we have another issue for #37? As per #testing channel in slack @alexpott and @mixologic was looking into it. We have a purposed fix which can be deployed later today.

@catch added the release notes to IS and it looks good to me so marking it fix.

Following up heddn's comments in #37 this probably affects all contrib testing on drupalci. I have just checked Scheduler (which I maintain) and we get the same problem, see attached. This means that coding standards are not being checked on patches. As an example, I created a new patch in our old issue #2861902-26: Fix coding standards violations in 8.x codebase to purposely add some coding standards faults. The test output is clean with no violations, however, it should show:

FILE: scheduler/src/Form/SchedulerAdminForm.php
--------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 100 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 1
 100 | ERROR | [x] Array indentation error, expected 6 spaces but found 1
 100 | ERROR | [x] Inline comments must start with a capital letter
 100 | ERROR | [x] Inline comments must end in full-stops, exclamation marks,
     |       |     colons, question marks, or closing parentheses
 100 | ERROR | [x] Comments may not appear after statements
--------------------------------------------------------------------------------

So currently, patches could be committed to contrib which contain coding standards violations and this would be unseen by the tester and maintainer.

@jonathan1055 - @mixologic is aware and will fix asap.

A fix has been pushed to drupalci and now contrib modules should no longer get the conflict on installation.
I re-ran the test in #42 as a proof and it now properly gets the errors:
https://www.drupal.org/pift-ci-job/1242574

Thanks very much @alexpott and @mixologic. Yes coding standards checking on contrib is now being run again.

Published the change record after minor updates.