Fix 'Drupal.Commenting.DocComment.TagGroupSpacing' coding standard [part 2]

In #2937513: Fix 'Drupal.Commenting.DocComment.TagGroupSpacing' coding standard there was quite a bit of discussion about Coder sniffs, and some changes were made. There were also some questions about why missing blank lines were not detected.

In #3181485: Regression in @ tag sniff TagGroupSpacing I discovered that the sniff was indeed not detecting all of the correct places where blank lines should inserted. Now that this is fixed, there are 12 additional places in core code which can be corrected.

The Coder fix is committed in dev, and will be available when Core 9.2 uses the new version 8.3.12 (not released yet). However, as this sniff is already enabled in Core, we should fix the 12 faults now, so that when 8.3.12 is used the problems do not suddenly appear.

phpcs --standard=core/phpcs.xml.dist --sniffs=Drupal.Commenting.DocComment core --report=summary

locally gives

PHP CODE SNIFFER REPORT SUMMARY
--------------------------------------------------------------------------------
FILE                                                            ERRORS  WARNINGS
--------------------------------------------------------------------------------
.../drupal92dev/core/lib/Drupal/Component/Gettext/PoHeader.php  1       0
...ib/Drupal/Core/Entity/Query/ConditionAggregateInterface.php  3       0
...re/lib/Drupal/Core/Entity/Query/QueryAggregateInterface.php  1       0
...l92dev/core/lib/Drupal/Core/Entity/Query/QueryInterface.php  1       0
...2dev/core/lib/Drupal/Core/Menu/MenuTreeStorageInterface.php  1       0
...Web/drupal92dev/core/lib/Drupal/Core/Test/TestDiscovery.php  1       0
.../WebServer/Web/drupal92dev/core/modules/views/src/Views.php  1       0
...core/modules/views/src/Plugin/Derivative/ViewsLocalTask.php  1       0
...v/core/tests/Drupal/Tests/Component/DrupalComponentTest.php  1       0
.../tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php  1       0
--------------------------------------------------------------------------------
A TOTAL OF 12 ERRORS AND 0 WARNINGS WERE FOUND IN 10 FILES
--------------------------------------------------------------------------------
PHPCBF CAN FIX 12 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

I will make a patch to demonstrate the errors using the dev version of Coder

Attempt to run the sniff using the Coder dev package.

Patch #4 only changes drupalci.yml, no change to phpcs.xml.dist

So if the patch alters phpcs.xml(.dist) then we get:

PHPCS config file modified, sniffing entire project.
Executing PHPCS.
cd core && sudo -u www-data /var/www/html/vendor/bin/phpcs --report-full=/var/lib/drupalci/workdir/phpcs/codesniffer_results.txt --report-checkstyle=/var/lib/drupalci/workdir/phpcs/checkstyle.xml --report-diff=/var/lib/drupalci/workdir/phpcs/codesniffer_fixes.patch /var/www/html/core

But if the patch only changes drupalci.yml then we get

No modified files are eligible to be sniffed

Now trying with

     phpcs:
        sniff-all-files: true

So it looks like Coder 8.x-3.x-dev is being loaded by Composer. All files are being sniffed. But we still do not get new failures. Trying with a patch to the Coder folder.

That worked. We now get the expected 12 coding standards warning which I see locally.Here's a patch to fix these 12.

Now the real patch to be committed, with no change to drupalci.yml

Hi @daffie,
Thanks for the comment. However, the sniff Drupal.Commenting.DocComment.TagGroupSpacing is already enabled and active in core as there is no exclude for it - see core/phpcs.xml.dist#L47 and my comments in #2 above - the sniff was incorrectly not detecting these errors, so we need to fix them in core before the new Coder version gets used.

I notice that the sniff name is not mentioned in the phpcs.xml.dist comment listing the sniffs which are checked. I looked back and this sniff was fixed and removed from the exclusion list in https://git.drupalcode.org/project/drupal/commit/4f61b6f but the comment was not changed. That can be ammeded here, and the same goes for the sniffs DocComment.Empty and DocComment.TagsNotGrouped which are already active.

Thanks.

The patch adds the Drupal.Commenting.DocComment.TagGroupSpacing, Drupal.Commenting.DocComment.Empty and Drupal.Commenting.DocComment.TagsNotGrouped rules to PHPCS.

Just to be clear, for anyone else reading this issue, those three sniffs are already active in phpcs.xml.dist, this patch ammends the comment to indicate that they are active, and fixes 12 regressions in TagGroupSpacing which would fail under Coder 8.3.12 (core 9.2 is currently using Coder 8.3.10)

It is all explained in the linked issue #3181485: Regression in @ tag sniff TagGroupSpacing. The fault in Coder (starting with 8.3.7) allowed incorrect lines which should have failed DocComment.TagGroupSpacing to be committed. This patch fixes those 12 regressions. The faults do not show yet because Core 9.2 is still running Coder 8.3.10 and the corrected sniff will be available from Coder 8.3.12. You can see in the results of patch #6 above where I patched Coder to update the sniff, that the test does show the 12 failures.

This patch does not do anything with Drupal.Commenting.DocComment.Empty and Drupal.Commenting.DocComment.TagsNotGrouped apart from list them in the phpcs.xml.dist comment where they should have been listed already.

Hi @longwave
The latest version of Coder is 8.3.11 which was released on 4 November 2020. The regression fix is still only available in Coder -dev (hence my use of the coder patch in #6 above). No harm in updating to 8.3.11 but would be nice to use 8.3.12 in core 9.2. If you have a planned timetable on #3187025: Update dependencies for Drupal 9.2 then Klausi may well release 8.3.12 to conincide with that (if you ask him nicely ;-)

@xjm This issue does not actually change the list of active coding standards sniffs - see my comments in #2 and #12. No core developer/reviewer would have to do anything differently as a result of this commit. Does that change the need for release notes?