Fix Drupal.Array.Array.ArrayIndentation coding standard

Hi @eiriksm, cs violations don't fail the tests. So you can add only one patch. Note that the second patch is empty.

Now I see what you mean. In fact it's a good idea.

Part of patch will not be needed after fix of related #2911280: RectangleTest.php takes a very long time to scan for coding standards

This needs work to update core/phpcs.xml.dist with sniffer & exclude other sub-sniffs

This needs a reroll but looks solid, we should probably also prepare a patch for 8.4 without the phpcs changes.

Re-rolled.

Friendly post #2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking" reroll.
No feedback addressed. No credit needed.

Drive by review:
1) when testing the reroll there where quite a few array indentation failures for me. Doesn't seem related to the reroll though.
2) it seems the standard that exclusion lists like is in this patch include a comment with the tests being run but this patch doesn't include that.

hi,
i gave this a new try
hope it'll be green

This looks great and it has the required changes to the phpcs file.

Already needs a reroll... :(

This is likely to be a tricky on to get in.

Also +1 to get this done it will will fix lots of common errors and help patch review of big arrays way easier. One less thing to visually check.

rerolled #28

1. Thanks everyone for working on this patch. Patches like this are tricky but core adhering to its coding standards is an important thing especially once we get to a place where all the rules are applied. Unfortunately this patch is making some changes to apply the rule but continue to break our coding standards. I'm not sure we should do that. See the partial review below.

2. +++ b/core/lib/Drupal/Core/Installer/Exception/AlreadyInstalledException.php
   @@ -24,9 +24,9 @@ public function __construct(TranslationInterface $string_translation) {
   -      ':base-url' => $GLOBALS['base_url'],
   -      ':update-url' => $GLOBALS['base_path'] . 'update.php',
   -    ]);
   +  ':base-url' => $GLOBALS['base_url'],
   +  ':update-url' => $GLOBALS['base_path'] . 'update.php',
   +]);
   

   This change doesn't really look correct. In fact the whole way the translatable text contains HTML is super interesting. I think we should open a separate blocking issue to address this. We don't usually translate exception messages so I think we should go back to the original issue and see discussions there.

3. +++ b/core/modules/comment/src/CommentStorage.php
   @@ -183,12 +183,12 @@ public function getNewCommentPageNumber($total_comments, $new_comments, Fieldabl
   -        ':status' => CommentInterface::PUBLISHED,
   -        ':entity_id' => $entity->id(),
   -        ':field_name' => $field_name,
   -        ':entity_type' => $entity->getEntityTypeId(),
   -        ':thread' => $first_thread,
   -      ])->fetchField();
   +                          ':status' => CommentInterface::PUBLISHED,
   +                          ':entity_id' => $entity->id(),
   +                          ':field_name' => $field_name,
   +                          ':entity_type' => $entity->getEntityTypeId(),
   +                          ':thread' => $first_thread,
   +                        ])->fetchField();
   

   This looks odd.

4. +++ b/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
   @@ -282,7 +282,7 @@ public function testSiteWideContact() {
   -      ]
   +    ]
        );
   

   Let's bring the ); up to the ]

5. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationOutdatedRevisionTranslationTest.php
   @@ -35,10 +35,10 @@ public function testFlagAsOutdatedHidden() {
   +      $entity->getEntityTypeId() => $id,
   +      'source' => 'en',
   +      'target' => 'it',
   +    ],
          [
            'language' => ConfigurableLanguage::load('it'),
            'absolute' => FALSE,
   
   @@ -54,10 +54,10 @@ public function testFlagAsOutdatedHidden() {
   +      $entity->getEntityTypeId() => $id,
   +      'source' => 'en',
   +      'target' => 'fr',
   +    ],
          [
            'language' => ConfigurableLanguage::load('fr'),
            'absolute' => FALSE,
   
   +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationRevisionTranslationDeletionTest.php
   @@ -57,10 +57,10 @@ public function doTestOverview($index) {
   -        $entity->getEntityTypeId() => $id,
   -        'source' => 'en',
   -        'target' => 'it',
   -      ],
   +      $entity->getEntityTypeId() => $id,
   +      'source' => 'en',
   +      'target' => 'it',
   +    ],
          [
            'language' => ConfigurableLanguage::load('it'),
            'absolute' => FALSE,
   
   @@ -182,9 +182,9 @@ public function doTestOverview($index) {
   -        'node' => $id,
   -        'node_revision' => $it_revision->getRevisionId(),
   -      ],
   +      'node' => $id,
   +      'node_revision' => $it_revision->getRevisionId(),
   +    ],
          [
            'language' => ConfigurableLanguage::load('it'),
            'absolute' => FALSE,
   

   This looks odd.

6. +++ b/core/modules/filter/tests/src/Kernel/FilterKernelTest.php
   @@ -335,12 +335,12 @@ public function testLineBreakFilter() {
   -        "<script>aaa\nbbb\n\nccc</script>" => TRUE,
   -        "<style>aaa\nbbb\n\nccc</style>" => TRUE,
   -        "<pre>aaa\nbbb\n\nccc</pre>" => TRUE,
   -        "<object>aaa\nbbb\n\nccc</object>" => TRUE,
   -        "<iframe>aaa\nbbb\n\nccc</iframe>" => TRUE,
   -      ],
   +  "<script>aaa\nbbb\n\nccc</script>" => TRUE,
   +  "<style>aaa\nbbb\n\nccc</style>" => TRUE,
   +  "<pre>aaa\nbbb\n\nccc</pre>" => TRUE,
   +  "<object>aaa\nbbb\n\nccc</object>" => TRUE,
   +  "<iframe>aaa\nbbb\n\nccc</iframe>" => TRUE,
   +],
   

   I think this needs a different fix. Maybe by creating the array key outside of the array.

7. +++ b/core/modules/filter/tests/src/Kernel/FilterKernelTest.php
   @@ -566,19 +566,19 @@ public function testUrlFilter() {
          '
    http://example.com or www.example.com
    ' => [
   -        '<a href="http://example.com">http://example.com</a>' => TRUE,
   -        '<a href="http://www.example.com">www.example.com</a>' => TRUE,
   -      ],
   +  '<a href="http://example.com">http://example.com</a>' => TRUE,
   +  '<a href="http://www.example.com">www.example.com</a>' => TRUE,
   +],
   ...
    person@example.com or mailto:person2@example.com or ' . $email_with_plus_sign . ' or ' . $long_email . ' but not ' . $too_long_email . '
    ' => [
   -        '<a href="mailto:person@example.com">person@example.com</a>' => TRUE,
   -        '<a href="mailto:person2@example.com">mailto:person2@example.com</a>' => TRUE,
   -        '<a href="mailto:' . $long_email . '">' . $long_email . '</a>' => TRUE,
   -        '<a href="mailto:' . $too_long_email . '">' . $too_long_email . '</a>' => FALSE,
   -        '<a href="mailto:' . $email_with_plus_sign . '">' . $email_with_plus_sign . '</a>' => TRUE,
   -      ],
   +  '<a href="mailto:person@example.com">person@example.com</a>' => TRUE,
   +  '<a href="mailto:person2@example.com">mailto:person2@example.com</a>' => TRUE,
   +  '<a href="mailto:' . $long_email . '">' . $long_email . '</a>' => TRUE,
   +  '<a href="mailto:' . $too_long_email . '">' . $too_long_email . '</a>' => FALSE,
   +  '<a href="mailto:' . $email_with_plus_sign . '">' . $email_with_plus_sign . '</a>' => TRUE,
   +],
   

   I would fix the array key here to be

   "\nhttp://example.com or www.example.com\n" => [
           '<a href="http://example.com">http://example.com</a>' => TRUE,
           '<a href="http://www.example.com">www.example.com</a>' => TRUE,
         ],
   

   Same for the MAILTO URLs test...

This is a duplicate of #2937515: Fix Drupal.Array.Array.[ArrayClosingIndentation, ArrayIndentation] coding standard

Current config in 9.5.x:

  <rule ref="Drupal.Arrays.Array">
    <!-- Sniff for these errors: ArrayClosingIndentation, ArrayIndentation, CommaLastItem -->
    <exclude name="Drupal.Arrays.Array.LongLineDeclaration"/>
  </rule>

@Mile23, thanks. I don't know how I missed that this is a duplicate.

I have moved credit to other issue.