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

Made all constant literal string equality checks use strict equality. #385

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boenrobot
Copy link

Implemented by literally searching for
' ==
" ==
== '
== "
and replacing the "==" with "===".

All other equality checks are left untouched, as changing them may result in different behavior, whereas these ones should behave the same, while reducing the surface area for type related bugs.

@boenrobot boenrobot force-pushed the strict-string-eq branch 2 times, most recently from a0a6f96 to 2e9e5a8 Compare October 11, 2023 14:41
@develart-projects
Copy link
Collaborator

Why exactly we need this, pls? I can see a lot o potential issues with this change.

@boenrobot
Copy link
Author

Why exactly we need this, pls? I can see a lot o potential issues with this change.

The hope was that it would be a trivial change to have less noise in IDEs and static analysis tools when further improving type safety (which in turn is important in ensuring cross version combability as PHP is moving into the direction of requiring more explicit things that were previously implicit).

But then again, the unit test suite already confirms your suspicions, so back to the drawing board for me 🤣

@boenrobot boenrobot marked this pull request as draft October 11, 2023 14:55
@develart-projects
Copy link
Collaborator

I can see a lot of potential troubles with

=== '1'
=== ''

Potentially forcing me to disconnect from upstream, as strict typing is not the way I like to go with PHP.
And backward compatibility is more important here, as static analyzers imo.

@boenrobot boenrobot force-pushed the strict-string-eq branch 2 times, most recently from 624aa82 to 5230746 Compare October 11, 2023 15:31
…ks use strict equality.

Implemented by search & replace.

All other equality checks are left untouched, as changing them may result in different behavior, whereas these ones should behave the same, while reducing the surface area for type related bugs.
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