Add deprecated code to Code Style check

[Ruud68]

Hi, For me one of the most import Coding Standards is to NOT use deprecated code in my Joomla extensions. Some IDE's have build in support where they read the classes / methods docblocks for the @deprecated tag. A lot of IDE's and Code editors do not have this possibility (Eclipse has this possibility but it is broken after the switch to name spaces in Joomla 3.8). Currently I made the switch to the open source 'text editor' Geany: I love it btw :) Geany has support for PHPCS: when running PHPCS (and PHP Code Fixer) it highlights the warnings and errors in the code. When we add the 'detection' of deprecated code to the Joomla Coding Standards, then Geany (and a lot of other editors) get directly the possibility to also check for (and highlight) deprecated code. I have done some research and found that the WP team is also using their PHPCS for this purpose: here is the discussion > https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/576

Does this also make sense for the Joomla Coding standards? Are the implemented WP sniffs for deprecated classes etc. something we can work with?

[Ruud68]

Okay, just did some experimenting with this deprecated function. Just posting here for discussion / guidelines (I'm new to this all :))

I have created a new sniff (./Joomla/Sniffs/Deprecated/DeprecatedFunctionsSniff.php):

class Joomla_Sniffs_Deprecated_DeprecatedFunctionsSniff extends Generic_Sniffs_PHP_ForbiddenFunctionsSniff

In that class you can set 'forbidden' functions like this:

public $forbiddenFunctions = array(
     'decodeData' => array(
     '5.0' => false, // false = deprecated
     '6.0' => true, // true = removed
     'alternative' => 'Joomla\Input\Files'
     ),

When running PHPCS I get the following report:

-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 186 | ERROR | Function decodedata is deprecated in JOOMLA version 5.0 and removed in JOOMLA version 6.0; use Joomla\Input\Files instead
-----------------------------------------------------------------------------------------------------------------------------------------

thoughts, input, remarks, etc. please :)

[Ruud68]

I don't hope that this is one of those threads that I am the one talking to myself :) #IthinkIjustdidit

I have done a change for everybody to test. It adds a deprecated sniff to the standard Joomla sniffs. The sniff is disabled by default and can be enabled either via a ruleset.xml or via a command line parameter: --runtime-set enable_deprecated_joomla_sniff 1

It currently sniff only a couple of deprecated functions: just for testing the concept.

In the patch there is also a cvs list of all deprecated functions I found in the Joomla 3.8 library directory (the tool to create the list is also in the deprecated directory). This should give some background on the discussion what function to include or not etc.

Looking forward to feedback :)

https://github.com/Ruud68/coding-standards/commit/e52b93557e34a508e8f9344335f55c0921f203c5

[phproberto]

hey @Ruud68 !

I like the idea. The main problem is that a coding standard should be decoupled from anything because it can be used for any package. It makes no sense to check for deprecated CMS stuff in a framework package. Rule can be probably disabled but wouldn't it be better to have an external tool that you can integrate anywhere?

I think that's the approach WP people are suggesting.

I haven't worked/contributed to coding-standards but I'm guessing you are not linking method with classes? I can have a decodeData() method deprecated for 1 class but not for other?

As I said I'm not a coding-standards expert so forgive me any mistake ;)

[Ruud68]

Hi @phproberto thanks for your reply :))) Rule is by default disabled (if you do not set the 'enable' variable, the sniff is not executed. This per request from @mbabker ) For me this IS the external tool that integrates into every IDE that can utilize PHPCS. I came from Eclipse as IDE, Eclipse had the deprecated check default on board, it checked the method against the class by reading the methods docblock (it broke down tho when Joomla switched to namespaces). I know phpstorm also has this functionality build in. The 'problem' is with editors like e.g. Geany (100% open source and lightning fast, OMG I love that tool :)). The only thing missing is the deprecated check. Discussion on the Geany mailinglist led me to doing an external check > via phpcs.

What I have come up with is (as far as I have deducted from the WP code style sniffs) the same way that WP CS does it. Check for methods in an array. The only thing WP has added is the ability to set a version number to check against. WP also has separate sniffs for e.g. deprecated classes.

So what I have come up with works: it checks for methods, by comparing the method name in the array with the methods found in the checked file. I can build the same for deprecated classes: it follows the same logic as methods. Before extending with deprecated classes I first would like to get some consensus on the deprecated methods.

The challenges / questions now are:

1. there is no method to class coupling: so the _construct is deprecated in class A but not in class B, both are reported as deprecated. I think it is possible to extend the check with a class when invoked directly like: JFactory::getApplication()->isSite(), but not with a $app->isSite();

2. there are 700+ methods deprecated (see https://raw.githubusercontent.com/Ruud68/coding-standards/e52b93557e34a508e8f9344335f55c0921f203c5/Joomla/Sniffs/Deprecated/_Joomla38DeprecatedFunctions.cvs). A lot of deprecated methods are very generic (name): like get, check, create, etc. It will only be possible to check on a subset of methods, when not every method is checked, is this still useful?

Would love to get some direction / feedback as I have limited experience in tools that check for deprecated code.

[Ruud68]

Just added DeprecatedClassesSniff.php > https://github.com/Ruud68/coding-standards/blob/master/Joomla/Sniffs/Deprecated/DeprecatedClassesSniff.php It will now correctly check against 300+ deprecated class uses (classes information from Joomla's classmap.php)

-------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 17 ERRORS AFFECTING 17 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------
  107 | ERROR | The use of class JFactory is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Factory instead.
  109 | ERROR | The use of function isSite is deprecated in Joomla version 3.2 and removed in Joomla version 5.0; Use isClient('site') instead.
  246 | ERROR | The use of class JFactory is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Factory instead.
  250 | ERROR | The use of class JURI is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Uri\Uri instead.
  253 | ERROR | The use of class JURI is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Uri\Uri instead.
  263 | ERROR | The use of class JRoute is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Router\Route instead.
  295 | ERROR | The use of class JRoute is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Router\Route instead.
  328 | ERROR | The use of class JFactory is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Factory instead.
  722 | ERROR | The use of class JText is deprecated in Joomla version 4.0 and removed in Joomla version 5.0; Use \Joomla\CMS\Language\Text instead.

[Ruud68]

here it starts (or ends :)) https://github.com/joomla/coding-standards/pull/221