Cannot redeclare wp_set_password() when using roots/bedrock

[mklepaczewski]

It seems that there are few conflicts when using https://roots.io/bedrock/ with roots/wp-password-bcrypt which declares its own variants of some funtions:

• wp_set_password
• wp_generate_password
• wp_check_password
• wp_hash_password

Running vendor/bin/phpstan analyze throws:

$ vendor/bin/phpstan analyze
Note: Using configuration file /var/www/example.com/phpstan.neon.
Fatal error: Cannot redeclare wp_hash_password() (previously declared in /var/www/example.com/vendor/roots/wp-password-bcrypt/wp-password-bcrypt.php:52) in /var/www/example.com/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php on line 105420

Steps to reproduce

# Install roots/bedrock
composer create-project roots/bedrock
cd bedrock
# Create some dummy plugin to test
mkdir -p web/app/plugins/test/
touch web/app/plugins/test/test.php
# Run phpstan
vendor/bin/phpstan analyze web/app/plugins/test/
[...]
Fatal error: Cannot redeclare wp_hash_password() (previously declared in /var/www/example.com/bedrock/vendor/roots/wp-password-bcrypt/wp-password-bcrypt.php:52) in /var/www/example.com/bedrock/vendor/php-stubs/wordpress-stubs/wordpress-stubs.php on line 105420

Workaround

As bad as this is - I just comment out these four functions mentioned above from vendor/php-stubs/wordpress-stubs/wordpress-stubs.php

[szepeviktor]

As bad as this is - I just comment out these four functions mentioned above from vendor/php-stubs/wordpress-stubs/wordpress-stubs.php

👍 👍 👍

I use sed also to do it.

if () : 
    function bla () {}
endinf;

Conditional function definition is an awful thing. But WordPress targets non-developers so we have to live with it.

[szepeviktor]

Related to #33

[mklepaczewski]

May I suggest adding your sed workaround to readme.md (possibly FAQ section)? It seems like this issue will be coming back from time to time.

[szepeviktor]

this issue will be coming back from time to time.

For sure.

[szepeviktor]

@mklepaczewski Done.

https://github.com/szepeviktor/phpstan-wordpress/commit/452e14714df8a584bd0ebf4c4e10da3936330455

[timnolte]

I'd like to revisit this issue and instead of attempting to edit a composer package by hand this should be done automatically via Composer when needed. This should be done using a Composer package patch and not manual edits. The Composer package cweagans/composer-patches can be used to patch this package. I'm going to work up a patch file to be used and do it this way.

[szepeviktor]

to work up a patch file

All right, Tim. Please keep in mind that each release needs that file!

[szepeviktor]

No! I need to be clear. This package is way more complex than I can bear.

Some use cases still need manual intervention. Sorry for that. It is WordPress.

[timnolte]

@szepeviktor well, yes, you are correct that each release will need the patch. And it is no different than having to manually edit the file after every release as well. The thing of it is that in order for this to work with CI/CD I have to be use the patching mechanism in order to maintain the automation.

Also, to be clear I'm not proposing something that needs to be maintained here, other than perhaps making a note of using a patching mechanism instead of just sed.

[szepeviktor]

@timnolte I gladly extend the Dirty corner section for you!! 😍

[timnolte]

@szepeviktor thank you for all of the work you do to maintain this stuff btw. I know it's not always easy, and goes very often without thanks and recognition.

[johnbillion]

I also saw this on a project in the past and decided it was actually roots/wp-password-bcrypt that was at fault for not wrapping its pluggable definition of wp_set_password() in a function_exists check but then I forgot to do anything about it.

[timnolte]

@johnbillion ah, thanks, I may actually patch that package instead of wordpress-stubs, as that is the one that is causing me grief right now.

[szepeviktor]

🙊 @timnolte You may also exclude roots/wp-password-bcrypt from analysis.

[timnolte]

@szepeviktor that's the interesting thing is that I actually did exclude it in my phpstan.neon.dist configuration but it still appears to cause a problem. I explicitly put it in the excludePaths under analyseAndScan and it still causes PHPStan to completely fail.

[timnolte]

I think the fact that it's being autoloaded via Composer might actually be the problem.

[szepeviktor]

You can uninstall that package before analysis or use https://github.com/mcaskill/composer-plugin-exclude-files or find another way.

[timnolte]

@johnbillion so actually, patching the roots/wp-password-bcrypt package doesn't resolve the issue, even with a function_exists wrapper. I just patched the package to see if that fixes things and the way things seem to be autoloading is that the roots/wp-password-bcrypt package appears to be loading before the php-stubs/wordpress-stubs methods.

[szepeviktor]

@timnolte Running and testing cannot be done with this conditional function definition. You may run it untouched, and test it patched. e.g. add return; in the second line of wp-password-bcrypt.php before testing

[timnolte]

@szepeviktor I'm not quite clear on what you mean by:

run it untouched, and test it patched.

This package is a Composer dependency and there is no distinction between "running" and "testing" by a local developer running the command or having PHPStan run/check files in their IDE. Additionally, for local development we are using a Docker setup that has the site running, so again there is no separation of "run" & "test" in terms of Composer actions.

[szepeviktor]

@timnolte Before you perform static analysis you have to get rid of roots/wp-password-bcrypt e.g. by uninstalling it or adding that return; - somehow you have to avoid those function definitions Stubs do not contain if-s, so core's password functions will always be there, even if those functions already exist. Thus you have to modify your environment, there is no clean solution.

[timnolte]

So it actually appears like this site might not actually being using that plugin properly. I completely removed the package and the site seems to actually run without issue, at least logging in which is what I would expect. Going to pull a production database to do more testing to be certain.

This issue causes a pain for any sort of CI/CD and local development linting functionality, like even IDE integration. And to be clear I don't feel like this is actually a PHPStand WordPress issue but that Roots plugin. I know all to well the pains of trying to hack on "modern" development strategies/tools on a system that is not modern by current standards.

[edpittol]

I added the sed commands to post-install-cmd in the scripts section.

{
  "scripts": {
    "post-install-cmd": [
      "sed -i -e 's#function wp_check_password#function __wp_check_password#' vendor/php-stubs/wordpress-stubs/wordpress-stubs.php",
      "sed -i -e 's#function wp_hash_password#function __wp_hash_password#' vendor/php-stubs/wordpress-stubs/wordpress-stubs.php",
      "sed -i -e 's#function wp_set_password#function __wp_set_password#' vendor/php-stubs/wordpress-stubs/wordpress-stubs.php"
    ]
  }
}