phpstan / phpstan-strict-rules

Extra strict and opinionated rules for PHPStan
MIT License
604 stars 48 forks source link

disallowedLooseComparison - allow only DateTime/DateTimeImmutable #232

Open realjjaveweb opened 11 months ago

realjjaveweb commented 11 months ago

disallowedLooseComparison is a nice rule and can be applicable to most of the cases, except DateTime/DateTimeImmutable

Comparing these 2 using comparing operators started in PHP 5.2.2, and is documented even in the current documentation:

Note: DateTimeImmutable and DateTime objects can be compared using comparison operators.

If we dig deeper in the php-src - https://github.com/php/php-src/blob/master/ext/date/php_date.c we can see that date_object_handlers_date.compare = date_object_compare_date; and date_object_handlers_immutable.compare = date_object_compare_date; have both the same comparator which leads us to timelib_time_compare in https://github.com/php/php-src/blob/master/ext/date/lib/timelib.c#L75 where we can see it DOES compare microseconds which is something some people may worry about.

In any case - I understand why PHP docs for object comparison say this:

When using the comparison operator (==), object variables are compared in a simple manner, namely: Two object instances are equal if they have the same attributes and values (values are compared with ==), and are instances of the same class.

It's because the == allows comparing objects with nested objects. But in many opinion worlds that is still bad and one should rather specify specifically what is to be compared => however that is not the case of DateTime/DateTimeImmutable.

So the core point is - one should be able to make an exception for disallowedLooseComparison - have it option something like "allowDateTime" or maybe just allow it by default.

Final question is whether this exception should only apply to the core classes, or, should also apply to any classes that extend them (like e.g. Carbon).

ondrejmirtes commented 11 months ago

Hi, you can solve that today by crafting the right regex to ignore specific errors like this: https://phpstan.org/user-guide/ignoring-errors#ignoring-in-configuration-file

realjjaveweb commented 11 months ago

To ignore some custom rule that would e.g. force == for \DateTime(Immutable) the regex would go something like this: in the file "phpstan.neon.dist" (in your app root):

parameters:
    # ...
    ignoreErrors:
        # ...
        - '#Some Sentence You Want To Ignore\.#'
ondrejmirtes commented 11 months ago

Where is Cannot compare DateTime instances with === coming from? That's not how the message looks like in phpstan-strict-rules.

realjjaveweb commented 11 months ago

@ondrejmirtes Oh sorry - that's a custom rule - I will remove that comment to not confuse future visitors... it was irrelevant. :see_no_evil:

The core point is that following

Loose comparison via "==" is not allowed. 💡 Use strict comparison via "===" instead.

Should not be triggered for \DateTime & \DateTimeImmutable since PHP has custom comparing implementation for these 2.