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

Using phpcompatibility/php-compatibility version 9.X while with the php >= 8.0, it should be develop branch #75

Closed
joshirohit100 opened this issue Oct 1, 2024 · 5 comments · Fixed by #76

Comments

@joshirohit100
Copy link

https://github.com/PHPCompatibility/PHPCompatibility - Last release of this package was on 5 years back and is 9.3.5

Based on various issues in the https://github.com/PHPCompatibility/PHPCompatibility package, it seems like for php >= 8.0 compatibility, its recommended to use the develop (10) branch but this package seems pinned to version 9 only and not allow to use.

@joshirohit100 joshirohit100 added the bug Something isn't working label Oct 1, 2024
@danepowell
Copy link
Collaborator

danepowell commented Oct 1, 2024

@TravisCarden you added the php compatibility package and sniff, what do you think? I'm not thrilled about moving a dependency to a dev release.

@joshirohit100 is the current setup causing any specific issue for you? If not, this seems more like a chore than a bug.

@danepowell danepowell removed the bug Something isn't working label Oct 1, 2024
@TravisCarden
Copy link
Contributor

I'm not thrilled about moving a dependency to a dev release.

I agree on principle, @danepowell. Even more so in this case as it would involve over 2,000 new commits! Who knows what's happened in that space. (My rule is to not review any diff that crashes my browser. 😛)

The whole value of the library is probably debatable at this point. I'll bet PHPStan has a abetter solution. My inclination would be to deprecate PHPCompatibility and provide documentation for removing it or overriding the version (with inline aliases, I would think) and link to better solutions if we know of any.

@danepowell
Copy link
Collaborator

danepowell commented Oct 1, 2024

I like the idea of moving compatibility checking out of the coding standards since it doesn't apply equally to all projects. Acquia CLI, for instance, is exempt from PHP 7.4 compatibility in the 2.x release.

Looks like PHPStan is a better candidate to enforce compatibility; the only downside is we don't have a standard PHPStan ruleset, so projects would need to implement this individually: https://stackoverflow.com/questions/75387960/check-code-compatibility-for-different-php-versions

I'd still like to hear from @joshirohit100 on what specific issues, if any, the older version of the phpcompatibility library is causing.

@joshirohit100
Copy link
Author

We use this library to check for php compatibility for php upgrade and noticed that if we using the 9.5 version (as with the acquia coding standard), it kind of ignoring the deprecations and checks and give all is fine while if we check same with the dev version, and then shows the issues in scan.

I checked for various scans / threads etc on the repo and its recommended to used (at-least thats what I found) to use dev version (or 10). Example - PHPCompatibility/PHPCompatibility#1358

Sample test - check for php 8.4 comptibilty for the code - with the 9.5 version, all fine while with dev version gives

FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 69 | WARNING | Implicitly marking a parameter as nullable is deprecated since PHP 8.4. Update the type to be explicitly nullable instead. Found implicitly nullable parameter: $data.
 71 | WARNING | Implicitly marking a parameter as nullable is deprecated since PHP 8.4. Update the type to be explicitly nullable instead. Found implicitly nullable parameter: $options.

When I tried to upgrade to dev version, I couldn ;t as seems locked by the the acquia package.

danepowell added a commit to danepowell/coding-standards-php that referenced this issue Oct 4, 2024
@danepowell
Copy link
Collaborator

From that issue, it looks like they've been "about to get 10.0 out the door" for over two years now. Hmm.

Anyway... if the maintainer recommends using the develop branch, I'm fine with it in principle. We'll need to evaluate it; we can continue technical discussion in the PR: #76

Long-term, I'd prefer to switch to PHPStan, but I only have a nominal amount of time to contribute to this and that sounds like a lot of work.

danepowell added a commit that referenced this issue Oct 7, 2024
* Fix #75: phpcompatibility/php-compatibility dev-develop

* stability dev
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants