False positive: Unreachable statement

[dereuromark]

Bug report

phpstan 0.12 latest

Code snippet that reproduces the problem

	protected function propertyInUse(array $tokens, int $closeTagIndex, string $variable): bool {
		/** @var string $property */
		$property = substr($variable, 1);

		$i = $closeTagIndex + 1;
		while (isset($tokens[$i])) {
			if ($tokens[$i]['code'] !== T_VARIABLE || $tokens[$i]['content'] !== '$this') {
				$i++;
				continue;
			}
			$i++;
			if ($tokens[$i]['code'] !== T_OBJECT_OPERATOR) {
				$i++;
				continue;
			}
			$i++;
			$token = $tokens[$i];
			if ($token['code'] !== T_STRING || $token['content'] !== $property) {
				$i++;
				continue;
			}
			$i++; // 520
			if ($tokens[$i]['code'] !== T_OBJECT_OPERATOR) {
				$i++;
				continue;
			}

			return true;
		}

		return false;
	}

Expected output

  520    Unreachable statement - code above always terminates.  

This is not true.

[mergeable]

This bug report is missing a link to reproduction on phpstan.org.
It will most likely be closed after manual review.

[muglug]

Here's a condensed example: https://phpstan.org/r/c65b52e1-1859-4073-9705-d838cadf59f8

[dereuromark]

It is actually

@param array<int, array> $tokens

but same result.

[ondrejmirtes]

Root issue is on line 14: https://phpstan.org/r/3bdee2b1-cfe9-44a0-8471-f41056cf851a

[dereuromark]

Well, the condensed example is not quite correct. It uses a too simplified array.
The integer is a level deeper, and as such it is not correct to assume the termination.

[muglug]

@ondrejmirtes sorry I don’t understand what you’re talking about

[muglug]

Or rather, @dereuromark I think the example does reproduce the issue (and @ondrejmirtes was just pointing out where the bug takes effect)

[dereuromark]

https://phpstan.org/r/dc254b38-d3c7-4ee1-8ab0-1a5969250e36 contains the full part of the above code without the int comparison side effect.

[dereuromark]

Hm, it looks like I have to test if the phpstan result could actually be true.
I think the issue here is valid, the extra ++ addition seems to throw the analyzer off a bit.

[muglug]

@dereuromark no, PHPstan is definitely wrong - you're incrementing $i, so whatever assertions you had previously made about $tokens[$i] are now invalid.

[muglug]

Assigning from $i to some new value $j (and asserting on that) fixes the issue: https://phpstan.org/r/0f0d9246-2603-441d-b079-e08c83bed59c

[BackEndTea]

This was (potentially) fixed by phpstan/phpstan-src#96

[muglug]

It wasn’t, though? I think the playground is synced automatically and the code still fails

[cs278]

This was (potentially) fixed by phpstan/phpstan-src#96

Nope, I actually looked at this bug whilst implementing that fix. My change fixes this code to work if you change it to use $i = $i + 1;, there is another problem with the way PHPStan deals with pre/post increment operations.

[ondrejmirtes]

The problem was that when the $i in $i++ wasn't constant string like 1 but a general string like int, no expression including $i was invalidated.

Fixed by adding invalidateExpression to NodeScopeResolver in this situation, and changing MutatingScope::invalidateExpression() to have similar logic to assignVariable(): phpstan/phpstan-src@9665e16

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.