Add sniffs for deprecated WP features

[westonruter]

Instead of finding out about a deprecated usage at runtime, we can use static analysis via PHPCS to list deprecated features. We should add sniffs for:

• Deprecated functions (calls to _deprecated_function)
• Deprecated constructors (calls to _deprecated_constructor)
• Deprecated files (calls to _deprecated_file)
• Deprecated hooks (calls to _deprecated_hook)

See also https://github.com/gedex/wpdc

[JDGrimes]

The only concern that I have is maintainability. If we can't automatically generate the list of deprecated features, then it won't end up getting maintained.

Also, we'd need to discuss what version of WordPress we'd be building the list against. I guess it makes sense to use the latest version, even though some folks using the sniffs may support older versions and need to use the deprecated functions for back-compat. Something to consider is whether to use the latest stable version or trunk, though.

[fklein-lu]

This would be very useful for #578 as well.

I consider that one issue with the approach that the WordPress Deprecated Checker uses is that not all functions that are deprecated in Core have been moved to the corresponding file. Additionally deprecated classes won't probably ever be moved, they stay in their own file, and get marked deprecated via Doc block.

On that note, the PHP Doc Parser used by the WordPress.org developer documentation site has an understanding of what code elements are deprecated, so one could possibly use this to make this code sniff maintainable.

[JDGrimes]

On that note, the PHP Doc Parser used by the WordPress.org developer documentation site has an understanding of what code elements are deprecated, so one could possibly use this to make this code sniff maintainable.

I've hacked on the PHP Doc Parser before, so maybe I'll take a look at that again and see what is possible. I think that the parser portion will probably provide us the information that we need, and then some. The main issue will probably be sifting through all the info that is returned and then getting it into a format that we can use in our sniffs.

As far as that goes, I wonder if we should let the list be stored in a file separate from the sniff itself, so that folks can easily switch to a different list (maybe for a different version of WordPress).

[jrfnl]

I wonder if we should let the list be stored in a file separate from the sniff itself, so that folks can easily switch to a different list (maybe for a different version of WordPress).

That sounds like a great idea!

[grappler]

I think related to this we should remove the deprected functions from DiscouragedFunctionsSniff

[jrfnl]

Also - a good starting point would be the list currently in Theme Check: https://github.com/Otto42/theme-check/blob/master/checks/deprecated.php

[jrfnl]

And another one to have a look at: https://github.com/Otto42/theme-check/blob/master/checks/more_deprecated.php

[grappler]

We have a PR for deprecated functions in the WPTRT fork. The cool feature is that it is possible to define how many WP versions that should be supported. WPTT/WPThemeReview#77

Currently we are using WordPress_AbstractFunctionRestrictionsSniff to check for function restrictions. Would it better to extend this to support the feature of the number of supported WP versions or should there be a separate sniff for deprecated functions?

[jrfnl]

We have a PR for deprecated functions in the WPTRT fork. The cool feature is that it is possible to define how many WP versions that should be supported. WPTRT#77

Sounds like an interesting sniff.

IMHO that sniff should probably be pulled here and added to the extra ruleset and then included in the TRT ruleset.

Currently we are using WordPress_AbstractFunctionRestrictionsSniff to check for function restrictions. Would it better to extend this to support the feature of the number of supported WP versions or should there be a separate sniff for deprecated functions?

I don't understand the question cause if you are using the WordPress_AbstractFunctionRestrictionsSniff sniff, you are already extending it.... Could you clarify ?

[grappler]

IMHO that sniff should be probably be pulled here and added to the extra ruleset and then included in the TRT ruleset.

Yes, I agree. Will get it ported over once it is ready.

I don't understand the question cause if you are using the WordPress_AbstractFunctionRestrictionsSniff sniff, you are already extending it.... Could you clarify ?

WordPress_Sniffs_Theme_DeprecatedWPFunctionsSniff implements PHP_CodeSniffer_Sniff instead of extending WordPress_AbstractFunctionRestrictionsSniff. I was wondering if we wanted to adapt WordPress_AbstractFunctionRestrictionsSniff for it to be able to only check certain WP version compatibility. Hope this makes sense.

[grappler]

Do we want to have a new sniff for checking deprecated WP functions or do we want to change WordPress_AbstractFunctionRestrictionsSniff so that it support the WP versions and the ability to define the minimum supported version?

[JDGrimes]

@grappler I was thinking that there wouldn't really be much use for the minimum supported version in other function restriction sniffs, but now I'm not sure. Probably the best thing to do is to introduce a new abstract sniff that extends WordPress_AbstractFunctionRestrictionsSniff, that will provide the WP version related features, and then extend your sniffs from it (if that makes sense, which it might not depending on differences between your sniffs and that one). If we find that the version checking feature is useful to a lot of the other function restriction sniffs, and that it doesn't provide extra overhead for those that don't need it, we could decide to move those features directly to WordPress_AbstractFunctionRestrictionsSniff in the future. But really, I don't think that will be necessary.

[grappler]

@jrfnl raised a point in ##826 (comment)

I wouldn't be adverse to both this sniff as well as the one for WP.DeprecatedFunctions being added to Core as - obviously - core shouldn't be using methods and parameters which it has deprecated, but others might have a different opinion.

I think it would be best discussed here as the decision would would affect any future sniffs.

For me Core is not just the coding standards that would apply to WordPress core but also the coding standards that every plugin and theme would use to have the code look the same. The deprecated sniffs displays a notice regardless if it has been added for backwards compatibility or a deprecated functionality is unknowingly being used. If I not mistaken core still uses deprecated functionality as a fallback as it is only deprecated and not removed.

[GaryJones]

WordPress.WP.DeprecatedFunctions, WordPress.WP.DeprecatedClasses and WordPress.WP.DeprecatedParameters all exist in WPCS.

Still outstanding from the initial list:

• Deprecated constructors (calls to _deprecated_constructor())
• Deprecated files (calls to _deprecated_file())
• Deprecated hooks (calls to _deprecated_hook())

[jrfnl]

Note: to find deprecated hooks we should be looking for apply_filters_deprecated() and do_action_deprecated() calls as _deprecated_hook() as such is only implemented in those two functions.

[jrfnl]

These sniffs would greatly benefit from tooling to create the initial lists and allow for updating the lists with ease. See #1803