Skip to content

Commit c36f45d

Browse files
ondrejmirtesondrejmirtes
committed
PHPStan 1.10 Comes With a Lie Detector
1 parent 71b28a6 commit c36f45d

File tree

1 file changed

+183
-0
lines changed

1 file changed

+183
-0
lines changed

‎website/src/_posts/phpstan-1-10-comes-with-lie-detector.md

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
---
2+
title: "PHPStan 1.10 Comes With a Lie Detector"
3+
date: 2023-02-21
4+
tags: releases
5+
---
6+
7+
I've been looking forward to implementing and releasing the ideas present in [PHPStan 1.10](https://github.com/phpstan/phpstan/releases/tag/1.10.0) for a long time.
8+
9+
Validate inline PHPDoc `@var` tag type
10+
----------------
11+
12+
<blockquote class="twitter-tweet" data-dnt="true"><p lang="en" dir="ltr">My personal mission after PHPStan 1.0 is to eradicate inline @​var tags from existence. Developers reach for it as an immediate remedy for their problems but it&#39;s the worst solution ever.<br><br>With @​var tags you&#39;re giving up all the type safety static analysis offers. <a href="https://t.co/WvRvFooce4">pic.twitter.com/WvRvFooce4</a></p>&mdash; Ondřej Mirtes (@OndrejMirtes) <a href="https://twitter.com/OndrejMirtes/status/1458020442258739206?ref_src=twsrc%5Etfw">November 9, 2021</a></blockquote> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
13+
14+
There are multiple problems with inline `@var` PHPDoc tag. PHP developers use it for two main reasons:
15+
16+
* To fix wrong 3rd party PHPDocs. A dependency [^not-use-sa] might have `@return string` in a PHPDoc but in reality can return `null` as well.
17+
* 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`.
18+
19+
[^not-use-sa]: That probably doesn't use static analysis.
20+
21+
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.
22+
23+
With the latest release and [bleeding edge](https://phpstan.org/blog/what-is-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:
24+
25+
```php
26+
function doFoo(): string
27+
{
28+
// ...
29+
}
30+
31+
/** @var string|null $a */
32+
$a = doFoo();
33+
34+
// PHPDoc tag @var with type string|null is not subtype of native type string.
35+
```
36+
37+
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.
38+
39+
By default PHPStan isn't going to report anything about the following code:
40+
41+
```php
42+
/** @return string */
43+
function doFoo()
44+
{
45+
// ...
46+
}
47+
48+
/** @var string|null $a */
49+
$a = doFoo();
50+
```
51+
52+
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`](/config-reference#reportwrongphpdoctypeinvartag), or install [phpstan-strict-rules](https://github.com/phpstan/phpstan-strict-rules).
53+
54+
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](/writing-php-code/phpdoc-types#conditional-return-types), [`@phpstan-assert`](/writing-php-code/narrowing-types#custom-type-checking-functions-and-methods), [generics](/blog/generics-in-php-using-phpdocs), [stub files](/user-guide/stub-files) for overriding 3rd party PHPDocs, or [dynamic return type extensions](/developing-extensions/dynamic-return-type-extensions).
55+
56+
Encourage handling of all enum cases
57+
----------------
58+
59+
What's wrong with the following example?
60+
61+
```php
62+
enum Foo
63+
{
64+
case ONE;
65+
case TWO;
66+
67+
public function getLabel(): string
68+
{
69+
return match ($this) {
70+
self::ONE => 'One',
71+
self::TWO => 'Two',
72+
default => throw new \Exception('Unexpected case'),
73+
};
74+
}
75+
}
76+
```
77+
78+
PHPStan 1.9 didn't complain about anything in that code. But it has problems:
79+
80+
* The `default` arm isn't really needed.
81+
* 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.
82+
83+
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.
84+
85+
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.
86+
87+
88+
Changes to "always true" expressions and unreachable code
89+
----------------
90+
91+
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.
92+
93+
```php
94+
function doFoo(\Exception $o): void
95+
{
96+
// not reported
97+
if ($o instanceof \Exception) {
98+
// code inside always executed
99+
}
100+
}
101+
```
102+
103+
If there was an `else` branch involved, PHPStan would report:
104+
105+
```php
106+
function doFoo(\Exception $o): void {
107+
if ($o instanceof \Exception) {
108+
} else {
109+
// dead code here
110+
// reports:
111+
// Else branch is unreachable because previous condition is always true.
112+
}
113+
}
114+
```
115+
116+
You could turn on a few [specific options](https://github.com/phpstan/phpstan-strict-rules/blob/66b378f5b242130908b8a2222bf8110f14f4375a/rules.neon#L4-L7), or install [phpstan-strict-rules](https://github.com/phpstan/phpstan-strict-rules), to also have the `instanceof` reported as "always true".
117+
118+
I chose this way because I didn't want to discourage writing "safe" code like this, because some developers prefer it:
119+
120+
```php
121+
// $foo is One|Two
122+
if ($foo instanceof One) {
123+
124+
} elseif ($foo instanceof Two) {
125+
// PHPStan reports "instanceof always true", wants you to write "else {" instead
126+
}
127+
```
128+
129+
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.
130+
131+
With [bleeding edge](https://phpstan.org/blog/what-is-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`](/config-reference#reportalwaystrueinlastcondition).
132+
133+
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](https://phpstan.org/blog/what-is-bleeding-edge).
134+
135+
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:
136+
137+
```php
138+
enum Foo
139+
{
140+
case ONE;
141+
case TWO;
142+
143+
public function getLabel(): string
144+
{
145+
return match ($this) {
146+
self::ONE => 'One',
147+
self::TWO => 'Two',
148+
default => throw new \Exception('Unexpected case'),
149+
};
150+
}
151+
}
152+
```
153+
154+
```diff
155+
-13 Match arm is unreachable because previous comparison is always true.
156+
+12 Match arm comparison between $this(aaa\Foo)&aaa\Foo::TWO and aaa\Foo::TWO is always true.
157+
+ 💡 Remove remaining cases below this one and this error will disappear too.
158+
```
159+
160+
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](https://phpstan.org/r/1b9cf4e1-5c2a-4e2f-b56d-b0846a303bd5) 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.
161+
162+
163+
Why my type isn't accepted here?
164+
----------------
165+
166+
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.
167+
168+
PHPStan 1.10 includes helpful contextual tips in these less intuitive scenarios. My favourites are:
169+
170+
* [About `@template-covariant`](https://phpstan.org/r/61cfbb65-1a04-471a-a5c5-d61f0540ae1d)
171+
* [About callable parameter contravariance](https://phpstan.org/r/24a23b74-af27-4443-986c-04af61427d50)
172+
* [About complex array shapes](https://phpstan.org/r/fed1c275-46d0-434f-b9c4-3212f4df6d1c)
173+
174+
175+
Deprecation of `instanceof *Type`
176+
----------------
177+
178+
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?](/blog/why-is-instanceof-type-wrong-and-getting-deprecated)
179+
180+
181+
---
182+
183+
Do you like PHPStan and use it every day? [**Consider supporting further development of PHPStan on GitHub Sponsors**](https://github.com/sponsors/ondrejmirtes/). I’d really appreciate it!

0 commit comments

Comments
 (0)