PHPCompatibility / PHPCompatibilityWP

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

Many false alarms. #54

Closed AphidGit closed 4 months ago

AphidGit commented 5 months ago

Maybe I'm doing something wrong, but it doesn't seem like the compatibility 'ignore' function for backfills works for my installation of phpcompatibility very well at all. It's reporting many backfills, that appear properly annotated upon inspection, as errors. Not just in plugins, but in the core wordpress code as well.

Here's an example of what I mean. My script to check php code for errors in 3rd party websites running on a server that's going to update its default php version eventually does something like this:

SITENAME=mysite.com
STANDARD=PHPCompatibilityWP
php phpcs.phar -p /var/www/vhosts/$SITENAME/ --standard=$STANDARD --extensions=php,phar,php5,php7,php8 | tee /usr/src/phpSniffer/results/$SITENAME.txt;

That gives a bunch of output, and a lot of errors that, on closer inspection, should have been ignored as they're backfills/polyfills or alternative code branches for older php versions and/or require specific settings like 'use mysql instead of mysqli'.

Here's one such example.

FILE: /var/www/vhosts/mysite.com/httpdocs/wp-admin/includes/class-wp-debug-data.php
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------
 861 | ERROR | Function mysql_get_client_info() is deprecated since PHP 5.5 and removed since PHP 7.0
------------------------------------------------------------------------------------------------------

Checking the contents of that file, here's the relevant snippet:

                if ( isset( $wpdb->use_mysqli ) && $wpdb->use_mysqli ) {
                        $client_version = $wpdb->dbh->client_info;
                } else {
                        // phpcs:ignore WordPress.DB.RestrictedFunctions.mysql_mysql_get_client_info,PHPCompatibility.Extensions.RemovedExtensions.mysql_DeprecatedRemoved
                        if ( preg_match( '|[0-9]{1,2}\.[0-9]{1,2}\.[0-9]{1,2}|', mysql_get_client_info(), $matches ) ) {
                                $client_version = $matches[0];
                        } else {
                                $client_version = null;
                        }
                }

So it has a 'phpcs:ignore' comment, yet that comment itself appears to be, well, ignored.

php phpcs.phar --version
PHP_CodeSniffer version 3.7.2 (stable) by Squiz (http://www.squiz.net)

php --version
PHP 7.0.33-0+deb9u12 (cli) (built: Oct 26 2021 17:51:39) ( NTS )
jrfnl commented 5 months ago

@AphidGit I don't know what you expect with opening this issue.

First off, the ignore annotation feature is a PHPCS feature and not specific to PHPCompatibility, so if there would really be a problem, it should be reported to the PHPCS repo. Secondly, aside from some teething problems in the beginning, I haven't seen any real issues with ignore annotations for quite a while and based on the issue as described, I cannot reproduce the behaviour you seem to be seeing.

What PHPCompatibility version are you using ? Is there any way you can describe your issue which would allow me to reproduce the issue ?


I do see one thing in your script which you may want to change, but that shouldn't be much of an issue (as those extensions are barely used) and is unrelated to what you are reporting:

---extensions=php,phar,php5,php7,php8
+--extensions=php,phar/php,php5/php,php7/php,php8/php

(PHPCS still supports multiple "parsers" at this time, so if you add extensions, you need to tell it what parser should be used for that extension)

AphidGit commented 5 months ago

PHPCompatibility version is 9.3.5, according to its changelog.md. I made a little shell script so you can replicate the way I've setup phpSniffer/phpCompatibility.

#!/bin/bash 

# This script installs php compatibility checker. 
# Use to check for deprecated functionality in webapps.

cd /usr/src
mkdir -p phpSniffer
cd phpSniffer
wget https://squizlabs.github.io/PHP_CodeSniffer/phpcs.phar
# For non-wp sites: 
git clone https://github.com/PHPCompatibility/PHPCompatibility
# For wordpress sites. 
git clone https://github.com/PHPCompatibility/PHPCompatibilityWP
# Dependency
git clone https://github.com/PHPCSStandards/PHPCSUtils
git clone https://github.com/PHPCompatibility/PHPCompatibilityParagonie
# Set. 
php phpcs.phar --config-set installed_paths  /usr/src/phpSniffer/PHPCompatibility/,/usr/src/phpSniffer/PHPCompatibilityWP/,/usr/src/phpSniffer/PHPCSUtils,/usr/src/phpSniffer/PHPCompatibilityParagonie

## Usage example: 
# cd /usr/src/phpSniffer
# SITENAME="example.com"
# STANDARD=PHPCompatibilityWP
# mkdir -p /usr/src/phpSniffer/results
# php phpcs.phar -p /var/www/vhosts/$SITENAME/ --standard=$STANDARD --extensions=php,phar,php5,php7,php8 | tee /usr/src/phpSniffer/results/$SITENAME.txt;

I'm also trying to run it on different versions of linux, to see if differences in the PHP versions supplied by the OS could explain it. While it officially supports using very old versions to do the 'sniffing', maybe something about them might cause it to ignore comments,

On a newer debian machine, the exact same site, files copied over, will give the same spurious error about mysql_get_client_info.

$ php --version
PHP 8.2.7 (cli) (built: Jun  9 2023 19:37:27) (NTS)
$ uname -a 
Linux s05 6.1.0-17-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.69-1 (2023-12-30) x86_64 GNU/Linux
jrfnl commented 5 months ago

@AphidGit Ah! Now I see your problem:

git clone https://github.com/PHPCompatibility/PHPCompatibility

You are cloning PHPCompatibility, which means you get the bleeding edge develop branch (10.0.0), while you are combining that with PHPCompatibilityWP and PHPCompatibilityParagonie for 9.3.5.

10.0.0 will contain some breaking changes and neither those rulesets, nor the ignore annotations in WP Core have been updated for that (yet) as 10.0.0 has not been released yet.

If you run with the -s flag, you would have figured this out as the error code being ignored is not the same as the one for the error you are seeing.

All in all, if you manage the codebase you are running PHPCompatibility on, feel free to use develop. If you don't manage the codebase you are running PHPCompatibility on, you can't expect that codebase to be updated (yet) for a non-released future version, so you'd be better of using a tagged release, i.e. add a line to checkout the 9.3.5 tag or the master branch of PHPCompatibility.