Ensure that we're using the right ruleset for coding standards checking

Plus, what I posted at #2851510-11: Fix phpcs regressions by running phpcbf...

+++ b/core/phpcs.xml.dist
@@ -59,37 +59,37 @@
     <exclude name="Drupal.Semantics.FunctionT.BackslashSingleQuote"/>
     <exclude name="Drupal.Semantics.FunctionT.NotLiteralString"/>
     <exclude name="Drupal.Semantics.FunctionT.ConcatString"/>

Do we need to change the ref to the excluded ones as well?

In general it will be great for the same rules to be used automatically for everyone, but making assumptions about relative filepaths is always a bit risky.

Creative change @alexpott, +1 from me. Reduces code we have to maintain, so that IMO trumps the downside of hard-coding the file location of sniffs like that. People can always copy phpcs.xml.dist to phpcs.xml anyway to do their own thing.

Patch does not apply anymore, sending for retesting.

Urgs, looks like 8.3.x doesn't pass anyway right now, so setting this to "needs work" manually.

I just ran into an issue where vendor/bin/phps could not find the sniffs referenced in core/phpcs/xml.dist unless I set installed_paths manually:

$ vendor/bin/phpcs --config-set installed_paths ~/Sites/drupal8x/vendor/drupal/coder/coder_sniffer

Note, for the record, I have two phpcs:

drupal8x (8.4.x=)$ phpcs --config-show
installed_paths: /Users/cjm/.composer/vendor/escapestudios/symfony2-coding-standard,/Users/cjm/.composer/vendor/drupal/coder/coder_sniffer
drupal8x (8.4.x=)$ vendor/bin/phpcs --config-show
installed_paths: /Users/cjm/Sites/drupal8x/vendor/drupal/coder/coder_sniffer

Some might raise an issue that this makes it more difficult to do coding standards checks against a non standard install where vendor has moved. But this is missing the point, coder and phpcs.xml.dist exist for checking core's coding standards at commit time and can suggest direction for the wider community. They are not there to provide a working system for contrib or custom. They are there to ensure core does not regress against the coding standards it is already complaint with.

That's indeed a bit sad. I'm using the core version in a global alias, as it is less crazy in terms of the amount of rules :)

My actual question is, can we introduce an entry inside the composer.json which does the linting (see https://getcomposer.org/doc/articles/scripts.md#writing-custom-commands)? Similar to the entry in package.json:
"lint:js": "eslint . || exit 0" we could have an entry in there which looks like this:
"phpcs": "./vendor/bin/phpcs --standard=core/phpcs.xml.dist .

I think this patch though is totally the right thing to do.

I worked on #2862565: Get rid of the absolute path generated by Composer::configurePhpcs today. Looks like its not necessary anymore because we remove the script in this issue.

Rerolled and fixed the new ExpectedException sniff.

I'm happy you had to reroll the patch!

I was just about to commit this because it solves some problems that I have with multiple projects, but it looks as though Drupal.Functions.FunctionDeclaration.SpaceAfter went missing in the patch. This was made plain with git --word-diff=color.

Doh! Ok.

Committed 34bf287 and pushed to 8.4.x. As I feel this is a non-disruptive dev tooling bug fix (see #13), I cherry-picked it as 8c970f7 and pushed to 8.3.x. Thanks!

This looks broken from the perspective of drupal/core as a root package, with a phpcs.xml.dist within drupal/core assuming a vendor dir outside of the drupal/core package. A standard drupal/core composer build will have vendor inside the drupal/core project root. There are enough use cases of developing/testing on just drupal/core.

coder and phpcs.xml.dist exist for checking core's coding standards at commit time

Shouldn't we instead move the phpcs.xml.dist into drupal/drupal then and reference ./vendor?

Shouldn't https://www.drupal.org/node/2839574 be published?

@jibran if you notice an unpublished change record you can publish it yourself.

I was merely asking about it because I was not sure. After reading my comment again I realized my tune was not helpful so sorry about that.

@Eric_A If you do run into workflow problems with this change, you can open a new issue so we can discuss further. Thanks!

Note that this patch broke code checking on DrupalCI for contrib modules that were referencing the "Drupal" rule in their phpcs.xml.dist files.

I overcome this by referencing the standards' directory directly:

   <!--Use Drupal code rule.-->
-  <rule ref="Drupal"></rule>
+  <rule ref="../../../vendor/drupal/coder/coder_sniffer/Drupal"></rule>

Actually AFAICS, if a contrib module does NOT have a phpcs.xml.dist file, PHPCS fails on DrupalCI with the following error, and no PHP code checking is performed (CSS an JS are OK).

ERROR: the "Drupal" coding standard is not installed. The installed coding standards are Zend, Squiz, PSR2, PSR1, PHPCS, PEAR and MySource

see https://dispatcher.drupalci.org/job/drupal_contrib/32270/console as an example.

Opened #2870645: PHPCS is failing on contrib modules in DrupalCI: Drupal.org Testing Infrastructure issue queue.

#2902381: Revert phpcs.xml.dist changes from "Ensure that we're using the right ruleset for coding standards checking"