Feature Request: Please address this plugin's conflicts with php-stubs/wordpress-stubs

[montchr]

Terms

• I have read the guidelines for Contributing to Roots Projects
• This request is not a duplicate of an existing issue
• This is not a personal support request that should be posted on the Roots Discourse community

Summary

It is currently not possible to use phpstan with phpstan-wordpress and the wordpress-stubs packages when installing roots/wp-password-bcrypt via composer.json without resorting to some very hacky (and unreliable/repetitive/frustrating) workarounds.

The full issue is described in szepeviktor/phpstan-wordpress#41.

Motivation

Why are we doing this?

To reduce the frustration of working with WordPress, to improve compatibility, and to facilitate reproducible development tasks in local and CI workflows.

Please read:

• Cannot redeclare wp_set_password() when using roots/bedrock szepeviktor/phpstan-wordpress#41
• Check if function exists in wordpress-stubs.php php-stubs/wordpress-stubs#70

I am aware this has been mentioned indirectly in the past, but it appears that there has been little consideration given to making a change in this plugin despite the numerous accounts over the years. I am referring specifically to #11, but there are other examples.

It would be great if roots/wp-password-bcrypt could find a way to improve compatibility with these other libraries, because both this plugin and those libraries have been positive improvements to the WordPress ecosystem that unfortunately do not mix well.

What use cases does it support?

• Using the widely-used phpstan static analysis tooling with WordPress integration via https://github.com/szepeviktor/phpstan-wordpress
• Using this plugin via Composer installation

What is the expected outcome?

Developers and CI pipelines should be able to run phpstan analyse when this plugin is installed via composer.json and the WordPress stubs are loaded without any conflict.

I encourage the authors of this plugin to listen to the feedback from other developers who have encountered such conflicts and have given specific suggestions as to a fix.

For example:

Check if function exists in wordpress-stubs.php · Issue #70 · php-stubs/wordpress-stubs

I've given a couple of workshops on WordPress+PHPStan and both times this exact problem was the first to arise. We determined that the most correct solution is for the Bedrock bcrypt library to wrap its functions in function_exists() checks because it has no way to ensure that another definition of one of those functions isn't loaded by another plugin. I'm not sure why that is not the case.

Potential conflicts / foreseeable issues

Unknown. I am only a user of both tools reporting my experience and, more importantly, linking the two GitHub issues together because it looks like there's only been isolated conversations in each repo about this issue.

Additional Context

No response

[timnolte]

This issue is still a problem. How are Roots sites even being tested for code quality when this issue exists. Looking at the history of this package and it seems like it is really not even properly maintained anymore.

[swalkinshaw]

What a shitty comment 👍

None of us are familiar with php-stubs/wordpress-stubs and from reading szepeviktor/phpstan-wordpress#41 it doesn't seem there's an obvious solution? This is an open source repo so it would be great if someone who is more familiar with the wordpress-stubs and the issue could contribute a solution.

[timnolte]

What a shitty comment 👍

None of us are familiar with php-stubs/wordpress-stubs and from reading szepeviktor/phpstan-wordpress#41 it doesn't seem there's an obvious solution? This is an open source repo so it would be great if someone who is more familiar with the wordpress-stubs and the issue could contribute a solution.

It's a legitimate question. If PHPStan isn't being used then I have to assume there are other code quality tools being used that are apparently better than PHPStan.

It's also, however, a fact that there don't appear to be any commits to the repository for the last 7 months. So I'm assuming then it just doesn't need any updates as it works fine with whatever code quality tools that Roots is using and is still fully compatible with the latest WordPress Core. If the answer is just that PHPStan is not supported and there is no plan to support it then so be it, and we can close this issue as won't fix.

[oxyc]

One solution is to package this as a mu-plugin instead of a library generoi@0903a48.

I felt this was a controversial change so haven't opened up a PR but happy to do so

Reading the filedoc of pluggable.php they specifically say plugins so it would align better with WordPress expectations.

[swalkinshaw]

😅 this package started out as an mu-plugin: #4

I have zero recollection why it was changed

[retlehs]

The only reason I can think of is that we figured if folks are already using Composer, we could just keep everything in the vendor directory instead of the plugin having to exist in the mu-plugins directory and showing up in the must-use plugins list 🤔

[johnbillion]

Previously: #33

[benvoynick]

Also running into this

[retlehs]

@benvoynick There is no need to bump this issue without adding anything helpful to the discourse 👍

[benvoynick]

Fair enough. I would be happy to try to contribute a PR, but this seems to be at an impasse.

montchr passed on the idea that conditionally declaring the functions would be the way to go.

oxyc suggested it could instead be turned into a mu-plugin.

This leaves two very different paths forward, and discussion since leaves me unclear which path maintainers would prefer to see in a PR, if a preference has been decided. Am I missing something there?

If you would like a recommendation from the recently arrived peanut gallery, I'd think conditional function declaration would be less disruptive, even if it might not be the blank-slate ideal.

[retlehs]

WordPress core is finally adopting bcrypt for passwords. This package should hopefully be able to be archived in the near future, and this issue will no longer be relevant.

See #44