Narrow types based on non-strict equality with constant types

[staabm]

closes phpstan/phpstan#10481

[phpstan-bot]

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

[staabm]

these 4 could be more precise

Suggested change

	if ($mixed == '1') {
	assertType("mixed", $mixed);
	}
	if ($mixed == '0') {
	assertType("mixed", $mixed);
	}
	if ($mixed == 1) {
	assertType("mixed", $mixed);
	}
	if ($mixed == 0) {
	assertType("mixed", $mixed);
	}
	if ($mixed == '1') {
	assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed);
	}
	if ($mixed == '0') {
	assertType("0|0.0|''|'0'|array{}|false|null", $mixed);
	}
	if ($mixed == 1) {
	assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed);
	}
	if ($mixed == 0) {
	assertType("0|0.0|''|'0'|array{}|false|null", $mixed);
	}

but I think its a unrelated problem for a followup?

[staabm]

same here, could be

Suggested change

	assertType("mixed", $mixed);
	assertType("0|0.0|''|'0'|array{}|false|null", $mixed);

[ondrejmirtes]

Hi, I'd like you to know that I welcome this effort, but I'm unlikely to merge it, because this area is more complex than this PR admits or implements. For example $mixed == 'backend' should not result in 'backend' but $mixed can also be true. And there are different strings that would be equal to false. This PR does not address that at all.

You've already tried a similar problem once (#2216) but didn't finish it.

[ondrejmirtes]

Also the behaviour is PHP version-dependent: https://3v4l.org/rij8A

[staabm]

thanks for the input.

what do you think about turning the assertions into (for all truethy-constant comparisons) ?

if ($mixed == 'backend') {
			assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed);
}

I feel we can't really get much better then do some blacklisting in this conditions for now?

[ondrejmirtes]

I'm for doing as precise types as possible, but it'd be a substantial effort.

[staabm]

and would you be open to do it in small PRs, like e.g. doing "mixed vs. constant-type equal compare narrowing" in a single PR?
(or any other small step you are "ok" with would work for me - so I don't need to build up a multi month effort in a single PR)

[ondrejmirtes]

We need to design a new method on Type for this. For example if you compare non-falsy-string against a different type, it tells us something about both left and right type if the comparison is true and something else about both types if the comparison is false.

The information from this method can be also used to figure out "is this always true or always false" comparison, similarly to what ImpossibleCheckTypeHelper figures out from the TypeSpecifier output.

Once we get this design right and have a proof of concept for a small number of Types, we can merge that and then implement the method in other types in smaller chunks, making the analysis more precise gradually.

[staabm]

my thinking is

example: non-falsey-string == int

-> take all candidate values out of the comparison table for the involved types

non-falsey-string -> "1", "-1" , "php"
int -> 1, 0, -1

-> based on the comparison table .. the results for these candidates are

php7:

"1" x 1 => true, "-1" x 1 => false , "php" x 1 => false

"1" x 0 => false, "-1" x 0 => false , "php" x 0 => true

"1" x -1 => false, "-1" x -1 => true , "php" x -1 => false

php8+:

"1" x 1 => true, "-1" x 1 => false , "php" x 1 => false

"1" x 0 => false, "-1" x 0 => false , "php" x 0 => false

"1" x -1 => false, "-1" x -1 => true , "php" x -1 => false

-> this means for the IF branch

php7: non-falsey-string stays non-falsey-string
php8: non-falsey-string gets narrowed to numeric-string

php7: stays int
php8: gets narrowed to int~0

-> this means for the ELSE branch

php7: non-falsey-string stays non-falsey-string
php8: non-falsey-string stays non-falsey-string

php7: staysint
php8: stays int

---

I think this means we need a type-method like Type->looseEqual(Type $other): Type - meaning the looseEqual method returns the resulting type of the comparison.

[ondrejmirtes]

Yeah, something like that, the method also needs to have PhpVersion passed in.

[staabm]

Looking longer at the comparison table, I think it would be helpful to add a new non-numeric-string type. Do you agree?

[staabm]

Ahh just found phpstan/phpstan#10239 (comment)