Use Slevomat CS

[mmoll]

No description provided.

[wickedOne]

to me it makes no sense to incorporate parts of another coding standard (other than the default sniffs provided by codesniffer) into this one. the whole idea is to provide a library with a custom set of rules based on a described coding standard. by referencing another library into this one you create an unnecessary dependency while someone can can do that customisation in their own phpcs.xml if they want to.

[mmoll]

The advantage IMHO is pulling in widely used, known well maintained 3rd party code (providing also auto-fixes) and thus reducing the code in this repository that needs to get maintained.

For #144 (comment) the advantage would also be that no code at all in this repo would need to get written.

[wickedOne]

by that sentiment it would make more sense to put out a seperate package which consists of a phpcs.xml with aggregated rules from different standards don't you think?

[mmoll]

a seperate package which consists of a phpcs.xml with aggregated rules from different standards

Something like https://github.com/mayflower/mo4-coding-standard? ;)

On a more serious note, I'd say for an end-user the purpose of escapestudios/symfony2-coding-standard is to have a "Symfony" ruleset for phpcs available that checks the code to comply to the Symfony coding standards. How this is done technically (i.e. rulesets coming with phpcs (like PSR2), own code, 3rd party library code...) is IMHO not important for the end user.

[wickedOne]

an added dependency which supports / replaces something which does not concern the core functionallity of a library makes sense. replacing the core functionality with the core functionality of another package doesn't if you'd ask me.

the end user has plenty of control and can choose to use this library, another one or can aggregate a ruleset of their own liking, we don't have to make that decision.

i think dropping sniffs and replacing them with a dependency is not the way to go.

@djoos what's your opinion on this?

[djoos]

Apologies for the late reply here... Trying to catch up here - immediately flagging that I'm in 2 minds ("well, that is not very helpful!") and definitely open for further discussion.

From a coding standard point of view
As @wickedOne mentioned: relying on sniffs provided by codesniffer makes absolute sense. Adding in a dependency to another library that is, in essence a specific company's coding standard, feels like re-entering where this coding standard originated from: I relied on the opensky/Symfony2-coding-standard back at Escape Studios, when all of a sudden they pulled the repo and there was no longer any Symfony coding standard to be found.
Is it just a matter of leaving people to customize their own phpcs.xml and leave it up to them which bits to pull in from which coding standard(s)? Let's suppose all core functionality of the current repo would be handled by Slevomat CS; it would still mean that this repo would have its value, as it would 1) put together and 2) enforce what it delivers is "the Symfony coding standard" and not Company X's standard.
The Doctrine coding standard has adopted Slevomat CS for a while now and are bumping it regularly. (disclaimer: this doesn't mean that we need to blindly follow that direction, as I mentioned: let's discuss this further!)

From a code point of view
In the spirit of not reinventing the wheel, even when putting together a coding standard, relying on another library for Sniffs instead of coding up our own Sniffs sounds sensible. We're obviously in control of what we'd include and ideally rigorously test our desired end goal, which is making sure we don't get pulled to one side or another but deliver on the Symfony coding standards. It might well be that having less code to maintain here would allow us to up the bar on the testing side of things and as a result increase the quality...

Thoughts?

[mmoll]

another library that is, in essence a specific company's coding standard

Since a few years, I personally do see that in the Slevomat case not as true anymore. Almost every sniff is highly configurable for most of them can also get configured to behave as exact opposite of the default. Thus I'm seeing it more as a collection of sniffs that can get included easily in other standards and configured there as needed, see SlevomatCodingStandard.Classes.RequireSingleLineMethodSignature as example in this PR. The codebase is well maintained (100% code coverage) and most sniffs do bring auto-fixing.

Furthermore I also do want to say that PHPCS development itself is sometimes not turning out the way, sniff/standard authors would like to see, which is why https://phpcsutils.com/ exists and may also see more widespread use in the future (it would for sure already have, if Slevomat wouldn't have gained such popularity in the community), so limiting the source of standards, sniffs or sniff helper methods to phpcs itself brings further maintenance load in any case.

ideally rigorously test our desired end goal

That's a weak point currently, for the Sniffs from PHPCS, already included now, but also for the combination of everything in the ruleset. For MO4, we do some light testing, but nothing regarding violations or auto-fixes.

[djoos]

@wickedOne: you complete the top 3 of contributers to this repo, so I'd love to hear from you as well on this.
Thanks in advance for your 2 cents!

[wickedOne]

first of all sincere apologies for my ridiculous late reply on this one;
a lot has been going on / happened last year or so as i guess did for everyone.

i'm a bit torn about this one: i'm with @djoos regarding his point breaking loose from escape studios that basically a library shouldn't have it's core feature rely an a third party library. on the other hand i'm also pro "not re-inventing the wheel" and indeed we should test the hell out of it and revert to our own implementation once things start falling apart if we decide to switch to slevomat's sniffs.

given the later scenario, this repo would be nothing more than a configured phpcs.xml and a shitload of tests and offer no more added value than a config for those not willing or able to doing that themselves.
that in itself would be perfectly acceptable, but should be an all or nothing scenario i think (i.e. be a phpcs.xml config or provide your own sniffs)

my gut feeling is "stick to our own". switching to slevomat's sniffs seems premature (it's not that there are a bunch of use cases in the issue list we can't cover / test in this repository). depending on a third party for this library's core functionality defeats it's purpose and adds an undesired dependency from my point of view