Regression in 5.18.0 throws RedundantCondition after comparing with === to another value of the same type

[nickvergessen]

The "problematic" code is:
https://github.com/nextcloud/spreed/blob/bb89b3f371cacd542d7aaa1961e5d65e2333e48e/lib/Service/RoomService.php#L490

ERROR: RedundantCondition - lib/Service/RoomService.php:490:7 - $newType is not int(5) has already been asserted (see https://psalm.dev/122)
		if (!in_array($newType, [Room::TYPE_GROUP, Room::TYPE_PUBLIC, Room::TYPE_ONE_TO_ONE_FORMER], true)) {


ERROR: RedundantCondition - lib/Service/RoomService.php:490:7 - $newType is not int(3) has already been asserted (see https://psalm.dev/122)
		if (!in_array($newType, [Room::TYPE_GROUP, Room::TYPE_PUBLIC, Room::TYPE_ONE_TO_ONE_FORMER], true)) {


ERROR: RedundantCondition - lib/Service/RoomService.php:490:7 - $newType is not int(2) has already been asserted (see https://psalm.dev/122)
		if (!in_array($newType, [Room::TYPE_GROUP, Room::TYPE_PUBLIC, Room::TYPE_ONE_TO_ONE_FORMER], true)) {

The problem seems to be due to:
https://github.com/nextcloud/spreed/blob/bb89b3f371cacd542d7aaa1961e5d65e2333e48e/lib/Service/RoomService.php#L474

		if ($newType === $room->getType()) {
			return true;
		}

As removing those lines fixes it.

So I changed it to use a variable:

		$oldType = $room->getType();
		if ($newType === $oldType) {
			return true;
		}

Still results in the same RedundantCondition.

However changing the order of the comparison now makes it work:

		$oldType = $room->getType();
		if ($oldType === $newType) {
			return true;
		}

The regression should be within this gap, as a75d26a (used by playground) works and b113f3e which is the 5.18.0 release reports the issue.

a75d26a...b113f3e

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/32b06be2e3

<?php

class Room {
	public const TYPE_UNKNOWN = -1;
	public const TYPE_ONE_TO_ONE = 1;
	public const TYPE_GROUP = 2;
	public const TYPE_PUBLIC = 3;
	public const TYPE_CHANGELOG = 4;
	public const TYPE_ONE_TO_ONE_FORMER = 5;
	public const TYPE_NOTE_TO_SELF = 6;
    
	/**
	 * @psalm-param Room::TYPE_* $type
	 */
	public function __construct(
		private int $type,
	) {
	}

	/**
	 * @return int
	 * @psalm-return Room::TYPE_*
	 */
	public function getType(): int {
		return $this->type;
	}

	/**
	 * @param int $type
	 * @psalm-param Room::TYPE_* $type
	 */
	public function setType(int $type): void {
		$this->type = $type;
	}
}

class RoomService {
	/**
	 * @param Room $room
	 * @param int $newType Currently it is only allowed to change between `Room::TYPE_GROUP` and `Room::TYPE_PUBLIC`
	 * @param bool $allowSwitchingOneToOne Allows additionally to change the type from `Room::TYPE_ONE_TO_ONE` to `Room::TYPE_ONE_TO_ONE_FORMER`
	 * @return bool True when the change was valid, false otherwise
	 */
	public function setType(Room $room, int $newType, bool $allowSwitchingOneToOne = false): bool {
		$oldType = $room->getType();

		if ($newType === $oldType) {
			return true;
		}

		if (!$allowSwitchingOneToOne && $oldType === Room::TYPE_ONE_TO_ONE) {
			return false;
		}

		if ($oldType === Room::TYPE_ONE_TO_ONE_FORMER) {
			return false;
		}

		if ($oldType === Room::TYPE_NOTE_TO_SELF) {
			return false;
		}

		if (!in_array($newType, [Room::TYPE_GROUP, Room::TYPE_PUBLIC, Room::TYPE_ONE_TO_ONE_FORMER], true)) {
			return false;
		}

		if ($newType === Room::TYPE_ONE_TO_ONE_FORMER && $oldType !== Room::TYPE_ONE_TO_ONE) {
			return false;
		}

		$room->setType($newType);

		if ($oldType === Room::TYPE_PUBLIC) {
			// … something
		}

		return true;
	}
}

Psalm output (using commit a75d26a):

No issues!

[kkmuffme]

Not sure, but could you check but I think this was a bug that existed previously, but wasn't reporting due to other bugs which were fixed #10481

If you got 1 commit before 955e7fe, does it work correctly?

[nickvergessen]

I bisected it now and it seems to be: 108f626

108f6267124377263ac1c99e5f22e105367b0842 is the first bad commit
commit 108f6267124377263ac1c99e5f22e105367b0842
Author: kkmuffme <11071985+kkmuffme@users.noreply.github.com>
Date:   Wed Dec 13 13:59:26 2023 +0100

    fix literal int/string comparisons only using one literal
    
    Fix https://github.com/vimeo/psalm/issues/9552

 .../Statements/Expression/AssertionFinder.php      |  9 +++++++--
 tests/TypeReconciliation/ConditionalTest.php       | 23 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

I also reverted the changes in .../Statements/Expression/AssertionFinder.php and it works on 5.x branch then again

[kkmuffme]

Ok, so the issue is that it works with literal strings, but literal int's have an issue. I won't have time to investigate this until beginning of next month, feel free to give it a try and PR a fix