4.0 | Formally stop support for sniffs not complying with the PHPCS naming conventions

[jrfnl]

While PHPCS currently has partial/limited support for sniff file includes of sniffs which do not comply with the PHPCS naming conventions (as outlined in the Coding Standard Tutorial), AFAICS this is more by accident than by design and I propose to formally drop support for this in PHPCS 4.0.

What does "formally drop support" mean ?

• A mention of this in the release notes/upgrade guide(s).
• Revert PR Common::getSniffCode(): be more lenient about sniffs not following naming conventions #676 (remove leniency about sniffs not following naming conventions from the Common::getSniffCode() method)
• If/when code is discovered which explicitly supports unconventional setups, remove that code (and any related tests; or possibly: change the tests to confirm this is not supported).
  Note: this task does not have to be completed for the 4.0 release and code covered by this task can be removed at any point in time after the 4.0 release, as long as the 4.0 task 1 (explicit mention in release notes and upgrade guide) has been executed.
• No longer accept any bug reports/feature requests related to sniffs which do not comply with the PHPCS naming conventions and close those immediately and without hesistation.
• Other than that, I think it would be good to review the text of the Coding Standard Tutorial and to clarify the expected naming conventions where needed.

Note: including sniffs by file name will still be supported, but the sniffs included MUST be in a directory structure and with a namespace and class name which comply with the PHPCS naming conventions.

Related #675, #680, #683

Opinions ?

/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg

[greg0ire]

Does PHPCS have a deprecation mechanism that could be leveraged to make people aware of this in 3.x, and to migrate ahead of time? Maybe that could reduce the number of reports filed.

[dingo-d]

A deprecation notice would be great and would definitely urge the external standards maintainers to fix this ahead of time.

[jrfnl]

Does PHPCS have a deprecation mechanism that could be leveraged to make people aware of this in 3.x, and to migrate ahead of time? Maybe that could reduce the number of reports filed.

Good point. PHPCS doesn't have a deprecation mechanism for this, but I think this can probably be detected when loading the ruleset, which would mean we could throw a deprecation notice without impacting CS runs.

I'm going to look into this.

[fredden]

Yes to the plan, but I'd also like to see a code change which detects sniffs which do not confirm and refuses to load these into a ruleset, with a user-visible notice ideally.

[jrfnl]

Just FYI: I am working on adding an "error handling" mechanism to the Ruleset to allow for throwing such deprecation notices (and the warning @fredden mentions once we get to 4.0).

Principles I'm working from:

• Classify "errors" as "blocking" (like no sniffs being registered) or "non-blocking" (like an incorrect <type> being passed).
• Don't hide one error behind another (if it can be avoided).

In practice this will come down to all errors being collected while the ruleset(s) are being processed and then at the end of the processing:

• First display non-blocking issues (deprecations, notices, warnings).
• Next display blocking errors.
• If there are no blocking errors: continue the scan run.

Non-blocking errors will not affect the exit code and not be displayed in -q (quiet) mode.
Blocking errors will result in exit code 3 (as before).

The above will also be applied to the pre-existing errors being thrown from the Ruleset class.

The "problem" I'm running into is that the Ruleset class has relatively low code coverage and a lot of the pre-existing errors are not covered by tests.

In other words, I'm currently writing a sh*tload of tests to improve the code coverage of the Ruleset class. I expect to pull a lot of these over the next week or two.

After that I can introduce the new error handling in v 3.12.0 and possibly combine it straight away with a deprecation notice for the type of setups being discussed in this issue.

How does that sound ?

[greg0ire]

Sounds like a great improvement!