Fix Rules coding standards messages

For the blank lines following an inline comment I totally agree that the code is clearer as-is. Our phpcs.xml.dist file currently has

<rule ref="DrupalPractice">
  <!-- Allow empty lines after comments, we don't care. -->
  <exclude name="DrupalPractice.Commenting.CommentEmptyLine"/>
</rule>

which has been there for a while, so I think Fago also agreed that we are fine with blank lines after inline comments. However, there is now (maybe not originally) also a "Drupal" as opposed to "DrupalPractice" sniff Drupal.Commenting.InlineComment.SpacingAfter which appears to do the same thing, and it is this one which is now reporting the fault. So I'm going to add that also to the excluded list. No point in having one and not the other.

For the rules.drush.inc long lines, I've just split the strings in the places required. It will be easy when we come to implement this to revert back to the un-split strings.

Well, that was surprising! I expected the blank lines and the drush.inc long lines to be fixed, so was anticipating having just the 'Missing member variable doc comment' in tests/src/Unit/TestMessenger.php. But that fault has disappeared too, and there are zero code standard problems.

I checked the Jenkins dispatcher log and yes, when phpcs.xml.dist is modified the entire project is sniffed.

PHPCS config file modified, sniffing entire project.
Executing PHPCS.
cd modules/contrib/rules && 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/modules/contrib/rules

So it was not just the changed files that are tested, as per normal patches when phpcs.xml.dist is untouched.

I do get the TestMessenger.php error locally, using exactly the same Rules code and phpcs.xml.dist as in this patch. However, it does not show up in my Travis test. So there must be something odd causing these differences. As you say, the code looks fine and it has not been changed during the period when d.o. started reporting this fault. It will be interesting to see the phpcs result when this patch is committed. The error may be back again.

Just had a lighbulb moment! I think it is related to the PHP/Core/PHP_CodeSniffer versions being run. The Rules test on 8th Sep at Core 8.7 using PHP7.2 and PHP_CodeSniffer 3.4.1 did not report this fault. There were only 4 errors
https://www.drupal.org/pift-ci-job/1399928

However, the test on 13th Sept at core 8.8 with PHP7.3, and PHP_CodeSniffer 3.4.2, reports the six messages
https://www.drupal.org/pift-ci-job/1404054

D.O patch testing uses PHP 7.2 and PHP_CodeSniffer 3.4.1 currently so that is why the message did not show. This would also explain why I get 4 messages not 6 in my Travis test. I am using PHP7.1 locally with PHP_CodeSniffer 3.4.2 and I do get the message, so it would suggest that it is the newer version of PHP_CodeSniffer which has caused the difference.

To see if my reasoning is correct I have requeued patch #4 using PHP7.3 and Core 8.8, so it should use PHP_CodeSniffer 3.4.2 and we'll get the one 'Missing member variable doc comment' for TestMessenger.php.

This is actually a fault we can fix, because line 12 is not the @inheritdoc for the test, but it is the $messages property, and it has no doc desciption. That check must have been introduced in the latest PHP_CodeSniffer.

Yes, the test of patch #4 at core 8.8/PHP7.3 has

Installing squizlabs/php_codesniffer (3.4.2)

in the log and produces the one error 'Missing member variable doc comment' in TestMessenger.php.

Here's an updated patch to correct this.

I like that solution even better.
Marking as RTBC