Update Coder to 8.2.10 and fix errors from improved sniffs

Are you sure we don't have issues for those new sniffs already? I'm just wondering, because I would have expected that those more or less quite fundamental bits have been fixed already.

1. +++ b/core/modules/system/src/Tests/Update/UpdatePathTestBase.php
   @@ -268,7 +268,7 @@ protected function runUpdates() {
            foreach (\Drupal::entityDefinitionUpdateManager()
   -                   ->getChangeSummary() as $entity_type_id => $summary) {
   +          ->getChangeSummary() as $entity_type_id => $summary) {
   

   wow, I would have not expected that coder can handle that complex case well, cool!

2. +++ b/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlConnectionTest.php
   @@ -29,7 +29,7 @@ protected function setUp() {
       * Data provider for testEscapeTable.
       *
   -   * @return []
   +   * @return array
       *   An indexed array of where each value is an array of arguments to pass to
       *   testEscapeField. The first value is the expected value, and the second
       *   value is the value to test.
   @@ -51,7 +51,7 @@ public function providerEscapeTables() {
   
   @@ -51,7 +51,7 @@ public function providerEscapeTables() {
      /**
       * Data provider for testEscapeAlias.
       *
   -   * @return []
   +   * @return array
       *   Array of arrays with the following elements:
       *   - Expected escaped string.
       *   - String to escape.
   @@ -68,7 +68,7 @@ public function providerEscapeAlias() {
   
   @@ -68,7 +68,7 @@ public function providerEscapeAlias() {
      /**
       * Data provider for testEscapeField.
       *
   -   * @return []
   +   * @return array
       *   Array of arrays with the following elements:
   

   Should we go with array[] for all those 3 cases instead?

Thank you @pfrenssen!

One thing I'm wondering though is whether we should add coder to the require-dev bit first and then update the version there.

@pfrenssen
Well, we can keep using 8.2.7 on our global composer.json file, and be fine with that, can't we?

We fixed all the 'regressions' for coder 8.2.8 in #2747073: Fix Drupal CS regressions for Coder 8.2.8, so #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core will be OK if we stick to 8.2.8 there.

This issue could update the dependency to 8.2.9 and be patched against core 8.3.x.

#2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core is a bit derailed now and probably needs more time, so I think we can continue here with just the fixes. Can you reroll the patch without composer changes?

Forgot one part, in Coder 8.2.9 the test files are no longer part of the distribution build so they don't need to be excluded any more.

Shouldn't we still leave them in in cased people install with --prefer-source?

Full patch of automated fixes and manual fixes from above.

As I understand this is blocking #2794567: Make core comply with new standard spacing for anonymous functions and enable the rule in phpcs.xml.dist and blocked by #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core, is that correct?

Sort of. The idea here was that since #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core locked us into then-recent Coder 8.2.8, we'd pick up the latest fixes here afterwards.

#2794567: Make core comply with new standard spacing for anonymous functions and enable the rule in phpcs.xml.dist requires a version of Coder >=8.2.9, which this issue doesn't supply any more, though it could.

Rerolled and updated to bring us up to fixed for 8.4.x and Coder 8.2.9.

The only real problem is this one:

--- a/core/modules/big_pipe/src/Render/BigPipe.php
+++ b/core/modules/big_pipe/src/Render/BigPipe.php
@@ -640,7 +640,7 @@ protected function filterEmbeddedResponse(Request $fake_request, Response $embed
    *
    * @param \Symfony\Component\HttpFoundation\Request $request
    *   The request for which a response is being sent.
-   * @param \Symfony\Component\HttpKernel\HttpKernelInterface::MASTER_REQUEST|\Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST $request_type
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterfaceMASTER_REQUEST|\Symfony\Component\HttpKernel\HttpKernelInterfaceSUB_REQUEST $request_type
    *   The request type.
    * @param \Symfony\Component\HttpFoundation\Response $response
    *   The response to filter.

Drupal Coder 8.x-2.10 has been out on 14th December . Do we need a new followup issue for fixing 8..x-2.10?

Rerolled + fixed BigPipe method as suggested.

Manually confirmed that phpcs output is clean and does not show errors after this patch.

Just tested Coder 8.2.10 and it has 0 new errors with this patch, so we can update straight to that.

Fixing title, we are not fixing new sniffs here but existing sniffs that got better :-)

1. +++ b/core/modules/big_pipe/src/Render/BigPipe.php
   @@ -640,8 +640,10 @@ protected function filterEmbeddedResponse(Request $fake_request, Response $embed
   -   * @param \Symfony\Component\HttpKernel\HttpKernelInterface::MASTER_REQUEST|\Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST $request_type
   -   *   The request type.
   +   * @param int $request_type
   +   *   The request type. Can either be
   +   *   \Symfony\Component\HttpKernel\HttpKernelInterface::MASTER_REQUEST or
   +   *   \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST.
   

   Checked that these constants are int, and they are: https://github.com/symfony/http-kernel/blob/master/HttpKernelInterface.p...

2. +++ b/core/phpcs.xml.dist
   @@ -57,6 +57,8 @@
        <exclude name="Drupal.Commenting.FunctionComment.ParamNameNoMatch"/>
   +    <exclude name="Drupal.Commenting.FunctionComment.ParamTypeSpaces"/>
   +    <exclude name="Drupal.Commenting.FunctionComment.ReturnTypeSpaces"/>
        <exclude name="Drupal.Commenting.FunctionComment.TypeHintMissing"/>
   

   These are excluded as per #2, with follow-ups: #2803891: Fix 'Drupal.Commenting.FunctionComment.ReturnTypeSpaces' coding standard #2803893: Fix 'Drupal.Commenting.FunctionComment.ParamTypeSpaces' coding standard

Applies to both 8.3.x and 8.4.x, and phpcs passes for both.

Rawk.

#34,
Awesome klausi to update the coder to 8.2.10
Patch looks good. However, it's still showing the warnings https://dispatcher.drupalci.org/job/drupal_patches/451/checkstyleResult/...

//Naveen

@naveenvalecha, those are *fixed* warnings. They're warnings fixed by the patch.

drupalci fail, queuing again.

I'm sorry. I think I broke your patch by accidentally running 8.2.10 locally and overfixing a different commit.

That said, maybe we can fix the standards in separate issues first? Since 8.2.10 is just a patch-level update, it can go into a patch release or RC.

Rerolled.

+++ b/core/phpcs.xml.dist
@@ -57,6 +57,8 @@
+    <exclude name="Drupal.Commenting.FunctionComment.ParamTypeSpaces"/>
+    <exclude name="Drupal.Commenting.FunctionComment.ReturnTypeSpaces"/>

When I originally read the patch I thought why are we adding new rules - but of course we're not we're excluding new rules. So all the changes are due to fixes to existing rules that are applying. Hopefully. And yes if phpcbf can apply the fixes then it is probably okay just to run that.

Committed and pushed fea11b6 to 8.4.x and 71fa660 to 8.3.x. Thanks!

I've tested this locally on both 8.3.x and 8.4.x everything passes PHPCS nice.

Next Coder update: #2857714: Upgrade Coder to 8.2.11