Menu

PHPStan 1.10 Comes With a Lie Detector

February 21, 2023 · 9 min read

I’ve been looking forward to implementing and releasing the ideas present in PHPStan 1.10 for a long time.

Validate inline PHPDoc @var tag type #

There are multiple problems with inline @var PHPDoc tag. PHP developers use it for two main reasons:

  • To fix wrong 3rd party PHPDocs. A dependency [1] might have @return string in a PHPDoc but in reality can return null as well.
  • To narrow down the returned type. When a function returns string but we know that in this case it can only return non-empty-string.

By looking at the analysed code we can’t really tell which scenario it is. That’s why PHPStan always trusted the type in @var and didn’t report any possible mistakes. Obviously that’s dangerous because the type in @var can get out of sync and be wrong really easily. But I came up with an idea what we could report without any false positives, keeping existing use-cases in mind.

With the latest release and bleeding edge enabled, PHPStan validates the inline @var tag type against the native type of the assigned expression. It finds the lies spread around in @var tags:

function doFoo(): string
{
    // ...
}

/** @var string|null $a */
$a = doFoo();

// PHPDoc tag @var with type string|null is not subtype of native type string.

It doesn’t make sense to allow string|null, because the type can never be null. PHPStan says “string|null is not subtype of native type string”, implying that only subtypes are allowed. Subtype is the same type or narrower, meaning that string or non-empty-string would be okay.

By default PHPStan isn’t going to report anything about the following code:

/** @return string */
function doFoo()
{
    // ...
}

/** @var string|null $a */
$a = doFoo();

Because the @return PHPDoc might be wrong and that’s what the @var tag might be trying to fix. If you want this scenario to be reported too, enable reportWrongPhpDocTypeInVarTag, or install phpstan-strict-rules.

I’d like the PHP community to use inline @var tags less and less over time. There are many great alternatives that promote good practices and code deduplication: Conditional return types, @phpstan-assert, generics, stub files for overriding 3rd party PHPDocs, or dynamic return type extensions.

Encourage handling of all enum cases #

What’s wrong with the following example?

enum Foo
{
	case ONE;
	case TWO;

	public function getLabel(): string
	{
		return match ($this) {
			self::ONE => 'One',
			self::TWO => 'Two',
			default => throw new \Exception('Unexpected case'),
		};
	}
}

PHPStan 1.9 didn’t complain about anything in that code. But it has problems:

  • The default arm isn’t really needed.
  • When we add case THREE in the enum, PHPStan will not tell us about an unhandled case, but in runtime the application will throw exceptions in our face.

PHPStan 1.10 reports “Match arm is unreachable because previous comparison is always true.” for the line with the default case. It encourages removing the default case, which solves both problems at once.

When we add case THREE, PHPStan will now report: “Match expression does not handle remaining value: Foo::THREE”. Which would not happen if the default case was still there.

Changes to “always true” expressions and unreachable code #

There’s a few more related changes spawned from the previous section. For a long time PHPStan reported inconsistently “always true” and “always false” conditions. But there was some logic to it. I didn’t want you to have dead code.

function doFoo(\Exception $o): void
{
	// not reported
	if ($o instanceof \Exception) {
		// code inside always executed
	}
}

If there was an else branch involved, PHPStan would report:

function doFoo(\Exception $o): void {
	if ($o instanceof \Exception) {
	} else {
		// dead code here
		// reports:
		// Else branch is unreachable because previous condition is always true.
	}
}

You could turn on a few specific options, or install phpstan-strict-rules, to also have the instanceof reported as “always true”.

I chose this way because I didn’t want to discourage writing “safe” code like this, because some developers prefer it:

// $foo is One|Two
if ($foo instanceof One) {

} elseif ($foo instanceof Two) {
    // PHPStan reports "instanceof always true", wants you to write "else {" instead
}

Very similar to the match example above, right? I also noticed and realized that people expect PHPStan to report “always true” out of the box more often than not, except for these examples.

With bleeding edge enabled and for everyone in the next major version, PHPStan will report “always true” by default, no extra options needed. To support the “last elseif” use-case, “always true” will not be reported for the last elseif condition and for the last arm in a match expression. You can override that with reportAlwaysTrueInLastCondition.

We no longer need unreachable branches reported at all - we’ll find out about them thanks to “always true” errors from previous branches. These rules are now completely disabled in bleeding edge.

Thanks to these changes, the error reported for the enum with the default case are different with and without bleeding edge on PHPStan 1.10:

enum Foo
{
	case ONE;
	case TWO;

	public function getLabel(): string
	{
		return match ($this) {
			self::ONE => 'One',
			self::TWO => 'Two',
			default => throw new \Exception('Unexpected case'),
		};
	}
}
-13     Match arm is unreachable because previous comparison is always true.
+12     Match arm comparison between $this(aaa\Foo)&aaa\Foo::TWO and aaa\Foo::TWO is always true.
+       💡 Remove remaining cases below this one and this error will disappear too.

PHPStan tries to be helpful and shows a tip next to the 💡 emoji on the command line. These tips are now incorporated into the playground as well. If you remove the default case, PHPStan will no longer complain about this piece of code. Until you add a new enum case.

Why my type isn’t accepted here? #

It’s not always obvious what is PHPStan complaining about. Type safety is of the most importance, it really doesn’t want to leave any gap for possible runtime errors. The developer might not know what situation it’s trying to avoid with certain checks, and might not know why they should fix it and how.

PHPStan 1.10 includes helpful contextual tips in these less intuitive scenarios. My favourites are:

Deprecation of instanceof *Type #

PHPStan 1.10 also comes with deprecations of less-than-ideal code patterns in custom rules and other extensions. I’ve written a separate article about those two weeks ago: Why Is instanceof *Type Wrong and Getting Deprecated?

Do you like PHPStan and use it every day? Consider supporting further development of PHPStan on GitHub Sponsors. I’d really appreciate it!

  1. That probably doesn’t use static analysis. ↩︎

Theme
A
Twitter Mastodon YouTube RSS
© 2016–2024 Ondřej Mirtes