Current Situation
Sniffs should (of course) implement the PHP_CodeSniffer\Sniffs\Sniff interface, either directly or via inheritance, and I expect all sniffs do.
However, there is no defensive coding in place at this time to safeguard this.
The consequences of this are as follows:
- Sniffs which do not implement the
Sniff interface, but do have a register() and process() method which comply with the interface will run without problems.
- Sniffs which do not implement the
Sniff interface, but do have a register() and process() method, but the method signatures do not comply with the interface are likely to throw PHP errors.
- Sniffs which do not implement the
Sniff interface, and are missing the register() method will break a scan run with a Fatal Error during the loading of the ruleset.
- Sniffs which do not implement the
Sniff interface, and are missing the process() method will break a scan run with a Fatal Error, but only if the token(s) the sniff is listening for are encountered, meaning the broken sniff may go unnoticed if the token() are some of the rarely used ones.
Proposal
In practice, removing support will mean that sniffs which do not implement the Sniff interface will no longer be included in the run and that a warning will be presented to the end-user letting them know the sniff is being ignored.
Terminology used
Soft deprecation: deprecation via changelog mention and/or announcement only.
Hard deprecation: a deprecation notice will be shown at runtime, but will not affect the exit code of PHPCS.
Impact
The impact of this change is expected to be low to non-existent for properly coded external standards.
For those edge-case external standards where this change may have an impact, the change will improve the end-user experience and may help the developers discover these type of problems sooner.
Related issues
Loosely related to #689
Opinions ?
/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg
Current Situation
Sniffs should (of course) implement the
PHP_CodeSniffer\Sniffs\Sniffinterface, either directly or via inheritance, and I expect all sniffs do.However, there is no defensive coding in place at this time to safeguard this.
The consequences of this are as follows:
Sniffinterface, but do have aregister()andprocess()method which comply with the interface will run without problems.Sniffinterface, but do have aregister()andprocess()method, but the method signatures do not comply with the interface are likely to throw PHP errors.Sniffinterface, and are missing theregister()method will break a scan run with a Fatal Error during the loading of the ruleset.Sniffinterface, and are missing theprocess()method will break a scan run with a Fatal Error, but only if the token(s) the sniff is listening for are encountered, meaning the broken sniff may go unnoticed if the token() are some of the rarely used ones.Proposal
Sniffinterface, but which do work properly, in the next 3.x minor. - done via Changelog for the 3.12.0 release #885This would take the form of showing a warning to the end-user and ignoring the sniff to prevent scans from breaking and improve the end-user experience.
Sniffinterface in what is expected to be the last 3.x minor. - PR Ruleset: hard deprecate support for sniffs not implementing theSniffinterface #891Sniffinterface in 4.0.0. - PR Ruleset: remove support for sniffs nor implementingSniffinterface #986In practice, removing support will mean that sniffs which do not implement the
Sniffinterface will no longer be included in the run and that a warning will be presented to the end-user letting them know the sniff is being ignored.Terminology used
Soft deprecation: deprecation via changelog mention and/or announcement only.
Hard deprecation: a deprecation notice will be shown at runtime, but will not affect the exit code of PHPCS.
Impact
The impact of this change is expected to be low to non-existent for properly coded external standards.
For those edge-case external standards where this change may have an impact, the change will improve the end-user experience and may help the developers discover these type of problems sooner.
Related issues
Loosely related to #689
Opinions ?
/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg