Disable WPCS alignment checks

[GaryJones]

One of the Neutron sniffs is DisallowAssignAlign.

Neutron claims that it should be applied in addition to the WPCS, but its code style is to ensure things like array value assignments and assignments on consecutive lines are aligned.

Here's an example. My config file (a giant array) follows the WPCS, so all of the => are suitably aligned. Neutron doesn't like that. If I change the first one to remove the alignment (only one space before the =>, then WPCS doesn't like it).

This is a direct conflict, and so Neutron (since I have it active in my ruleset) should actively disable (severity 0) the conflicting checks from WPCS (e.g. WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned would be one for arrays, but there would be others), within the Neutron standard ruleset.xml file.

I could do this in my own project ruleset of course, but I'd rather see Neutron handle this conflict directly for everyone's benefit.

---

If Neutron is meant to be independent of WPCS (and therefore can't guarantee that WPCS standard would also be installed), then that's a different matter, since referencing WordPress.... in an exclude from the Neutron ruleset when it's not available will likely cause PHPCS to throw an error. In which case, removing the claim of working on top of WPCS, and adding some copy-paste snippet to a wiki page (and a reference to the page in the README) would be the way to go.

[sirbrillig]

This is a good point. This project has kind of grown organically since its inception and now maybe it's a good time to step back and figure out what it should actually represent.

Since it's how we're using it, it might be best for this to actually reference WPCS as a dependency and override the rules which we are overriding in our config file now. The reason we haven't done that so far is because while some of the overrides are due to conflicts (eg: assignment alignment and yoda conditions), others are simple preference (eg: docblocks and file names) and I didn't want to presume that everyone who might benefit from these rules would want our preferences. Also because some of these sniffs could be helpful to others on their own without WPCS at all.

Maybe it would be best to split this into two rulesets: one meant to append and override WPCS and the other just with general sniffs (like how I split off import detection).

[GaryJones]

As I was reading:

I didn't want to presume that everyone who might benefit from these rules would want our preferences

I thought of:

Maybe it would be best to split this into two rulesets

Well, I was thinking of two packages, but two rulesets would be enough. Specifically, I was thinking of how PHPCompatibility has a base package, and then PHPCompatibilityWP uses that with a few extra tweaks. There could be a NeutronStandard, and a NeutronStandardWPCS (but named better) which uses the former but with a few extra amendments on the assumption it's being used with WPCS.

For BC, it may need to be NeutronStandardBase and NeutronStandard (again, with better names), so that those who are already using Neutron with WPCS get the amendments without having to update their project rulesets.

[GaryJones]

Another situation where a violation might be reported twice, is the use of extract(). WPCS already has a sniff that handles it, yet Neutron adds its own.

One of these should be disabled when using the ruleset that assumes WPCS is present.

(If you want to re-title this ticket to cover the wider issue than just assignment alignments, that's fine.)

[sirbrillig]

Well, I was thinking of two packages, but two rulesets would be enough

Thanks, I meant two packages. Or two standards, to be specific.

This is also complicated by the way in which phpcs conflates collections of rules (sniffs), configurations of rules, and dependencies on other standards. I didn't originally want to include any configs in this standard which referenced other standards for the reasons mentioned above.

In order to keep our config separate, I created phpcs-neutron-ruleset to handle just the dependencies and the configuration. So arguably someone who wants to use this ruleset with WPCS should use that package and not this one. OTOH, that package has a lot of custom configuration for our codebase, so it might not be general enough (or stable enough) for general use.

All of this is to say that I'm not sure how best to structure this going forward. Suggestions are welcome!