fix union/intersect involving enum case

[schlndh]

Fixes https://phpstan.org/r/2bf03c1e-d38f-4d1d-9d67-0b4ace5e1629

IMO the real issue is that EnumCaseObjectType should not be SubtractableType (i.e. there is nothing to subtract from it, other than itself which should then be NeverType). Unfortunately, it inherits SubtractableType implementation from ObjectType. I checked the other types and the only other suspicious "subtractable" type I found is ErrorType (it doesn't subtract anything).

I'm not sure what the best way to fix it is. Phpstorm found just 15 usages of instanceof SubtractableType. So a quick and dirty fix along the lines I did could probably be feasible (at least in the short term). I was thinking e.g. adding isReallySubtractable(): bool to SubtractableType, or having isSubtractableType(Type): bool somewhere, ... For now I did just the minimal changes necessary to make the tests pass, but the other usages may also be causing bugs.

OFC a proper fix would be to get rid of the inheritance. Since PHPStorm reports instanceof ...Type as errors, I assume that is the long-term plan. But given that ObjectType is >1600 lines, I don't think that I have enough experience to tackle that.

Please let me know what you think. If you say that I should just wait until the inheritance is removed then that's fine with me.

[ondrejmirtes]

The SubtractableType methods can be basically made no-op in EnumCaseObjectType. Looking at the logic you're iffing-out:

			$subtractedType = $type->getSubtractedType() === null
				? $subtractedType
				: self::union($type->getSubtractedType(), $subtractedType);
			if ($subtractedType instanceof NeverType) {
				$subtractedType = null;
			}
			return $type->changeSubtractedType($subtractedType);

If EnumCaseObjectType::getSubtractedType() returns null and changeSubtractedType returns $this then the fix would be equivalent, right?

[schlndh]

@ondrejmirtes Interesting idea, but I didn't manage to get it working (infinite recursion).

Looking at unionWithSubtractedType(EnumCaseObjectType, whatever), I need

$subtractedType = $type->getSubtractedType() === null
	? $subtractedType
	: self::union($type->getSubtractedType(), $subtractedType);
if ($subtractedType instanceof NeverType) {
	$subtractedType = null;
}

return $type->changeSubtractedType($subtractedType);

to be equivalent to return self::remove($type, $subtractedType);. Which is possible to do this way (though it could easily lead to infinite loop with future changes):

public function changeSubtractedType(?Type $subtractedType): Type
{
	if ($subtractedType === null) {
		return $this;
	}

	return TypeCombinator::remove($this, $subtractedType);
}

And then there's intersectWithSubtractedType(whatever, EnumCaseObjectType). Here I need

$subtractedType = $b->getSubtractedType();
if ($subtractedType === null) {
	return $a->getTypeWithoutSubtractedType();
}

to be equivalent to

$subtractedTypeTmp = self::intersect($a->getTypeWithoutSubtractedType(), $a->getSubtractedType());
if ($b->isSuperTypeOf($subtractedTypeTmp)->yes()) {
	return $a->getTypeWithoutSubtractedType();
}
$subtractedType = new MixedType(false, $b);

It could be done by returning new MixedType(false, $this) from getSubtractedType (+ removing getSubtractedType call from isSuperTypeOf to avoid infinite recursion). This would affect unionWithSubtractedType because now getSubtractedType would not be null anymore, but it's still equivalent. However, it also leads to infinite recursion like this (the first union is Enum~(Enum::A|Enum::B) and (Enum::A|Enum::D)):

TypeCombinator.php:441, PHPStan\Type\TypeCombinator::compareTypesInUnion()
TypeCombinator.php:329, PHPStan\Type\TypeCombinator::union()
TypeCombinator.php:546, PHPStan\Type\TypeCombinator::unionWithSubtractedType()
TypeCombinator.php:1178, PHPStan\Type\TypeCombinator::intersect()
TypeCombinator.php:612, PHPStan\Type\TypeCombinator::intersectWithSubtractedType()
TypeCombinator.php:441, PHPStan\Type\TypeCombinator::compareTypesInUnion()
TypeCombinator.php:329, PHPStan\Type\TypeCombinator::union()
TypeCombinator.php:546, PHPStan\Type\TypeCombinator::unionWithSubtractedType()
TypeCombinator.php:1178, PHPStan\Type\TypeCombinator::intersect()
TypeCombinator.php:612, PHPStan\Type\TypeCombinator::intersectWithSubtractedType()
TypeCombinator.php:441, PHPStan\Type\TypeCombinator::compareTypesInUnion()
TypeCombinator.php:329, PHPStan\Type\TypeCombinator::union()
TypeCombinator.php:546, PHPStan\Type\TypeCombinator::unionWithSubtractedType()
TypeCombinator.php:1178, PHPStan\Type\TypeCombinator::intersect()
TypeCombinator.php:1080, PHPStan\Type\TypeCombinator::intersect()
TypeCombinator.php:612, PHPStan\Type\TypeCombinator::intersectWithSubtractedType()
TypeCombinator.php:441, PHPStan\Type\TypeCombinator::compareTypesInUnion()
TypeCombinator.php:329, PHPStan\Type\TypeCombinator::union()
TypeCombinatorTest.php:2799, PHPStan\Type\TypeCombinatorTest->testUnion()
TestCase.php:1617, PHPUnit\Framework\TestCase->runTest()
TestCase.php:1223, PHPUnit\Framework\TestCase->runBare()
TestResult.php:729, PHPUnit\Framework\TestResult->run()
TestCase.php:973, PHPUnit\Framework\TestCase->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestRunner.php:651, PHPUnit\TextUI\TestRunner->run()
Command.php:146, PHPUnit\TextUI\Command->run()
Command.php:99, PHPUnit\TextUI\Command::main()
phpunit:107, {main}()

Unfortunately, I didn't find a way to neatly break the infinite recursion.

[ondrejmirtes]

Alright, another idea - instead of instanceof EnumCaseObjectType, please try Type::getFiniteTypes() === 1 (or < 2?).

1. It will work for ObjectType with a single enum case (feel free to test it).
2. It might work for some other scenarios as well.

[schlndh]

@ondrejmirtes The getFiniteTypes approach did work, but it felt a bit weird. So I decided to have another look at making the subtracting work better with EnumCaseObjectType and I found what seems to be a working solution. As a bonus it fixed a few other edge-cases as well.

[ondrejmirtes]

Thanks! Feel free to mark it as ready.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Thank you!