PHPCompatibility / PHPCompatibilityWP

PHPCompatibility ruleset for WordPress projects
GNU Lesser General Public License v3.0
171 stars 10 forks source link

In which regard can I take this test serious? #32

Closed Simbaclaws closed 3 years ago

Simbaclaws commented 3 years ago

Dear maintainer,

When I do a test to see whether my wordpress installation is compatible with version 7.4 of php with the following syntax:

./vendor/bin/phpcs -p -d memory_limit=16G --standard=PHPCompatibilityWP --extensions=php --report-full=./report.txt --runtime-set testVersion 7.4 .

it mentions to me files that are inside of wordpress like the following warnings:


FILE: .../wp-admin/includes/class-plugin-installer-skin.php
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
------------------------------------------------------------------------------

FILE: .../wp-includes/SimplePie/Cache/MySQL.php
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
------------------------------------------------------------------------------

FILE: .../wp-includes/random_compat/random_bytes_mcrypt.php
------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------
 58 | ERROR | Function mcrypt_create_iv() is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2; Use random_bytes() or OpenSSL instead
 58 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed
    |       | since PHP 7.2; Use openssl (preferred) or pecl/mcrypt once
    |       | available instead
 58 | ERROR | The constant "MCRYPT_DEV_URANDOM" is deprecated since PHP 7.1
    |       | and removed since PHP 7.2
------------------------------------------------------------------------------

FILE: .../wp-includes/random_compat/byte_safe_strings.php
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
------------------------------------------------------------------------------
 32 | WARNING | INI directive 'mbstring.func_overload' is deprecated since
    |         | PHP 7.2
 86 | WARNING | INI directive 'mbstring.func_overload' is deprecated since
    |         | PHP 7.2
------------------------------------------------------------------------------

FILE: .../wp-includes/wp-db.php
------------------------------------------------------------------------------
FOUND 26 ERRORS AFFECTING 26 LINES
------------------------------------------------------------------------------
  804 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
  811 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
  831 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
  845 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
  877 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1086 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1170 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1466 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1605 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1685 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1688 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1817 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1960 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 1985 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 2007 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 2014 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 2027 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 2058 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3190 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3407 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3409 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3491 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3493 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3530 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3627 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
 3680 | ERROR | Extension 'mysql_' is deprecated since PHP 5.5 and removed
      |       | since PHP 7.0; Use mysqli instead
------------------------------------------------------------------------------

FILE: .../wp-includes/nav-menu.php
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
------------------------------------------------------------------------------

These are files shipped with wordpress! This is the latest version of wordpress... How am I now certain that these errors and warnings are not some kind of false positive?

I would like to make my site as stable as possible for version 7.4 of php but if I can't be certain whether wordpress itself is properly supported for version 7.4, I have a site that I need to maintain for a customer that was previously made by someone else, and I need to be absolutely sure that there are no stability issues when switching to a higher version of php.

Does that mean I should try solve all of these issues? Including wordpress core files? Because the documentation explicitly says: Do not hack core wordpress files.

Should I only look at the files located in the plugins folder and not wordpress core files? How am I certain these are not false-positives as well?

Please let me know what I'm doing wrong or am missing the point here.

wimg commented 3 years ago

Take a look at https://github.com/PHPCompatibility/PHPCompatibilityWP for the Wordpress-specific ruleset. This will avoid getting false positives.

Simbaclaws commented 3 years ago

I am using --standard=PHPCompatibilityWP does this not apply the wordpress-specific ruleset? Full command shown in first post at the top...

I did everything according to the installation using composer properly.

EDIT:

❯ vendor/bin/phpcs -i

The installed coding standards are PEAR, Zend, PSR2, MySource, Squiz, PSR1, PSR12, PHPCompatibility, PHPCompatibilityParagonieRandomCompat, PHPCompatibilityParagonieSodiumCompat and PHPCompatibilityWP
wimg commented 3 years ago

