Fix 'Drupal.ControlStructures.ControlSignature' coding standard

-    if (empty($comments))
-      return '';
+    if (empty($comments)) {
+      return ''; }

While adding braces is good for readability, the closing braces should be on a line on their own.

The empty cases might also warrant adding a comment as to why the block is empty.

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.

Rerolled patch. After phpcbf changes.

vendor libs should be excluded.

Vendor libs are excluded from patch fix-2572693-5.patch :)

1. +++ b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
   @@ -97,7 +97,8 @@ public function build($class_name, $proxy_class_name = '') {
   -            if (isset($interfaces[$parent_interface])) {}
   +            if (isset($interfaces[$parent_interface])) {
   +}
   

   not sure about that, but indent is wrong

2. +++ b/core/lib/Drupal/Core/Archiver/ArchiveTar.php
   @@ -181,45 +182,54 @@ public function __construct($p_tarname, $p_compress = null)
   -                    } elseif ($data == "BZ") {
   +                    }
   +elseif ($data == "BZ") {
   ...
   -                    } elseif (file_get_contents($p_tarname, false, null, 1, 4) == '7zXZ') {
   +                    }
   +elseif (file_get_contents($p_tarname, false, null, 1, 4) == '7zXZ') {
   ...
   -            } else {
   +            }
   +else {
   ...
   -                } elseif ((substr($p_tarname, -3) == 'bz2') ||
   +                }
   +elseif ((substr($p_tarname, -3) == 'bz2') ||
   

   indent wrong, in that file

ArchiveTar.php should be excluded I think. It's a slightly mod version.

Fix indentation after phpcbf auto correction.
To fix indentation I used Code > Reformat code tool in PhpStorm with with "Only VCS changed text".
It's really powerful option :)

@droplet Can you add more argument and details about reason to exclude ArchiveTar.php file please.

Now patch is great.
ArchiveTar.php is a part of core so should be fixed as well, another round on it should convert indent to 2spaces

Fixing ArchiveTar.php would bring some headache for next release port. There're more things in ArchiveTar.php can be improve but we skipped on each patching.

I'm not sure about ArchiveTar.php changes either.

+++ b/core/lib/Drupal/Core/Datetime/Element/Datelist.php
index 4c7fea9..a09f6dd 100644
--- a/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php

--- a/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php
+++ b/core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php

From the top of the file...

 * NOTE: 98% of this code is a literal copy of Symfony's YamlFileLoader.
 *
 * This file does NOT follow Drupal coding standards, so as to simplify future
 * synchronizations.

See not good reason given here to break that.

* NOTE: 98% of this code is a literal copy of Symfony's YamlFileLoader.

ArchiveTar.php is 99.9%.

We could just pull in https://github.com/pear/Archive_Tar via Composer?

Actually, #2610984: Add Archive Tar via Composer, with BC shim is already dealing with that.

All this fixes are blocked by #2610984: Add Archive Tar via Composer, with BC shim

This rule needs work cos indent is wrong, so manually fixed patch

      3 Drupal.ControlStructures.ControlSignature.NewlineAfterOpenBrace
      3 Drupal.ControlStructures.ControlSignature.SpaceAfterCloseBrace
     11 Drupal.ControlStructures.ControlSignature.NewlineAfterCloseBrace
     19 Drupal.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis
     44 Drupal.ControlStructures.ControlSignature.SpaceAfterKeyword

Patch in #23 gives me this:

$ git apply 2572693-23-fixed.patch 
2572693-23-fixed.patch:266: trailing whitespace.
                  if (count($parameters) === 3) {  
2572693-23-fixed.patch:429: trailing whitespace.
                if ($file->getOwnerId() != \Drupal::currentUser()->id()) {  
2572693-23-fixed.patch:521: trailing whitespace.
      foreach (array('form', 'view') as $type) {  
2572693-23-fixed.patch:534: trailing whitespace.
    elseif (trim($values['title'])) {  
2572693-23-fixed.patch:622: trailing whitespace.
    if ($path instanceof Url) {  
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.

...which I suppose is a bit backwards.

But the patch also yields no errors when phpcs is run.

Patch no longer applies, so I just re-did the work with phpcbf and some manual whitespace edits.

1. +++ b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
   @@ -92,7 +92,8 @@ public function build($class_name, $proxy_class_name = '') {
   -            if (isset($interfaces[$parent_interface])) {}
   +            if (isset($interfaces[$parent_interface])) {
   +}
                unset($interfaces[$parent_interface]);
   

   This if should be removed.

2. +++ b/core/modules/file/src/Element/ManagedFile.php
   @@ -97,7 +97,7 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
   -                if ($file->getOwnerId() != \Drupal::currentUser()->id())  {
   +                if ($file->getOwnerId() != \Drupal::currentUser()->id()) {  ¶
   

   The fixer added some space on the end of the line :(

3. +++ b/core/tests/Drupal/KernelTests/Core/Database/InsertDefaultsTest.php
   @@ -34,7 +34,8 @@ function testDefaultEmptyInsert() {
   +    }
   +catch (NoFieldsException $e) {
   
   +++ b/core/tests/Drupal/KernelTests/Core/Database/LargeQueryTest.php
   @@ -31,7 +31,8 @@ function testMaxAllowedPacketQueryTruncating() {
   +        }
   +catch (DatabaseException $e) {
   
   +++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
   @@ -230,7 +230,8 @@ function testSchema() {
   +    catch (\Exception $e) {
   +}
   

   Needs indenting

Fixed the issues in #27, and went through the diff *again* to double-check...

Looks great for me now!

Committed a6efc8c and pushed to 8.1.x and 8.2.x. Thanks!