AcquiaDrupalStrict is not as strict as the core Drupal sniff

[swichers]

There are explicit overrides made by the AcquiaDrupalStrict sniff in the form of:

  <!-- Relax rules for automated tests -->
  <rule ref="Drupal.Arrays.Array.LongLineDeclaration">
    <exclude-pattern>tests/*</exclude-pattern>
  </rule>
  <rule ref="Drupal.Commenting.ClassComment.Missing">
    <exclude-pattern>tests/*</exclude-pattern>
  </rule>
  <rule ref="Drupal.Commenting.DocComment.MissingShort">
    <exclude-pattern>tests/*</exclude-pattern>
  </rule>
  <rule ref="Drupal.Commenting.FunctionComment.Missing">
    <exclude-pattern>tests/*</exclude-pattern>
  </rule>

This can wind up with your module passing a sniff local to the module folder, but failing to pass by a sniff run elsewhere.

My preference here is that a sniff labeled as strict would be at least as strict as the core sniff. I wanted to create this issue to call out this behavior and get clarity around the intent for the Acquia coding standard sniffs.

If the intent is to use AcquiaDrupalStrict in lieu of the Drupal and DrupalPractice sniffs, then I believe these overrides should be pulled out. Currently if I want to ensure I am core compatible I cannot rely on AcquiaDrupalStrict alone.

If the intent is that we use AcquiaDrupalStrict in tandem with Drupal (and DrupalPractice) then I would recommend that be added to the documentation. I do not believe this is the case since AcquiaDrupalStrict includes Drupal and DrupalPractice.

If the intent is that the Acquia sniffs are not necessarily core compatible, but instead are geared towards Acquia expectations then I would suggest this be specifically documented in the README. Currently the name and description imply a more anal version of the core sniffs which is not the case.

[TravisCarden]

Thanks for the question, @swichers. I want to clarify one point before I answer: when you speak of "core" here, I think you're always referring to the Drupal and DrupalPractice rulesets from the Drupal Coder module. I want to make sure you're not referring to Drupal core when you say, for example, "if I want to ensure I am core compatible", because the standards don't come from Drupal core and Drupal core doesn't obey them entirely. (See core/phpcs.xml.dist).)

That being clarified, the difference is deliberate. This project is intended to document and enforce Acquia's opinionated technical doctrine pertaining to coding standards. As such, it will modify and extend community standards on certain points, and it may serve as a proving ground for contributions to be pushed upstream much as Lightning does for Drupal. I will try to clarify this in the README. As to the rules themselves, you are welcome to argue for any changes on their own merits in the issue queue. Thanks!

[swichers]

You are correct that when I am referencing "core" sniffs I am referring to the Drupal and DrupalPractice sniffs. I mistakenly thought these were part of basic core install from composer. Since they are not part of Drupal core it makes this issue not as important as I originally thought.

I'm still of the opinion that our standards should be as or more strict than what the community is using, but it's harder to make that case when we're not bucking what core provides OOTB.

Just wanted to provide clarification here. No further action is necessary.