Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Rules to only allow loose comparison with numeric operands #41

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

Closed
jasny wants to merge 5 commits into phpstan:1.4.x from jasny:numeric-loose-comparison
Closed

Rules to only allow loose comparison with numeric operands #41

jasny wants to merge 5 commits into phpstan:1.4.x from jasny:numeric-loose-comparison

Conversation

@jasny
Copy link

@jasny jasny commented Oct 20, 2018
edited
Loading

strictly and strongly typed code with no loose casting for those who want additional safety in extremely defensive programming.

Loose comparison does loose casting with unexpected result. To achieve the purpose of this library, this has to be dealth with.


Similar to arithmetic operators (+/-/*///**/%), loose comparison operators should only be used for numeric (and DateTimeInterface) values. This is true for ==, !=, <, >, <=, >= and <=>.

Using any of these operators for non-numeric values may lead to unexpected results.

See issue #40 for more info.

adaamz reacted with thumbs up emoji Majkl578 reacted with thumbs down emoji
Copy link
Author

jasny commented Nov 5, 2018

@carusogabriel Please take a look

Loose comparison should only be done on numeric or DateTime values.
Fixes #40
Closes #12 
Blindly copied from `OperandsInArithmeticAdditionRule`, but doesn't belong here.
Fixed merge error in rules.neon.
Fixed tests for v0.11 where TypeNodeResolver requires a container.
Copy link
Author

jasny commented Sep 20, 2019

@ondrejmirtes Fixed and updated. If anything more is needed for this to be merged, please let me know.

Copy link

enumag commented Sep 26, 2019

<, >, <=, >= and <=> should be allowed for DateTimeInterface values.

Copy link
Author

jasny commented Sep 27, 2019

Co-Authored-By: Jáchym Toušek <enumag@gmail.com>
Copy link
Member

Hi, I tried out this rule and when I get this message:

Only int|float|DateTimeInterface is allowed in ==, null given on the left side.

It's not obvious to the user what they should do instead. So what would probably be better is to center the wording about "you need to use === instead of ==" rather than that something is not allowed...

Can you bring some ideas about the rule positioning and wording with regard to this? Thanks.

Copy link
Author

jasny commented Aug 18, 2020
edited
Loading

@ondrejmirtes I'm happy to change the text. However, I'm unsure about the wording.

The current messages are copied from the existing rules for arithmetic operators and boolean conditions. Do you want something completely different? What is a good example that I might copy?e

Note that what you should do isn't always clear for other operators than == and != (similar to the arithmetic operator rules). What does [object] > 99 mean? I don't know, but it's probably not correct.

Copy link
Contributor

IMHO, the rule should only apply to <, >, <=, >= and <=>.

== and != should be handle (or maybe is already) differently, because === and !== should always be preferred.

@jasny jasny closed this by deleting the head repository May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@enumag enumag enumag left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /