phpstan / phpstan-strict-rules

Extra strict and opinionated rules for PHPStan
MIT License
598 stars 46 forks source link

Only allow loose comparison with numeric operands #40

Open jasny opened 5 years ago

jasny commented 5 years ago

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

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

The option to disallow == and != completely doesn't cover the full problem as other comparison operators are still allowed and might give unexpected result. It also disallows cases where you do want to use == because you're comparing numbers.

Good

42 == 42.0 // true
42 == "42" // true
42 == "4.2e1" // true
42 < "0xf1" // true

Bad

// convert to int, I guess
1000 >= "42 bytes" // true
(int)"42 bytes" // 42

// but not always :-(
(int)"4.2e1 bytes" // 42
1000 >= "4.2e1 bytes" // false ??

Universal rules of logic state that if a > b and b > c than a > c

$a = '42';
$b = 10;
$c = '9 dwarfs';

$a > $b // true
$b > $c // true
$a > $c // false ??

The spaceship operator should also not be used for strings.

function cmp1(string $x, string $y) {
    return $x <=> $y;
}

function cmp2(string $x, string $y) {
    return ($x . 'Foo') <=> ($y . 'Foo');
}

// Both functions should do the same, but...
cmp1("55", "5.5e1"); // 0
cmp2("55", "5.5e1"); // 1

The logic behind wether or not a string is converted to a number is just to complex. It's party explained in the manual, but that's not conclusive.

For strings you SHOULD always use === or strcmp.

While == could be useful to compare objects, the strange behaviour on strings makes this too dangerous.

$one = (object)['a' => 0];
$two = (object)['a' => '0.0'];
$three = (object)['a' => 'bar'];

$one == $two; // true
$one == $three; // true
$two == $three; // false
jasny commented 5 years ago

This supersedes #12. That ticket fails to properly identify and describe the problem.

jasny commented 5 years ago

Given #38, the ==, !=, >= and <= should also not be used if both parts are floats. This is already covered by that PR but could be covered by this test instead.

jasny commented 5 years ago

Also see