As part of the initiative to fix coding standards in Drupal core, a problem was discovered in the Drupal.Array.Array sniff. Ref. #2572613: Fix 'Drupal.Array.Array.CommaLastItem' coding standard.

It fails on the new array syntax, all lines get de-indented

       '#cache' => [
-        'contexts' => $this->entityType->getListCacheContexts(),
+      'contexts' => $this->entityType->getListCacheContexts(),
       ],
CommentFileSizeAuthor
#12 i2572587-12.patch13.01 KBattiks
#9 i2572587-9.patch13.17 KBattiks
#8 i2572587-8.patch13.29 KBattiks
#7 i2572587-7.patch11.18 KBattiks
#6 i2572587-6.patch9.13 KBattiks
#4 interdiff.txt2.06 KBpfrenssen
#4 2572587-4.patch10.54 KBpfrenssen
#3 i2572587-4.patch8.49 KBattiks

Comments

attiks created an issue. See original summary.

attiks’s picture

attiks commented

It is even worse, the following does not get detected

  $form['region-' . $region]['title'] = [
    'data-dialog-type' => 'modal',
      'data-dialog-type' => 'modal',
  ];
attiks’s picture

attiks commented
Status: Active » Needs review
StatusFileSize
new i2572587-4.patch8.49 KB

First patch

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Priority: Critical » Normal
StatusFileSize
new 2572587-4.patch10.54 KB
new interdiff.txt2.06 KB

Added a test. This only specifically testing for warnings on the missing final commas, but since the phpcbf fixing of whitespace is also part of the test I guess this is sufficient to cover the whitespace bug.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Issue summary: View changes
Related issues: +#2572613: Fix 'Drupal.Array.Array.CommaLastItem' coding standard
attiks’s picture

attiks commented
StatusFileSize
new i2572587-6.patch9.13 KB

Add support for nested short arrays

attiks’s picture

attiks commented
StatusFileSize
new i2572587-7.patch11.18 KB

Ignore #6

attiks’s picture

attiks commented
StatusFileSize
new i2572587-8.patch13.29 KB

Added more tests

attiks’s picture

attiks commented
StatusFileSize
new i2572587-9.patch13.17 KB

Fixed the unit tests

pfrenssen’s picture

pfrenssen
Location Sofia
commented

Great work, nice test coverage was added, and it's all green!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Makes sense, please use the PHPCS coding standard for our sniff code (4 space indentation etc) to be in line with PHPCS code and to be consistent in one file.

attiks’s picture

attiks commented
Status: Needs work » Needs review
StatusFileSize
new i2572587-12.patch13.01 KB

Patch fixing - hopefully - the coding standards

  • klausi committed a4dd621 on authored by attiks 
    Issue #2572587 by attiks, pfrenssen, klausi: Extend array sniff to short...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Title: phpcbf - Drupal.Array.Array » Extend array sniff to short array syntax
Status: Needs review » Fixed

Did a bit of cleanup by running "phpcs --standard=PHPCS" on the file. Committed, thanks!

  • klausi committed 53350dd on 8.x-2.x 
    Issue #2572587: Add test coverage for allowed short array syntax
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Follow-up: #2580017: Short array syntax false positives

Status: Fixed » Closed (fixed)

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