Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.0 vsprintf #1967

Closed
1 task
str opened this issue Dec 18, 2020 · 23 comments
Closed
1 task

PHP 8.0 vsprintf #1967

str opened this issue Dec 18, 2020 · 23 comments

Comments

@str
Copy link

str commented Dec 18, 2020

Bug Description

When testing with PHP 8 and wpcs 2.3.0 I get the following error message:

PHP Fatal error:  Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given in /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php:1051
Stack trace:
#0 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php(1051): vsprintf()
#1 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php(670): PHP_CodeSniffer\Files\File->addMessage()
#2 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php(778): PHP_CodeSniffer\Files\File->addError()
#3 /home/str/.config/composer/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php(395): PHP_CodeSniffer\Files\File->addFixableError()
#4 /home/str/.config/composer/vendor/wp-coding-standards/wpcs/WordPress/Sniff.php(910): WordPressCS\WordPress\Sniffs\WhiteSpace\ControlStructureSpacingSniff->process_token()
#5 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php(496): WordPressCS\WordPress\Sniff->process()
#6 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(91): PHP_CodeSniffer\Files\File->process()
#7 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Runner.php(630): PHP_CodeSniffer\Files\LocalFile->process()
#8 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Runner.php(434): PHP_CodeSniffer\Runner->processFile()
#9 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
#10 /home/str/.config/composer/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
#11 {main}
  thrown in /home/str/.config/composer/vendor/squizlabs/php_codesniffer/src/Files/File.php on line 1051

I debugged and the 2 params sent to vsprintf where

  • string(97) "Expected exactly one space between closing parenthesis and opening control structure; "%s" found."
  • string(1) "\n"

Minimal Code Snippet

This is the code that generates the error

        // See if there is a custom error message format to use.
        // But don't do this if we are replaying errors because replayed
        // errors have already used the custom format and have had their
        // data replaced.
        if ($this->replayingErrors === false
            && isset($this->ruleset->ruleset[$sniffCode]['message']) === true
        ) {
            $message = $this->ruleset->ruleset[$sniffCode]['message'];
        }
        if (empty($data) === false) {
            $message = vsprintf($message, $data);   // <-- here is what trigers the error
        }

For bugs with fixers: How was the code fixed? How did you expect the code to be fixed?

Error Code

Environment

Question Answer
PHP version 8.0
PHP_CodeSniffer version 3.5.8
WPCS version 2.3.0
WPCS install type Composer global
IDE (if relevant) vscode / cli

Additional Context (optional)

I saw some changes in wpcs around php 8.0, from July. wpcs 2.3.0 was released on May, before those changes. How can I test the latest wpcs changes after 2.3.0?

Tested Against develop branch?

  • I have verified the issue still exists in the develop branch of WPCS.
    I don't know how
@jrfnl
Copy link
Member

jrfnl commented Dec 18, 2020

This was already fixed in #1935. The fix will be included in the next release.

@str
Copy link
Author

str commented Dec 18, 2020

Is there something I can do to get that version without having to wait for 3.0.0?

@dingo-d
Copy link
Member

dingo-d commented Dec 18, 2020

Is there something I can do to get that version without having to wait for 3.0.0?

You can try to pull the develop version of the WPCS into your project, but I wouldn't recommend that. You can just try to ignore that error with // phpcs:ignore comment.

@str
Copy link
Author

str commented Dec 18, 2020

Yea... I saw it's not that easy with the new phpcsutils.

I'll have to downgrated my phpcli to 7.x meanwhile. Thank you.

@str str closed this as completed Dec 18, 2020
@nishantwrp
Copy link

Hi @dingo-d, I recently got this error again. Can you please check if this issue is still present?

@stingray21
Copy link

Will there be a release that includes the fix #1935 before the 3.0.0 release?

@jrfnl
Copy link
Member

jrfnl commented Mar 27, 2021

