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

Upgrade Psalm to 6.7 and silence issues #19602

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

kamil-tekiela
Copy link
Contributor

@kamil-tekiela kamil-tekiela commented Feb 17, 2025

This PR upgrades Psalm and silences all the newly enabled nonsense checks introduced in vimeo/psalm#11283:

  • Final classes. While most agree that classes should be final by default, our testing scenario demands that we allow classes to be doubled.
  • Override attribute. I don't see much value in this attribute. It would have to be added to many files for little benefit. All it does is help find potentially dead code, which static analysers and IDEs can already do.
  • Strict binary operands. This one just seems plain wrong to me. PHP can happily cast the operands into the required type without causing any issues. echo 'Hello' . 42; is perfectly fine in my opinion. Suppressing this check would require scattering explicit type casts all over the code. This really shouldn't be an issue reported by static analysis. Maybe I am missing something here, but I couldn't find any justification for this one.

These checks can be enabled later on in separate PRs, but for the sake of a seamless Psalm upgrade let's keep them disabled as they previously were.

Signed-off-by: Kamil Tekiela <[email protected]>
@MauricioFauth MauricioFauth merged commit 81a057a into phpmyadmin:master Feb 17, 2025
40 of 41 checks passed
@MauricioFauth MauricioFauth self-assigned this Feb 17, 2025
@MauricioFauth MauricioFauth added this to the 6.0.0 milestone Feb 17, 2025
@kamil-tekiela kamil-tekiela deleted the vimeo/psalm branch February 17, 2025 20:39
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 this pull request may close these issues.

2 participants