Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard

Comparing #24 with #8 seems to be missing a big chunk of the fixes.

Some of them may have naturally been fixed but the few files I checked they have not.

Thanks @smustgrave this is the updated patch

CI failure #28

Updated the patch

Nice to see progress here!

Fixing PHP coding standards is done by first adding a Coder rule, or sniff. The current patch is missing that so it needs to be added.We do this so that new code does not introduce regressions. When a patch is tested various checks are run, such as coding standard checks, before the tests run.

So, what needs to happen here is to start by just adding the sniff. Then run phpcs to find what needs to be fixed. Then fix the comments and run phpcs core until there are no errors. The latest patch will have some of those fixes but it also contains out of scope changes. This patch is to fix array syntax in comments, not code.

I have updated the proposed resolution in the Issue Summary with some details.

I update phpcs.xml and remove unwanted changes.
I was not able to apply patch 32, so I made some changes to it, I added manually created interdiff.
Please review.

Oops, made mistake when creating a patch file.
Provide new patch file and interdiff.

Update interdiff #32 and #37.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

I created MR as requested by Needs Review Queue Bot.

Hide patch files.

Failure is random and unrelated, ran it a few times and sometimes it passes sometimes fails but is not impacted by this
#3317520: Random failure in Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testEditModeEnableDisable →

Large MR but not terrible to read.

Think if anything was missed the phpcs line added would of got it.

As the person who committed the 12-gigabyte array() to [] conversion, I feel it's my very great honor to review this one as well.

Something odd is going on with the docblock of EntityFormDisplayInterface::buildForm(); there are more array closures than openings. It appears to be pseudocode rather than example code, and at least one array closure was missed. That's an existing problem and out of scope for a color-words issue like this, so tagging for a followup.

I reviewed this locally with git diff --color-words to verify that all the changes were complete and correct replacements of array() with [] in API documentation.

There was another existing issue in core/modules/views/src/Plugin/views/cache/CachePluginBase.php with a missing closing paren that's fixed here. I opened the file locally to confirm that it's fixed correctly.

I also grepped the codebase to check for remaining array(). There are many in inline comments, but that seems to be outside the scope of this issue, which is all docblocks. So, I think we need a second followup for inline comments.

However, there are somehow also numerous instances in docblocks that are apparently not being caught by the rule?