@stingray21 Spreading the conversation out over several issues is not helpful (yesterday you were asking about this in #1935).

There is no intention for a patch release before the 3.0.0 release.

Note: While PHP_CodeSniffer is run-time compatible with PHP 8.0 since 3.5.7 - i.e. it throws no errors while running -, the last release of PHP_CodeSniffer can not handle most PHP 8.0 syntaxes correctly yet. It is expected that the PHPCS 3.6.0 release will contain most, if not all fixes, needed for that.

@stingray21
Copy link

@jrfnl I apologize, I understand. My intention was not to spread out the conversation, I thought at the time of posting that the questions were different enough (I was trying to make it work for many hours and maybe wasn't thinking that straight anymore). But I can see now that they are very similar.

I don't fully understand your reply though. Does that mean the error will disappear with PHPCS 3.6.0? Is that expected to happen soon?

The latest release of WPCS was almost a year ago, and there is no due date for Milestone 3.0.0 (at least I couldn't find it) with currently 65% finished. A minor patch release would be nice, if only to make it run without errors. Not trying to be pushy, I'm very grateful for your work. I'm just curious and trying to figure out how to make PHPCS with WPCS work.

@jrfnl
Copy link
Member

jrfnl commented Mar 28, 2021

Does that mean the error will disappear with PHPCS 3.6.0?

No, it does not.

Is that expected to happen soon?

Sort of, but no idea when.

The latest release of WPCS was almost a year ago, and there is no due date for Milestone 3.0.0 (at least I couldn't find it) with currently 65% finished. A minor patch release would be nice, if only to make it run without errors. I'm just curious and trying to figure out how to make PHPCS with WPCS work.

The simple answer is: don't use PHP 8.0 yet for PHPCS with WPCS and don't expect PHPCS or WPCS to give accurate sniff results when scanning PHP 8.0 code. I'd recommend you use PHP 7.4 for now.

For the long answer, please see the explanation I gave in another external standard for PHPCS: PHPCompatibility/PHPCompatibility#1236 (comment)

The same what applies for that repo, basically applies here as well. PHP 8 introduced a huge amount of tokenizer changes, probably more than any PHP version before and all of that needs to be accounted for in PHPCS, PHPCSUtils and WPCS and unless more people who actually understand what they are doing, start contributing, it simply comes down to the fact that I am only one person and there are only 24 hours in a day and between PHPCS, PHPCSUtils, PHPCompatibility, PHPCSExtra and WPCS, the first three take priority (the first two because WPCS needs them, the third because accounting for PHP 8 there just has higher urgency than more accurate sniffs in WPCS).

In reality, WPCS 3.0.0 is probably closer to 35% finished (not all actions have equal time investment) and on top of that with PHP 8, a lot of new actions should probably be added to the list.

Also keep in mind, that all the work being done on all of the above mentioned projects is unpaid volunteer work and that I sometimes need sleep too.

@stingray21
Copy link

Thank you for your reply. Again, I'm very grateful for your (and the others) work on the projects, and I know that it is a lot of work that goes into developing and maintaining these. So your work and efforts are very appreciated.

Just to give you the perspective of a new and beginner user of PHPCS and WPCS:

I spent many hours (not complaining, just that I didn't post here after the first 5-min-attempt) trying to get PHPCS with WPCS set up. I tried multiple tutorials and read the documentation, but still couldn't get it to work. Up to a point when I almost gave up, I decided not to use it at all.

PHP8 is out for a while now and active support for 7.4 ends in Nov, therefore I don't think it's unreasonable to start a new project with PHP8 right now.

Since the documentation for PHPCS and WPCS both say under requirements PHP 5.4 or higher, I assumed it would work without error messages on every file and run I tried.

I understand that it takes a lot of work and testing to release a new version. So if there won't be a patch release to fix/suppress the error messages, it would be nice if it's mentioned in the docs that it does not work properly with PHP8 yet. Or some instructions or pointers on how to make it work. I apologize if it's already there and I overlooked it, but maybe it should be in a more prominent spot then.

@jrfnl
Copy link
Member

jrfnl commented Mar 29, 2021

@stingray21 Grateful doesn't pay the bills. There is a pinned, opened issue in the PHPCS repo about PHP 8 support which very clearly shows that PHPCS is not compatible yet and by extension WPCS cannot be either.

If you want things in open source projects like this to move faster, step up and start contributing.

@stingray21
Copy link

I apologize if I offended you in any way. I was not trying to push you to work harder or whatnot. I am planning to contribute more to open source projects, but as I mentioned I am still a beginner and I first need to make things run before I will be able to contribute anything meaningful.

My last comment was an attempt to contribute by giving you constructive criticism. I assume that part of why this project is open source, is that people use it. If beginners (maybe it's just me) can not figure out how to run it, they won't use it.

To me it was not obvious that WPCS (and PHPCS) won't work with PHP8 (because of e.g. requirements PHP 5.4 or higher or As of version 3.5.7, PHPCS will run fine on PHP 8 when scanning code without PHP 8 specific syntax.).

A small note on the project landing page or installation instructions like PHP 5.4 to 7.4 (PHP 8 not fully supported) with maybe a link to the mentioned pinned issue, would have been nice.
This might also help you, if it prevents people asking questions and opening issues.

But I won't bother you anymore about this. Again, my apologies, I am sorry if I caused any trouble or hurt feelings.

@jrfnl
Copy link
Member

jrfnl commented Mar 29, 2021

@stingray21 I gave you the answer you asked for the first time. I explained the situation twice more. I think I've been patient enough. It is easy to push, demand and criticize people in open source, but maybe actually reading what is said and learning effective searching would have been a better way to spend your time and not to waste mine.

@fharper
Copy link

fharper commented May 18, 2021

So good news, the latest release of PHPCS officially support PHP 8.0.

@jrfnl
Copy link
Member

jrfnl commented May 18, 2021

@fharper Eh.. actually... no, it intended to, but doesn't. And you're welcome.

@fharper
Copy link

fharper commented May 18, 2021

@jrfnl it's written in the release notes, am I missing something? They also closed the pinned issue about it because of that release.

@jrfnl
Copy link
Member

jrfnl commented May 18, 2021

@fharper Yes, you are. While the PHPCS 3.6.0 release is mostly compatible with PHP 8.0, it's still not there yet - most notably see squizlabs/PHP_CodeSniffer#3298 (with open PR squizlabs/PHP_CodeSniffer#3320) and squizlabs/PHP_CodeSniffer#3336.

In particularly the second issue (reserved keywords) is pretty darn complex to solve. I'm working on it, but it will still be at least a few more weeks before I have a stable solution.

I also expect there are still quite some - as yet undiscovered - issues with sniffs not handling attributes correctly. For the other PHP 8.0 changes (those which I pulled), I also did a review to find sniffs which would need adjusting.
I wasn't the one to sort out attributes though and no such review has been done. There have been one or two reports so far about sniffs not working correctly and those have been addressed, but I expect there are quite a few more sniffs which still need adjusting.

@fharper
Copy link

fharper commented May 18, 2021

@jrfnl thanks for the reply, I appreciate you took the time to educate me on the topic. Still, I like to be positive, it's going in the right direction :)

@jrfnl
Copy link
Member

jrfnl commented May 18, 2021

@fharper That it is indeed. Still a ways to go though, as even when PHPCS is compatible with PHP 8.0, that doesn't mean WPCS is. So there's still lots of work to be done there.

I intend to open up issues for all the different sniff reviews which will need to be done for that soonish, in case you're interested in helping out with making WPCS compatible.

@fharper
Copy link

fharper commented May 18, 2021

@jrfnl I can certainly give it a closer look, and would be happy to help if I can. Can you link the issues here once you create them (when you have time obviously), that would be super helpful!

@jrfnl
Copy link
Member

jrfnl commented May 18, 2021

Can you link the issues here once you create them

@fharper Well, chances are I'll forget as it's not really directly related to this (closed) issue anyway. Might be better to "watch" the repo if you want to keep an eye on things, if you don't mind.

@fharper
Copy link

fharper commented May 18, 2021

@jrfnl make way more sense, thanks!

@openmindculture
Copy link

After installing PHP 7.4 and configuring PhpStorm to /usr/bin/php74 , the error message still appears, using the latest stable versions of wpcs and php_codesniffer in composer.json:

"require": {
  "php": "^7.4"
},
"require-dev": {
  "squizlabs/php_codesniffer": "^3.7.1",
  "wp-coding-standards/wpcs": "^2.3.0"
},

@jrfnl jrfnl added this to the 3.0.0 milestone Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants