search

PHPCompatibility / PHPCompatibility

PHP Compatibility check for PHP_CodeSniffer
http://techblog.wimgodden.be/tag/codesniffer/
GNU Lesser General Public License v3.0
2.14k stars 190 forks source link

The code is scanned but clear & obvious errors aren't being surfaced #1648

Closed AlchemyUnited closed 5 months ago

AlchemyUnited commented 10 months ago

Bug Description

I've followed the directions in the README and while the scan is running, it's not flagging things that are definitely not appropriate for the testVersion I'm targeting.

Given the following reproduction Scenario

The issue happens when running this command:

phpcs -ps file.php --standard=PHPCompatibility --runtime-set testVersion 7.4

To make sure it's working correctly, I'm using 7.4 full-stop. I don't want 7.4 & up. Ultimately, I have code old code that I'm wanting to analyze to see where it'll bomb in 8.x.

... over a file containing this code:

<?php

function newFeature()
{
    return match( 2 ) {
        1 => 'One',
        2 => 'Two',
        3 => 'Three',
        default => 'Other'
    };
}

xxx;

echo nXewFeature(); // Outputs: Two

I put the xxx as an intentional bug presuming that would get flagged as an error. Regardless, the function would not have been possible with 7.4 so that should be showing as an error. Yet, I'm not getting any errors in the console.

I've used -vv and -vvv and there's plenty of "activity" so it looks like the file is getting scanned but...

    *** END SNIFF PROCESSING REPORT ***

DONE in 147ms (0 errors, 0 warnings)

... with this custom ruleset:

<?xml version="1.0"?>
<ruleset name="My Custom Standard">

      <rule ref="PHPCompatibility"/>
    <config name="installed_paths" value="vendor/squizlabs/php_codesniffer,vendor/phpcompatibility/php-compatibility,vendor/phpcompatibility/php-compatibility-wp,vendor/phpcompatibility/phpcompatibility-paragonie" />

</ruleset>

afaik, I didn't get any errors from the composer'ing. Maybe these details will help?

$ composer --version Composer version 2.6.3 2023-09-15 09:38:21

$ composer show phpcompatibility/php-compatibility name : phpcompatibility/php-compatibility descrip. : A set of sniffs for PHP_CodeSniffer that checks for PHP cross-version compatibility. keywords : compatibility, phpcs, standards versions : * 9.3.5

$ composer show phpcompatibility/phpcompatibility-wp name : phpcompatibility/phpcompatibility-wp descrip. : A ruleset for PHP_CodeSniffer to check for PHP cross-version compatibility issues in projects, while accounting for polyfills provided by WordPress. keywords : compatibility, phpcs, standards, static analysis, wordpress versions : * 2.1.4

$ composer show phpcompatibility/phpcompatibility-paragonie name : phpcompatibility/phpcompatibility-paragonie descrip. : A set of rulesets for PHP_CodeSniffer to check for PHP cross-version compatibility issues in projects, while accounting for polyfills provided by the Paragonie polyfill libraries. keywords : compatibility, paragonie, phpcs, polyfill, standards, static analysis versions : * 1.3.2

I've been trying to troubleshoot this for hours :( I've lost track of the number of times I've run composer clear-cache, composer dump-autoload, and composer update.

Any tips and insights would be appreciated.

I'd expect the following behaviour

I expect to be notified about obvious erros.

Instead this happened

END SNIFF PROCESSING REPORT DONE in 147ms (0 errors, 0 warnings)

Environment

Environment Answer

$ php -v PHP 7.3.1 (cli) (built: Jan 9 2019 22:43:14) ( ZTS MSVC15 (Visual C++ 2017) x86 ) Copyright (c) 1997-2018 The PHP Group Zend Engine v3.3.1, Copyright (c) 1998-2018 Zend Technologies with Xdebug v2.7.0RC2, Copyright (c) 2002-2019, by Derick Rethans

| PHP_CodeSniffer version | x.y.z | PHPCSUtils version | x.y.z | PHPCompatibility version | x.y.z | Install type | e.g. Composer global, Composer project local, git clone, other (please expand)

Tested Against develop branch?

jrfnl commented 10 months ago

@AlchemyUnited

The xxx; code is not a PHPCompatibility issue. You could have a constant defined in your code called xxx and in that case, it would be perfectly valid, though not very useful, code.

As for not seeing any issues regarding the match, PHP 8+ things are in the develop branch (which you are not using, nor have tested against).

When run against PHPCompatibility develop with the command you are using, you would see it being flagged as a feature not available on PHP 7.4:

-----------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------
 5 | ERROR | The "match" keyword is not present in PHP version 7.4 or earlier
   |       | (PHPCompatibility.Keywords.NewKeywords.t_matchFound)
-----------------------------------------------------------------------------------------------------------------------------------
jrfnl commented 10 months ago

Oh and considering you clearly got things running, I presume issue https://github.com/PHPCompatibility/PHPCompatibilityWP/issues/48 can be closed ?

jrfnl commented 10 months ago

@AlchemyUnited A response would be appreciated.

AlchemyUnited commented 10 months ago

Thanks for following. I apologize. My proj manager shifted my priorities and 8.x testing got bumped down the list :(

In any case, yes. The 8.x only code is 8.x only. Via one of the online PHP exe tools (in the browser) I pasted the code and ran it targeting 7.4. It bombed. So my level of confidence is high that it's 8.x only.

That said, once I get back to my laptop I'll share that code. Perhaps the issue is that code's syntax it too new? That said, among other things I'm needing to test WordPress themes. They're not recent so the odds of having PHP 8 issues is very high. Yet I'm not getting anything. No errors. No warnings. The code is 8 clean and I just can't imagine that :(

I'll get back to you. Thx again for the follow up.

AlchemyUnited commented 10 months ago

Here's the PHP 8.x+ code that I would expect 7.4 to flag

<?php

function newFeature()
{
    return match( 2 ) {
        1 => 'One',
        2 => 'Two',
        3 => 'Three',
        default => 'Other'
    };
}

echo newFeature(); // Outputs: Two
AlchemyUnited commented 10 months ago

As for this issue: https://github.com/PHPCompatibility/PHPCompatibilityWP/issues/48#issuecomment-1722303614

Here's the composer info for the project:

doctrine/instantiator                          1.5.0   A small, lightweight utility to instantiate objects in PHP without invoking their constructors
myclabs/deep-copy                              1.11.1  Create deep copies (clones) of your objects
nikic/php-parser                               v4.17.1 A PHP parser written in PHP
phar-io/manifest                               2.0.3   Component for reading phar.io manifest information from a PHP Archive (PHAR)
phar-io/version                                3.2.1   Library for handling version information and constraints
phpcompatibility/php-compatibility             9.3.5   A set of sniffs for PHP_CodeSniffer that checks for PHP cross-version compatibility.
phpcompatibility/phpcompatibility-paragonie    1.3.2   A set of rulesets for PHP_CodeSniffer to check for PHP cross-version compatibility issues in projects, while accounting for polyfi...   
phpcompatibility/phpcompatibility-wp           2.1.4   A ruleset for PHP_CodeSniffer to check for PHP cross-version compatibility issues in projects, while accounting for polyfills prov...   
phpunit/php-code-coverage                      9.2.28  Library that provides collection, processing, and rendering functionality for PHP code coverage information.
phpunit/php-file-iterator                      3.0.6   FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-invoker                            3.1.1   Invoke callables with a timeout
phpunit/php-text-template                      2.0.4   Simple template engine.
phpunit/php-timer                              5.0.3   Utility class for timing
phpunit/phpunit                                9.6.12  The PHP Unit Testing framework.
sebastian/cli-parser                           1.0.1   Library for parsing CLI options
sebastian/code-unit                            1.0.8   Collection of value objects that represent the PHP code units
sebastian/code-unit-reverse-lookup             2.0.3   Looks up which function or method a line of code belongs to
sebastian/comparator                           4.0.8   Provides the functionality to compare PHP values for equality
sebastian/complexity                           2.0.2   Library for calculating the complexity of PHP code units
sebastian/diff                                 4.0.5   Diff implementation
sebastian/environment                          5.1.5   Provides functionality to handle HHVM/PHP environments
sebastian/exporter                             4.0.5   Provides the functionality to export PHP variables for visualization
sebastian/global-state                         5.0.6   Snapshotting of global state
sebastian/lines-of-code                        1.0.3   Library for counting the lines of code in PHP source code
sebastian/object-enumerator                    4.0.4   Traverses array structures and object graphs to enumerate all referenced objects
sebastian/object-reflector                     2.0.4   Allows reflection of object attributes, including inherited and non-public ones
sebastian/recursion-context                    4.0.5   Provides functionality to recursively process PHP variables
sebastian/resource-operations                  3.0.3   Provides a list of PHP built-in functions that operate on resources
sebastian/type                                 3.2.1   Collection of value objects that represent the types of the PHP type system
sebastian/version                              3.0.2   Library that helps with managing the version number of Git-hosted PHP projects
squizlabs/php_codesniffer                      3.7.2   PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects violations of a defined set of coding standards.
theseer/tokenizer                              1.2.1   A small library for converting tokenized PHP source code into XML and potentially other formats

Maybe it's easier to just tear it all out and start over? And start with PHPCompatibilityWP? As it is I had PHPCompatibility working then decide to add WP's. Maybe I missed something somewhere.

@jrfnl Note: I do want to get this working. Unfortunately, this work / project is for an agency where priorities can shift every day or so. I can't always get back to you as quickly as I would like. I've already invested a significant amount of time trying to get this resolved. The only way for me to get a return from that investment is to get this working correctly. Bear with me, please. I'm already demoralized :(

jrfnl commented 10 months ago

I've already invested a significant amount of time trying to get this resolved.

I understand that, but you haven't demonstrated that by sharing what you tried, providing detailed reproduction steps, or even filling out the requested information in the issue report template completely/correctly, which doesn't help in figuring out what you need.

At least the output from composer infoabove clearly shows why things aren't working as you have not installed the Composer PHPCS plugin, which means PHP_CodeSniffer does not know about the standards you installed.

The README has clear instructions on getting it up and running with the Composer PHPCS plugin, so I'm not sure why you decided not to follow those ?

Having said that, as I said before, your code snippet would be flagged when you use the develop branch, but not with the last stable release.

As you say you want to use PHPCompatibilityWP rather than PHPCompatibility, it's a little more complicated to get the develop version up and running. On the other hand, you will no longer need to install the Composer PHPCS plugin yourself, that will be installed automatically as of the 10.0.0 release (currently develop), you just need to give it permission to run (so when asked whether the dealerdirect/phpcodesniffer-composer-installer plugin is allowed to run: answer y).

With the following three commands you can get things running for a project. Run these command from the root directory of your project.

composer config minimum-stability dev
composer require --dev phpcompatibility/phpcompatibility-wp
composer require --dev phpcompatibility/php-compatibility:"dev-develop as 9.99.99"

When asked to give the plugin permission, answer y.

Once you have installed, , you can verify that it is all working correctly and that the standards are installed via:

vendor/bin/phpcs -i

And you should be able to run PHPCompatibility from the root of your project using:

vendor/bin/phpcs --standard=PHPCompatibilityWP

(you will still need to tell it what files to scan and such)

If you prefer a "global" install, replace composer with composer g to install things globally. If the Composer global directory is in your system path, you should then be able to run PHPCompatibility anywhere on your system using:

phpcs --standard=PHPCompatibilityWP

(again you will still need to tell it what files to scan and such)

Also see this external article about this: https://docs.wpvip.com/technical-references/php/version-updates/phpcs-scans/#Upcoming-releases-of-PHPCompatibility

And if you want more information about why no new release has been tagged, have a read through this ticket: https://github.com/PHPCompatibility/PHPCompatibility/issues/1651

I've already invested a significant amount of time trying to get this resolved.

So have I by now, but at least you get paid for it....

jrfnl commented 5 months ago

Closing for lack of response. Presuming it means the provided answer was sufficient.