[ayrton:maintainer | Thu 18:37:29] $ grep -r "\barray(" * | grep -F \* | grep -v "vendor" | grep -v "node_modules"
core/scripts/dump-database-d7.sh:  $result = db_query('SELECT * FROM {'. $table .'}', array(), array('fetch' => PDO::FETCH_ASSOC));
core/lib/Drupal/Core/Database/Database.php:   * array(
core/lib/Drupal/Core/Database/Log.php:   * array(
core/lib/Drupal/Core/Database/Log.php:   *   $logging_key = array(
core/lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
core/lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
core/lib/Drupal/Core/Database/Query/Select.php:   * array(
core/lib/Drupal/Core/Cache/UseCacheBackendTrait.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
core/lib/Drupal/Core/Datetime/DrupalDateTime.php: * DrupalDateTime::createFromArray( array('year' => 2010, 'month' => 9, 'day' => 28) )
core/lib/Drupal/Core/Entity/EntityAutocompleteMatcherInterface.php:   *   autocomplete API (e.g. array('value' => $value, 'label' => $label)).
core/lib/Drupal/Core/Render/Element.php:   *   property; e.g., array('#property_name' => 'attribute_name'). If both
core/lib/Drupal/Core/ImageToolkit/ImageToolkitInterface.php:   *   operation, e.g. array('width' => 50, 'height' => 100,
core/lib/Drupal/Core/Field/FieldConfigInterface.php:   *   - NULL or array() for no default value.
core/lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
core/lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
core/lib/Drupal/Core/Field/BaseFieldDefinition.php:   *   each item being a property/value array (array() for no default value).
core/lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
core/lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
core/lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
core/lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
core/lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
core/modules/views_ui/admin.inc: *   array('dynamic_content', 'section') for this parameter.
core/modules/system/tests/modules/common_test/common_test.module: * \Drupal::moduleHandler()->alter(array(TYPE1, TYPE2), ...) allows
core/modules/page_cache/src/StackMiddleware/PageCache.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   normally be found in $form_state->getValue(array('wrapper', 'select')),
core/modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   you would pass array('wrapper', 'select') for this parameter.
core/modules/views/src/Plugin/views/field/Markup.php: *           array('field' => {$field}) where $field is the field in this table
core/modules/rest/src/Plugin/ResourceInterface.php:   *   The list of supported methods. Example: array('GET', 'POST', 'PATCH').

Setting NR to look into those. Several of them appear to be in sentences rather than @code/@endcode -- maybe that is the common factor? If that's confirmed, we could have a third followup to properly use @code where possible and another to fix them in sentences.

Thanks everyone!

Then how come the rule didn’t catch those?

@smustgrave, yes that is the question that xjm is asking those working on this issue to answer.

To help move this along I did the following. I started by searching the vendor directory for DocCommentLongArraySyntax and found the class implementing the sniff.

/**
 * Ensures @code annotations in doc blocks don't contain long array syntax.
 *
 * @category PHP
 * @package  PHP_CodeSniffer
 * @link     http://pear.php.net/package/PHP_CodeSniffer
 */
class DocCommentLongArraySyntaxSniff implements Sniff
{

The summary line confirms what xjm suspected in #47. And as I read it then the remainder of the work in #47 can proceed.

I have added #46 and #47 to the remaining tasks.

For the follow ups of 46 + 47

Rebased the MR. Then, I searched for the same pattern used in #47 and then checked the files and I am confident these are not detected because they are not in @code @endcode blocks. I have updated the issue summary with the followups that need to be made.

So applied the MR and ran grep -r "\barray(" * | grep -F \* | grep -v "vendor" | grep -v "node_modules"

lib/Drupal/Core/Database/Database.php:   * array(
lib/Drupal/Core/Database/Log.php:   * array(
lib/Drupal/Core/Database/Log.php:   *   $logging_key = array(
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Log.php:   *     array('query' => '', 'args' => array(), 'caller' => '', 'target' => '', 'time' => 0, 'start' => 0),
lib/Drupal/Core/Database/Query/Select.php:   * array(
lib/Drupal/Core/Cache/UseCacheBackendTrait.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
lib/Drupal/Core/Datetime/DrupalDateTime.php: * DrupalDateTime::createFromArray( array('year' => 2010, 'month' => 9, 'day' => 28) )
lib/Drupal/Core/Entity/EntityAutocompleteMatcherInterface.php:   *   autocomplete API (e.g. array('value' => $value, 'label' => $label)).
lib/Drupal/Core/Render/Element.php:   *   property; e.g., array('#property_name' => 'attribute_name'). If both
lib/Drupal/Core/ImageToolkit/ImageToolkitInterface.php:   *   operation, e.g. array('width' => 50, 'height' => 100,
lib/Drupal/Core/Field/FieldConfigInterface.php:   *   - NULL or array() for no default value.
lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Field/FieldDefinitionInterface.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Field/BaseFieldDefinition.php:   *   each item being a property/value array (array() for no default value).
lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
lib/Drupal/Core/Utility/ProjectInfo.php:   *   file. Defaults to array().
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
lib/Drupal/Component/Gettext/PoHeader.php:   *   array(
modules/views_ui/admin.inc: *   array('dynamic_content', 'section') for this parameter.
modules/system/tests/modules/common_test/common_test.module: * \Drupal::moduleHandler()->alter(array(TYPE1, TYPE2), ...) allows
modules/page_cache/src/StackMiddleware/PageCache.php:   *   tags. For example array('node' => array(123), 'user' => array(92)).
modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   normally be found in $form_state->getValue(array('wrapper', 'select')),
modules/views/src/Plugin/views/wizard/WizardPluginBase.php:   *   you would pass array('wrapper', 'select') for this parameter.
modules/views/src/Plugin/views/field/Markup.php: *           array('field' => {$field}) where $field is the field in this table
modules/rest/src/Plugin/ResourceInterface.php:   *   The list of supported methods. Example: array('GET', 'POST', 'PATCH').
scripts/dump-database-d7.sh:  $result = db_query('SELECT * FROM {'. $table .'}', array(), array('fetch' => PDO::FETCH_ASSOC));

Which spot checking appear to be outside of @code blocks.

For #45 follow up is that covered by 📌 Convert code blocks in comments using array() syntax to @code/@endcode Needs review or 📌 Convert use of array() syntax in sentences Needs review ?

There is a hunk in core/modules/views/src/Plugin/views/field/FieldPluginBase.php that has out-of-scope changes. It could either be a bad merge, or someone correcting the documentation while fixing this bug.

If it's the latter, we should make a followup issue to correct the keys; regardless, the hunk should only include the changes to array().

Going to do a local review of this with git diff --color-words; if someone wants to fix the hunk and file a followup if appropriate, we could get this in still as a beta phase(-ish) cleanup.

Saving issue credits.

Followups were already created.

Tagging "Novice" for the task described in #53.

📌 Update use of arguments used in comments Active opened, already tagged for title update cuz that one is bad.

Oops did that before I got the novice email.

No worries, I prefer to just land the issue in this case. :D

Committed to 11.x and 11.0.x.

It cherry-picked cleanly to 10.4.x with some auto-merging, but I'm going to open a quick 10.4.x MR here based on that cherry-pick just to ensure there aren't additional instances in D10 that would cause the rule to fail there.

xjm → closed merge request !5441

Good thing I tested that backport before pushing it; looks like the 10.4.x backport will need some additions. :D

--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 258 | ERROR | Long array syntax must not be used in doc comment code
     |       | annotations
     |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
--------------------------------------------------------------------------------
FILE: ...ds/project/drupal/core/modules/tour/tests/src/Functional/TourTestBasic.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 19 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 20 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 21 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 22 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
--------------------------------------------------------------------------------
FILE: ...lds/project/drupal/core/modules/tour/tests/src/Functional/TourTestBase.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 23 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 24 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 25 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
 26 | ERROR | Long array syntax must not be used in doc comment code
    |       | annotations
    |       | (Drupal.Commenting.DocCommentLongArraySyntax.DocLongArray)
--------------------------------------------------------------------------------
Time: 10.49 secs; Memory: 8MB
PHP CODE SNIFFER REPORT SUMMARY
--------------------------------------------------------------------------------
FILE                                                            ERRORS  WARNINGS
--------------------------------------------------------------------------------
/builds/project/drupal/core/includes/install.inc                1       0
...pal/core/modules/tour/tests/src/Functional/TourTestBase.php  4       0
...al/core/modules/tour/tests/src/Functional/TourTestBasic.php  4       0
--------------------------------------------------------------------------------
A TOTAL OF 9 ERRORS AND 0 WARNINGS WERE FOUND IN 3 FILES
--------------------------------------------------------------------------------

Fixed up 10.4 backport, passing phpcs now.

10.4 rebase seems fine.

Thanks @b_sharpe, nice work!

Since 10.3.x is in RC, it is not eligible to have new rules enabled. So, instead of testing a 10.3.x backport, I backported the 10.4.x changes only for parity, with the following commands:

[ayrton:maintainer | Mon 15:24:12] $ git cherry-pick -x 10.4.x
Auto-merging core/modules/responsive_image/responsive_image.module
[10.3.x e64d867c62] Issue #2874067 by kkalashnikov, Nikolay Shapovalov, quietone, b_sharpe, smustgrave, xjm, benjifisher, MerryHamster, dww: Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard
 Date: Mon Jun 17 15:22:41 2024 -0500
 107 files changed, 579 insertions(+), 578 deletions(-)
[ayrton:maintainer | Mon 15:24:20] $ git checkout HEAD^ --  core/phpcs.xml.dist  
[ayrton:maintainer | Mon 15:27:29] $ git commit --amend -m 'Issue #2874067 by kkalashnikov, Nikolay Shapovalov, quietone, b_sharpe, smustgrave, xjm, benjifisher, MerryHamster, dww: Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard'

Thanks everyone!

xjm → closed merge request !8433

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