Kleinast commented 2 years ago

Describe the bug I get an error when run phpcs or phpcs-fix when I upgrade my project on php 8.1. vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php:32 it seem some path files are "null" in LocalFile->files and php 8.1

PHPCS output here

PHP Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: trim(): Passing null to parameter #1 ($string) of type string is deprecated in /srv/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php on line 32 in /srv/vendor/squizlabs/php_codesniffer/src/Runner.php:604 Stack trace:

0 [internal function]: PHP_CodeSniffer\Runner->handleErrors()

1 /srv/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(32): trim()

2 /srv/vendor/squizlabs/php_codesniffer/src/Files/FileList.php(192): PHP_CodeSniffer\Files\LocalFile->__construct()

3 /srv/vendor/squizlabs/php_codesniffer/src/Runner.php(484): PHP_CodeSniffer\Files\FileList->current()

4 /srv/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()

5 /srv/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()

6 /srv/vendor/bin/phpcs(120): include('...')

7 {main}

thrown in /srv/vendor/squizlabs/php_codesniffer/src/Runner.php on line 604

Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: trim(): Passing null to parameter #1 ($string) of type string is deprecated in /srv/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php on line 32 in /srv/vendor/squizlabs/php_codesniffer/src/Runner.php on line 604

Versions (please complete the following information):

PHP: 8.1
PHPCS: [3.7.1]

This small PR seem fix the problem https://github.com/squizlabs/PHP_CodeSniffer/pull/3676 (but as said in PR, not sure it is a total fix)

jrfnl commented 2 years ago

@Kleinast Could you please elaborate how you got this deprecation notice and how it can be reproduced ?

What type of setup are you using ? (Composer, PHAR, etc)
How are you running PHPCS ? (CLI, via IDE, etc)
What command are you using ?
Are you using a custom ruleset or one of the build-in rulesets ? And if so, which ?
Are you using any external standards ?

Kleinast commented 2 years ago

* What type of setup are you using ? (Composer, PHAR, etc)
We use composer

How are you running PHPCS ? (CLI, via IDE, etc) running from cli

What command are you using ? php vendor/bin/phpcs php vendor/bin/php-cs-fixer fix --dry-run --diff

Are you using a custom ruleset or one of the build-in rulesets ? And if so, which ? we are using this package for ruleset https://github.com/Geolid/phpcs

Are you using any external standards ? Not sure what you mean by external standards

jrfnl commented 2 years ago

I'll have a look see, but may still need more information.

php vendor/bin/phpcs
php vendor/bin/php-cs-fixer fix --dry-run --diff

Just FYI: php-cs-fixer is a completely different tool. The fixer which is included with PHPCS can be called using phpcbf.

jrfnl commented 2 years ago

Tried to reproduce and failed.

I created a new project.
Ran composer require --dev geolid/phpcs
Created a simple dummy file to run tests on.
Copied the phpcs.xml.dist file to the project root and adjusted the file directive.
Ran vendor/bin/phpcs -ps . and saw the expected CS errors and no errors related to the LocalFile class.

Ran with PHPCS develop and PHP 8.1.10.

In other words, please post an exact reproduction scenario as, as things are, the issue is not reproducible.

Not sure what you mean by external standards

The geolid/phpcs package is an external standard which provides a ruleset.

jrfnl commented 2 years ago

Note: I do see some weird stuff going on in that repo, but not sure if it is related and you should probably report all that to the maintainers of that package:

The ruleset.xml file contains a <config name="installed_paths" ... directive. This should generally not be used in an installable external standard and may be the cause of the problem.
That <config name="installed_paths"... directive "registers" paths to at least two other external standards which aren't being installed.
The composer.json file includes at least one external standard which includes the dealerdirect/phpcodesniffer-composer-installer Composer plugin, which will actually register the installed external standards correctly.

Based on this, I have a feeling this is an issue with the external standard you are using doing things wrong and not something which needs to be solved in PHPCS.

Kleinast commented 2 years ago

thx a lot for your tests, i'll dig into the geolid/phpcs package or try to reproduce the error on tiny project

Kleinast commented 2 years ago

I tryed to dig, and it seem it is linked to the parallel feature, I get no error on whe it is set to 1 and I get some trim() error when more than 1. I think the batches in parrallel may be create some $path = $todo->key(); = null but I dont understand very much what exactly is the cause of that.

Kleinast commented 2 years ago

I try different configs on my project and found what in my conf create the issue, geolid/phpcs is not the cause I tryed without it. In my config:

<?xml version="1.0"?>
<ruleset name="PHP CS">
    <arg name="basepath" value="."/>
    <arg name="colors"/>
    <arg name="parallel" value="75"/>
    <arg name="extensions" value="php"/>
    <arg value="p"/>

    <file>src</file>
    <file>databases</file>
    <file>tests</file>
    <file>public/index.php</file>
    <file>databases/DoctrineMigrations</file>

    <rule ref="./vendor/geolid/phpcs/src/Geolid/ruleset.xml"/>
</ruleset>

The error trigger when parallel is more than 1 and I reference this files to check:

    <file>databases</file>
    <file>databases/DoctrineMigrations</file>

If I keep only one of this two file to check, I don't get the error anymore So it seem that parrallel testing with config referencing common file créate some null path in FileList->current()

saas786 commented 1 year ago

~~Same issue here...~~ Apologies, my issue was related to https://github.com/WordPress/WordPress-Coding-Standards

Using

php 8.1.7
squizlabs/PHP_CodeSniffer 3.7.1
slevomat 8.5.2

ConEmu64_bn2LNjJKi2

jimmy-sandoval commented 1 year ago

I have the same error in multiple php files:

Using

PHP 8.1.11 (cli) (built: Sep 29 2022 19:44:28) (NTS) Copyright (c) The PHP Group Zend Engine v4.1.11, Copyright (c) Zend Technologies with Zend OPcache v8.1.11, Copyright (c), by Zend Technologies
squizlabs/PHP_CodeSniffer 3.7.1
Moodle LMS

Screenshot 📷

jrfnl commented 1 year ago

@jimmy-sandoval Not the same issue. Different problem source.

jrfnl commented 1 year ago

what can you recommend?

@jimmy-sandoval

Wrong repo - PHPCompatibility is maintained elsewhere.
Don't pollute an unrelated issue with your own issue.
In your case - run PHPCS like so: phpcs -d error_reporting="E_ALL&~E_DEPRECATED" (or adjust in your php.ini) and you should be fine.

jimmy-sandoval commented 1 year ago

@jrfnl thanks a lot, i will remove my comments

squizlabs / PHP_CodeSniffer

Parallel running with multiple file directives leads to "passing null to non-nullable" notice on PHP 8.1 #3677

0 [internal function]: PHP_CodeSniffer\Runner->handleErrors()

1 /srv/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(32): trim()

2 /srv/vendor/squizlabs/php_codesniffer/src/Files/FileList.php(192): PHP_CodeSniffer\Files\LocalFile->__construct()

3 /srv/vendor/squizlabs/php_codesniffer/src/Runner.php(484): PHP_CodeSniffer\Files\FileList->current()

4 /srv/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()

5 /srv/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()

6 /srv/vendor/bin/phpcs(120): include('...')

7 {main}