PHPCSStandards / PHPCSUtils

A suite of utility functions for use with PHP_CodeSniffer
https://phpcsutils.com/
GNU Lesser General Public License v3.0
53 stars 7 forks source link

PHPCSUtils Breaks With Deprecation Notice ~~Because of Itself~~ #380

Closed timnolte closed 1 year ago

timnolte commented 1 year ago

Getting the following error when attempting to use the develop branch due to the requirements needed to use the 'develop' branch of PHPCompatibility in order to scan for PHP 8.0 compatibility.

The PHPCSUtils\Tokens\Collections::functionDeclarationTokensBC() method is deprecated since PHPCSUtils 1.0.0-alpha4. Use the PHPCSUtils\Tokens\Collections::functionDeclarationTokens() method instead. in vendor/phpcsstandards/phpcsutils/PHPCSUtils/Tokens/Collections.php on line 719 (Internal.Exception)

This is breaking all uses of PHPCS. I was able to lock in to the tagged 1.0.0-alpha3 version and it did resolve the issue for now.

jrfnl commented 1 year ago

@timnolte How did you manage that ? I've just tried and confirmed, but if you require-dev PHPCompatibility at dev-develop, it will install PHPCSUtils 1.0.0-alpha3. I tried with both minimum-stability dev as well as alpha and in both cases, as expected, PHPCSUtils 1.0.0-alpha3 was installed.

Could it be that you have a root requirement for PHPCSUtils in your project set to dev-develop and if so, why ?

Test config I used:

{
  "name" : "foo/bar",
  "config": {
    "allow-plugins": {
      "dealerdirect/phpcodesniffer-composer-installer": true
    }
  },
  "require-dev" : {
    "phpcompatibility/php-compatibility": "dev-develop"
  },
  "minimum-stability": "dev",
  "prefer-stable": true
}
timnolte commented 1 year ago

Do to additional library requirements such as phpcompatibility/phpcompatibility-wp we have our versioning set as "phpcompatibility/php-compatibility": "dev-develop as 9.3.5" and when we first implemented this a couple of months ago we had also been required to use "phpcsstandards/phpcsutils": "1.x-dev as 1.0" in order for php-compatibility to install. However, additionally, because we have a PHP 8.0 version requirement we are also required to use "dealerdirect/phpcodesniffer-composer-installer": "^0.7.1" which further complicates things.

The short of it is that this all goes back to the need to require PHP 8.0 and run as many PHP 8.0 compatibility tests that are available. Unfortunately, the PHPCS libraries are all behind in regards to the fact that PHp 7.4 will EOL for security updates next month which posts widespread security risks.

jrfnl commented 1 year ago

@timnolte In that case, I can only suggest you run PHPCS with PHP error_reporting set to a value not showing deprecation notices.

This should only be needed for the next few days until PHPCSUtils 1.0.0-alpha4 has been tagged and the upgrade PR to PHPCompatibility has been pulled & merged, but as an end-user you don't need to see deprecation notices from the tooling you use, so turning deprecation notices off for PHPCS isn't necessarily a bad thing.

You can do this either by changing the value in the php.ini file (like for CI) or by running PHPCS like so:

phpcs -d error_reporting="E_ALL&~E_DEPRECATED"

If you have a script in place in your composer.json which your team typically uses, you could add this to the script:

    "scripts": {
        "check-cs": [
            "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs -d error_reporting=\"E_ALL&~E_DEPRECATED\""
        ]
    }

❗ I understand you fixed PHPCSUtils to alpha3 for now, but that will start running into trouble as soon as PHPCompatibility starts merging PRs which rely on Utils alpha4, so the suggestion I'm making here will be more stable while you are using dev versions of these tools.

PHPCSUtils Breaks With Deprecation Notice Because of Itself

Regarding the title of this issue. No, PHPCSUtils doesn't break because of itself. PHPCSUtils itself runs cleanly, but PHPCompatibility uses a few of the deprecated utilities and PHPCS doesn't show a backtrace, but only the top-level error message when it encounters any type of error, which is why it gives the impression that the deprecations come from PHPCSUtils itself, while in reality it is because PHPCompatibility does not allow for alpha4 yet (and rightfully so as it hasn't been tagged yet).

The short of it is that this all goes back to the need to require PHP 8.0 and run as many PHP 8.0 compatibility tests that are available. Unfortunately, the PHPCS libraries are all behind in regards to the fact that PHp 7.4 will EOL for security updates next month which posts widespread security risks.

I'm well aware of that and various PHPCS libraries lagging behind is largely due to the HUGE amount of syntax changes PHP has been introducing since PHP 7.4 and there only being 24 hours in a day.

All I can say is that the changes which have been made recently in PHPCSUtils will allow for (finally) getting PHPCompatibility (and some other libraries) to a release soonish, so bear with me.

jrfnl commented 1 year ago

@timnolte Just checking - did my suggestion work for you ?

jrfnl commented 1 year ago

I'm going to close this issue now.

Running composer update for any of these packages which are on develop with --with-[all-]dependencies should solve any issues you previously saw.

timnolte commented 1 year ago

@jrfnl sorry I didn't get back to you but since some of this came up again, as you rightly pointed out it would, I have updated the Composer dependencies to use alpha4 now. I'll have to evaluate the implications of ignoring deprecated notices, I'm a but hesitant to do so. Thanks for your work and support!

jrfnl commented 1 year ago

@timnolte Glad to hear it all worked out now.

If you've updated to Utils alpha4 and have updated PHPCompatibility dev-develop, you should no longer need to ignore deprecation notices - at least not for PHPCSUtils. No other deprecations will be introduced ahead of the 1.0.0 release.

Having said that, if you have the deprecation ignoring in the PHPCS command itself (like via the composer.json script I suggested above), instead of at php.ini level, you should be okay to leave it in. Any deprecations coming from the tooling itself is not your concern, so can be safely ignored in the knowledge that all these tools - PHPCS, PHPCSUtils, PHPCompatibility - run their tests in CI against the full range of supported PHP versions and have high test coverage, so deprecation notices are generally fixed pretty darn quickly.

One side-note on that: I'm aware of one PHP 8.2 deprecation still existing in PHPCS itself - a PR to solve that is open, but would/will in part remove a previously supported feature, which is why it has been earmarked for the next minor.