Raise PHPStan to level 6 #837
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtojnar
force-pushed
the
phpstan-3
branch
2 times, most recently
from
9b23542
to
46c813b
Compare
jtojnar
force-pushed
the
phpstan-3
branch
2 times, most recently
from
cb17a85
to
3e4edc6
Compare
|
sorry I've been away, it this ok to merge or do we need to look at other pr's first? |
Necessary for PHPStan level 3.
Mostly include false in return types when needed. Necessary for PHPStan level 3.
Looks like it could potentially return true since it has been split out of init in ce9ac85 Not sure if it can happen in practice, possibly when cache expires? Necessary for PHPStan level 3.
The headers returned from `curl_exec` are strings, whereas `File::$headers` expects an array. Necessary for PHPStan level 3.
According to type annotations in PHPDoc. Necessary for PHPStan level 3.
Necessary for PHPStan level 4.
It can be set to null by `Locator::find`. Necessary for PHPStan level 4.
- `explode` with non-empty separator will return a non-empty array, remove the redundant `if`. - Do not reuse `$duration_parent` variable name, it has different type. Necessary for PHPStan level 4.
instanceof will return false on non-object types.
jtojnar
force-pushed
the
phpstan-3
branch
2 times, most recently
from
551f102
to
e60fc48
Compare
Explicitly unroll since PHPStan does not support recursive array types. Four levels should be good enough for most domains in practice. Necessary for PHPStan level 4.
We often have checks that throw `InvalidArgumentException`
when value of an argument does not match the type specified by PHPDoc
to ensure consumers do not break preconditions.
As a result, PHPStan level 4 would complain:
Strict comparison using !== between 'GET' and 'GET' will always evaluate to false.
Result of && is always false.
We could suppress it by setting `treatPhpDocTypesAsCertain: false` in `phpstan.neon`
but that would essentially disable static type checking for all internal
uses of the API as well, significantly decreasing our level of confidence.
Let’s ignore just the specific places where it causes issues.
It would return `false` on PHP < 8.0 when using out-of-range indices. Necessary for PHPStan level 4.
Assignment of non-empty string will always be truthy. Necessary for PHPStan level 4.
The check was here since the method was introduced 01eb2c5 As far as I can tell, `File` does not really parse headers any further, so `$headers['link']` can only be a string, if it exists. Necessary for PHPStan level 4.
It contained a reference to OldTests class, which was removed in 22b8eb0 Necessary for PHPStan level 5.
Necessary for PHPStan level 5.
`curl_version()` only returned string in PHP 4: https://web.archive.org/web/20051214063609/http://www.php.net/manual/en/function.curl-version.php Necessary for PHPStan level 5.
As per PHP docs, the XML functions also accept just a method name as a string,
even though it is not actually a callable:
Note: Instead of a function name, an array containing an object reference and a method name can also be supplied.
PHPStan level 5 will complain though:
Parameter simplepie#2 $handler of function xml_set_character_data_handler expects callable(): mixed, 'cdata' given.
Let’s use proper callables.
In psr/simple-cache < 2.0.0, which is installed on PHP 7.4 or lower,
the `Psr\SimpleCache\InvalidArgumentException` interface does not extend `Throwable`.
But the current versions of PHPUnit require subtypes of `Throwable` to be given to `expectException()` method.
As a result, PHPStan will complain when we try to expect the non-`Throwable`-inheriting interface:
Parameter simplepie#1 $exception of method PHPUnit\Framework\TestCase::expectException() expects class-string<Throwable>, string given.
This is in preparation for PHPStan level 6, which will complain when “method has no return type specified”. Mostly done using Rector’s `AddVoidReturnTypeWhereNoReturnRector` and `SetList::TYPE_DECLARATION` with some minor nudges.
These became unnecessary in the previous commit.
There are not used by anything and prevent PHPStan from recognizing documentation comments, resulting in complaints about missing type annotations.
So that we can keep property annotations simple.
It is expected to be a nested array so exploding does not make sense. This has been broken since it was introduced in 8c375c2. Let’s also remove the checks that can be addressed by a type hint.
The fourth argument (`$max_checked_feeds`) is supposed to be `int`.
These were missed in d894521
In preparation for PHPStan level 6
Follow up to 64c32f7.
|
@mblaney I have fixed the issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.