New Feature: handle deprecation of sniffs natively

[jrfnl]

Is your feature request related to a problem?

There are times when a sniff gets superseded by another sniff or just isn't relevant anymore and needs to be deprecated.

In PHPCS itself, this was most recently done in PHPCS 3.3.0 for the Squiz.WhiteSpace.LanguageConstructSpacing sniff and in PHPCS 3.4.0 for the Generic.Formatting.NoSpaceAfterCast sniff.

It can also be said that the MySource standard + all CSS/JS sniffs are effectively deprecated as they will be removed in PHPCS 4.0.

However, deprecating sniffs is not something which is unique to PHPCS itself. External standards will also, at times, want to deprecate sniffs.

Current options for deprecating a sniff

At this moment, when deprecating a sniff, there are effectively two options:

1. Soft deprecation, i.e. deprecation in name only via a mention in the changelog.
   This will largely go unnoticed and end-users will be confronted with the removal in the next major.
2. Hard deprecation, i.e. deprecation by throwing a PHPCS warning from the sniff when the sniff is used.
   This potentially breaks builds when CI is set to fail on warnings, which makes the impact of a deprecation too large for it to still be called a deprecation.

All in all, neither option is very satisfactory.

Describe the solution you'd like

Now to solve this conundrum, I'm thinking of introducing a new DeprecatedSniff interface which extends the existing Sniff interface and adds one additional method getDeprecationMessage().

interface DeprecatedSniff extends Sniff {
    /**
     * Retrieve an arbitrary deprecation notice.
     * @return string
     */
    public functon getDeprecationMessage();
}

When a sniff gets deprecated, the implements Sniff would then be swopped out for an implements DeprecatedSniff and the getDeprecationMessage() method would need to be added.

final class MySniff implements DeprecatedSniff {
    public function register() { ... }

    public function process() { ... }

    public function getDeprecationMessage() {
        // Example for a sniff which is deprecated with replacement:
        return 'This sniff is deprecated since [VERSION] and will be removed in [VERSION]. Use the [REPLACEMENT] sniff instead.';

        // Example for a sniff which is deprecated without replacement:
        return 'This sniff is deprecated since [VERSION] and will be removed in [VERSION].';
    }
}

In the PHPCS framework, when the ruleset gets loaded, it could then be detected if a sniff is deprecated and all sniff deprecation messages could be collected and displayed above the report output. Similar to messages about the ruleset containing a reference to a non-existent sniff or to a property which doesn't exist.

These deprecation messages would not influence the exit code of a PHPCS run and would therefore be non-breaking.

When the -q (quiet) option is used, these messages would also be silenced.

Opinions ?

Open questions:

• Should the DeprecateSniff interface extend Sniff or should it be a stand-alone interface ?
  A stand-alone interface would possibly allow it to be used more easily in combination with arbitrary abstract sniff classes.
• Should there be just a getDeprecationMessage() method which allows for an arbitrary message or should the message be standardized more ?
  If the message should be more standardized, this could be done by having multiple methods: getDeprecationVersion(), getRemovalVersion(), getReplacement(); or by a getDeprecationInfo() method which would expect an array with this info in three predefined keys.

/cc @kukulich @wimg @GaryJones @dingo-d @fredden @michalbundyra @stronk7

Additional Context

In a future iteration, I can imagine another interface which would allow for PHPCS natively handling the deprecation of one or more public properties in a sniff.

• I intend to create a pull request to implement this feature.

[kukulich]

Should the DeprecateSniff interface extend Sniff or should it be a stand-alone interface?
Should there be just a getDeprecationMessage() method which allows for an arbitrary message or should the message be standardized more ?

I prefer standalone interface with just one method.

[dingo-d]

I think it's also better to have a single interface. When a sniff is deprecated you can just implement multiple interfaces (which to me sounds like a better OOP practice anyhow - composition over inheritance). That way it's not directly coupled to the Sniff interface.

And some standardization in the message output would be beneficial IMO, as that would bring more consistency across the standards. Imagine using 3-4 different standards, each having different depreciation messages, I think that would increase the cognitive load on the end user.

With a standardized depreciation message, users know what to expect, no matter which standard (bundled with PHPCS or not) they are using.

[jrfnl]

Notes to self - additional ideas to make this feature complete:

• Mark deprecated sniffs as such in the sniff list which can be displayed when the -e option is used.
• Display a notice that a sniff is deprecated when displaying the documentation for that sniff (--generator=... options).

[jrfnl]

I tend to agree with @dingo-d about the standardization of the messages, so my preliminary implementation is a stand-alone interface with three methods.

Here's screenshot of what the output could look like (work in progress, feedback welcome):

[jrfnl]

Next iteration - as before, feedback welcome. I expect to open a PR for this change later today.

[jrfnl]

And here's a screenshot of the output for "explain" when a standard contains deprecated sniffs:

[stronk7]

Q: Is there any way to show the deprecation details in the "explain" report?

Other than that, it looks nice to me!

[jrfnl]

@stronk7 That would be possible, but would make it quite noisy IMO. In that case, it would probably be better to show the whole deprecations block for the explain command as well and not change the output of explain itself.

I mean, the details as per the above screenshot would now show on every normal run (except when running with -q (quiet), -e (explain) and --generator=... (docs)). That is, of course, providing there are deprecations to show.

[jrfnl]

PR #281 is now open for this feature. Reviewing and testing would be appreciated.

[jrfnl]

Notes to self - additional ideas to make this feature complete:

• Mark deprecated sniffs as such in the sniff list which can be displayed when the -e option is used.
• Display a notice that a sniff is deprecated when displaying the documentation for that sniff (--generator=... options).

Of the above, the first has been included in PR #281. The second has not as it would involve more extensive changes. This could potentially still be added at a later date.

[jrfnl]

Unless anyone leaves a comment/feedback on the PR, I will merge it tomorrow.