Sorry I completely missed that. I blame my migraine ;-) Perhaps @jrfnl can take a look, she's the resident Wordpress (and PHPCompatibility) specialist.

Simbaclaws commented 3 years ago

Thank you @wimg, hopefully she is able to help me :) Or anyone else for that matter.

dingo-d commented 3 years ago

This package is used to test the compatibility of your app (theme or plugin) with WordPress core, not to test the WordPress core against itself 👀

Simbaclaws commented 3 years ago

I thought it was meant to test an entire wordpress site...

In the case of wanting to see the compatibility of an entire wordpress site, can I use this plugin too?

I suppose all I have to do is go through the plugins and themes that are giving warnings/errors? And ignore core in this case?

dingo-d commented 3 years ago

No, not the entire WP site. You can rest assured that WP core itself is tested against numerous PHP versions, with numerous tools 😄

The way to use this package is to add it to your plugin or theme and check that code (not the WP core). If the codebase contains any incompatible code given the PHP version you target, you'll get a warning or error.

You should also set a phpcs.xml.dist file and set which folders you'd want to check.

You can check the details of using phpcs here: wiki

jrfnl commented 3 years ago

👋🏻 Okay, let me pitch in.

  1. This package is both suitable for WordPress Core as well as for plugins and themes build for WordPress.
  2. This package may sometimes flag things which on closer examination with the wider context available (which the tooling doesn't have) turn out not to be a problem. Those things can be ignored either via an inline ignore comment or via the ruleset.
  3. WordPress Core uses this package and uses both inline ignore comments as well as a custom ruleset phpcompat.xml.dist for those violations which have been verified to be not a problem and passes with a clean bill of health if you run against that ruleset.
  4. There are always limitation to what this package will be able to find due to the nature of static analysis and it is no replacement for having a good set of unit/integration) tests and running those against the PHP version you want to target. At the same time, test suites may be incomplete, so using both this package as well as running the tests is the best of both worlds.

So in conclusion: please do not worry about WP Core itself too much, a subproject is currently underway to make it compatible with PHP 8.1, though there are still some obscure PHP 8.0 issues to be fixed, but those mostly won't impact the average website.

Using this tool for the used plugins and themes is highly recommended when upgrading to a higher PHP version, but, as stated before, there may be work-arounds in place this package cannot see/detect, so any violation thrown should be investigated and evaluated on its own merits by an experienced developer.

Does that help ?

Simbaclaws commented 3 years ago

Thank you @jrfnl,

This makes a lot of sense to me 👍 .

I'm checking all of my plugins and correct execution of the site against a live website with an older version of php by hand. (Seeing whether the output on all pages is the same, and checking whether everything works accordingly)

I was just thinking about using this tool and perhaps others to see if there are any underlying problems with the code I haven't checked yet. Or might've overlooked. This for me was just a last check to see if I can fix some of the php incompatibility issues some of these plugins/themes might have.

For me it's just a tool to not overlook things.

So far everything seems to work fine, but I wouldn't want to run into any issues when keeping this site up for another half a year to a year, just because of some incompatibility issue I haven't looked at.

I'll do some more tests and try to figure out whether what is being shown causes any actual issues or not.

EDIT:

phpcs --standard=phpcompat.xml.dist -q --report=checkstyle | cs2pr

I suppose this one for wordpress core?

jrfnl commented 3 years ago

@Simbaclaws Happy to help and I hope you'll find this tool helpful in the end.

If there's anything you think we can improve in the README to make the usage of the package in the context of WordPress clearer, a PR would be very welcome ;-)

Also be aware that PHPCompatibility 10.0.0 (not yet released) will contain significant improvements, so hopefully we'll be able to find even more issues and prevent more false positives once that's released.

As for this issue, I presume we can close it now ?

Simbaclaws commented 3 years ago

Yes you can!

I'm not sure if I've got the time to make a PR for a suggestion to make it more clear right now. If I find some time to work on this I will 👍

Thank you for this great plugin!