Deprecation-message for PHP 8.4 at PHP_CodeSniffer\Tests\TestSuite::run()

[alexgit2k]

Describe the bug

Deprecation-message fpr PHP 8.4 at PHP_CodeSniffer\Tests\TestSuite::run(): Implicitly marking parameter $result as nullable is deprecated, the explicit nullable type must be used instead in composer/squizlabs/php_codesniffer/tests/TestSuite.php on line 26

To reproduce

Steps to reproduce the behavior:

1. git clone https://github.com/PHPCSStandards/PHP_CodeSniffer
2. find PHP_CodeSniffer -type f -name "*.php" -print0 | xargs -0 -I{} sh -c 'php -d error_reporting=E_ALL -d display_errors=1 -l "{}" 2>&1 | grep "Deprecated:"'
3. See error message displayed

Deprecated: PHP_CodeSniffer\Tests\TestSuite::run(): Implicitly marking parameter $result as nullable is deprecated, the explicit nullable type must be used instead in PHP_CodeSniffer/tests/TestSuite.php on line 26

Expected behavior

No deprecation-message

Versions (please complete the following information)

Operating System	Rocky Linux 9.4
PHP version	8.4
PHP_CodeSniffer version	3.12 / master
Standard	-
Install type	composer local

Please confirm:

• I have searched the issue list and am not opening a duplicate issue.
• I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• I have verified the issue still exists in the master branch of PHP_CodeSniffer.

[jrfnl]

Closing as bogus.

The PHPCS test suite was already fixed a year ago in #404. The file you refer to is only used with PHP < 7.1 and PHPUnit < 7, so should NOT be changed.

[alexgit2k]

Oh, I am sorry. I thought the 7 in TestSuite7.php meant PHP 7.0 and below (and TestSuite.php was meant for 7.1 and up).

[jrfnl]

@alexgit2k You could have just tried to run the tests to see everything was fine ... ? Making assumptions like this is harmful and wastes people's time.

[alexgit2k]

Since I can't be sure about test coverage or calls from other files/functions, linting is good to catch issues.
It's not perfect, but it helped me to spot many deprecations in other packages in the last years.

[jrfnl]

@alexgit2k I agree linting/static analysis can help a lot, which is why I write that type of tooling. But a linting finding alone is no excuse for filing nonsense reports.

• You could have checked how that file was loaded in the test setup.
• You could have checked the CI in this project to verify that the tests were being run (without any problems) on PHP 8.4.
• Or, as I said before, you could have tried to run the tests yourself.
• You could even have searched this repository and found similar issues previously closed for the same reason.

The bug report template asks the "To confirm" items for a reason and you blindly checked two checkboxes for things you clearly didn't do. Please stop wasting maintainers time.

[alexgit2k]

Juliette, I do not have the time to go through every issue in detail. My time is better invested in finding issues, even if they are not 100% correct.

I checked master before and the linting bug was and still is there 😉
I just looked more closely at TestSuite7.php before writing this issue. So I suggest to put a comment in the file itself, that it is not used for PHP > 7.0. That may help others.

[jrfnl]

My time is better invested in finding issues, even if they are not 100% correct.

No, it's not. Your time is better spend reporting 10 valid issues versus reporting 100 bogus issues because you didn't do your due diligence. You are wasting people's time, including your own.

[alexgit2k]

It seems this discussion may not be productive at this point. I will continue with my reports, bogus ones are very rare among them.

You mentioned that similar issues have occurred for the same reason previously. Instead of continuing this debate, it would be more efficient to simply add a note in the file about it. This could save your valuable time for future occurrences.

[jrfnl]

Instead of continuing this debate, it would be more efficient to simply add a note in the file about it. This could save your valuable time for future occurrences.

No. The file is slated for removal as anyone who had done their due diligence could know.