Reference the Drupal coding standard instead of forking it

Rather than modify the Drupal coding standard, we can reference it, as per option 3 in this comment: https://github.com/civicrm/coder/issues/13#issuecomment-784728961.

This MR is a WIP attempt to do that. I think it more or less follows the approach outlined in that issue.

Referencing the Drupal ruleset

See https://lab.civicrm.org/michaelmcandrew/civicrm-coding-standards/-/blob/1-reference-the-drupal-coding-standard-instead-of-forking-it/CiviCRM/ruleset.xml.

If people like this idea in principle then it would just be a case of transferring the modifications from coder/coder_sniffer/Drupal/ruleset.xml to CiviCRM/ruleset.xml in this repo.

A couple of initial thoughts and comments on that.

In the forked branch, it looks like most of the rules have been modified to set severity to 0 rather than excluding them. I wasn't sure what the difference in practice was. The exclude syntax is slightly less verbose but if there is a good reason to use the 'rule/severity=0' approach, then we can continue with that.

Also, there some merge requests that touch the Sniffs directory. I think we should / will need to look at an alternative approach to them. It could be that we just turn off the rule. We might want to consider replacing with another more appropriate rule. It might be that we no longer need to patch if we upgrade to upstream.

https://github.com/civicrm/coder/pull/9/files https://github.com/civicrm/coder/pull/11/files https://github.com/civicrm/coder/pull/14/files https://github.com/civicrm/coder/pull/16/files

One final thing. The MR switches to ~8.2.12. It might be that there is a more specific / appropriate constraint to use initially.

Running this alongside Drupal rules

@sluc23's issue was 'Rename ruleset as "CiviCRM" to be able to have default "Drupal" standard installed as well'. This MR allows that to some extent but it is constrained to the 8.2 branch (which was last updated 3 years ago). So we would only be able to use out of date Drupal rules. If you want to be able to use the latest Drupal standards alongside these, then we would need to change that constraint to be something like ~8.3. This means joining the Drupal coding standards band wagon. I'm not sure how much/rapidly those standards are evolving changing, and it is probably not a bad band wagon to be on since they have more resources than we do to do things like keep their sniffs compatible with the latest version of PHP, etc. and it is probably better to have to deal with the issues as they appear rather than save them up until they become unmanageable.

The other thing to say is that this is only a problem if you want to use a single phpcs to validate both Drupal and CiviCRM code with the same version of phpcs (e.g. you global one). If you are happy to use two versions of phpcs (one installed with the latest Drupal and the other installed with CiviCRM on the 8.2.x branch, then you are fine. That said, I think we should probably stick closer to the Drupal version in any case for the reasons mentioned above.

@totten said

eg choosing the "Drupal" style in PHPStorm will give you warnings on a "CiviCRM" style codebase

FWIW, you can add .phpcs.xml files to different directories in your codebase to specify what standards should be applied to that directory and subdirectories. You can also add a ./path/to/directory to a ruleset to specify that it only be applied to a specific subdirectory, so I think something like the below should work (haven't tried it). I tried it - it didn't work.

<ruleset>
  <description>Use Drupal for Drupal files</description>
  <rule ref="Drupal" />
</ruleset>
<ruleset>
  <description>Use CiviCRM for CiviCRM files</description>
  <rule ref="CiviCRM" />
  <file>./sites/all/modules/civicrm</file>
</ruleset>

So in summary, the approach would be

1. transfer modifications from drupal/coder/coder_sniffer/Drupal/ruleset.xml to civicrm-coding-standards/CiviCRM/ruleset.xml, and deal with the MRs above.
2. remove the version constraint on the drupal/coder requirement

Let me know what you think about this MR and the approach in general (like I mentioned above, I think it is more or less in line with the approach already outlined but I might be overlooking something).