Part of meta-issue #2571965: [meta] Fix PHP coding standards in core, stage 1 and a follow-up for #3183673: Fix 'Drupal.Commenting.DocComment.ShortFullStop' coding standard - part 1 auto-fixes
That issue dealt with the large proportion of automatic fixes for this sniff. This issue will fix the problems that need manual editing.

Step 1: Preparation

Open the file core/phpcs.xml.dist and add a line for the sniff of this ticket. The sniff name is in the issue title. Make sure your patch will include the addition of this line.

Step 2: Install & configure PHPCS

Install PHP CodeSniffer and the ruleset from the Coder module:

$ composer install
$ ./vendor/bin/phpcs --config-set installed_paths ../../drupal/coder/coder_sniffer

Once you have installed the phpcs package, you can list all the sniffs available to you like this:

$ ./vendor/bin/phpcs --standard=Drupal -e

This will give you a big list of sniffs, and the Drupal-based ones should be present.

Step 3: Prepare the phpcs.xml file

To speed up the testing you should make a copy of the file phpcs.xml.dist (in the core/ folder) and save it as phpcs.xml. This is the configuration file for PHP CodeSniffer.

We only want this phpcs.xml file to specify the sniff we're interested in. So we need to remove all the rule items, and add only our own sniff's rule. Rule items look like this:

<rule ref="Drupal.Commenting.DocComment.ShortFullStop"/>

Remove all of them, and add only the sniff from this issue title. This will make sure that our tests run quickly, and are not going to contain any output from unrelated sniffs.

Step 4: Run the test

Now you are ready to run the test! From within the core/ folder, run the following command to launch the test:

$ cd core/
$ ../vendor/bin/phpcs -p

This takes a couple of minutes. The -p flag shows the progress, so you have a bunch of nice dots to look at while it is running.

Step 5: Fix the failures

When the test is complete it will present you a list of all the files that contain violations of your sniff, and the line numbers where the violations occur. You could fix all of these manually, but thankfully phpcbf can fix many of them. You can call phpcbf like this:

$ ../vendor/bin/phpcbf

This will fix the errors in place. You can then make a diff of the changes using git. You can also re-run the test with phpcs and determine if that fixed all of them.

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

In #3183673: Fix 'Drupal.Commenting.DocComment.ShortFullStop' coding standard - part 1 auto-fixes I used a modified version of the Coder ShortFullStop sniff to avoid making auto-fixes where the change would likely be incorrect. See Coder issue #3184314: Improve the auto-fixing for DocComment.ShortFullStop coding standard for details.

The above work left 40 problem lines which would need a manual fix. To demonstate the incorrect auto-fixes for these, patch #2 will run phpcs and use Coder as-is (version 8.3.10 in core 9.2) and will generate patch with the proposed (incorrect) fixes.

jonathan1055’s picture

StatusFileSize
new2.67 KB

As expected, we have 40 coding standards warnings and the generated codesniffer_fixes.patch shows that the attempted fixes are incorrect, or at best need manual checking.

Patch #3 is similar to the aboev except that it applies a patch to Coder to include the modifications I have worked on in #3184314: Improve the auto-fixing for DocComment.ShortFullStop coding standard. This should still have 40 warnings but they will not be auto-fixed.

jonathan1055’s picture

StatusFileSize
new2.66 KB

I put the host_command outside the validate_codebase:, it should within it. Patch #4 should be better.

jonathan1055’s picture

StatusFileSize
new25.73 KB

As expected we have the 40 faults, but there are no suggested auto-fixes.

Patch #5 has manual corrections for all of these coding standards warnings. Many of the changes are just fixing the incorrect original ending punctuation or changing {inheritdoc} (which fails the standard) to {@inheritdoc} which is an allowed exception and should not have the full stop. Other corrections require a blank lines where the short comment was not separated form the long comment (and the long comment had no full stop). Other fixes required splitting a long line and where that has been done I have tried to keep the new text as close to the original text as possible. Some comments were only half written and so I have made it into a proper useful comment.

There are probably a few places where these corrections clash with possible fixes in #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard but the last patch on that issue was six months ago at 9.1 so would need a re-roll now anyway.

This patch still has the drupalci.yml change to avoid running the phpunit tests. That should remain in until any re-writes are done following review, then we can run one final test with phpunit included.

jonathan1055’s picture

StatusFileSize
new24.17 KB
new2.84 KB

Patch #5 still applies OK.
Patch #6 has the same 40 fixes, but with no change to drupalci.yml, so this will run the full test suite.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

The patch adds the Drupal.Commenting.DocComment.ShortFullStop rule to PHPCS.
All violations are fixed.
All code changes look good to me.
For me it is RTBC.

jonathan1055’s picture

Assigned: jonathan1055 » Unassigned

That's great. Thanks for the review @daffie

  • catch committed 203664a on 9.2.x
    Issue #3185652 by jonathan1055, daffie: Fix 'Drupal.Commenting....
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 203664a and pushed to 9.2.x. Thanks!

xjm’s picture

Issue tags: +9.2.0 release notes

Coding standards changes always get summarized in the release notes.

Status: Fixed » Closed (fixed)

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