Fix several issues with nullsafe type specification#2577
Fix several issues with nullsafe type specification#2577ondrejmirtes merged 5 commits intophpstan:1.10.xfrom
Conversation
050fd24 to
162e3e3
Compare
162e3e3 to
12bc2c1
Compare
|
Yes, you got it right, there's a difference between These two methods are different on purpose, sometimes you want one and sometimes you want the other one. It's possible that the usage is wrong in some places as you already figured out :) |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Please move the tests out of tests/PHPStan/Analyser/AnalyserIntegrationTest.php. This test class is reserved for tests that crash PHPStan or cause performance problems.
Where to correctly put these tests? It depends on whether the issue creator complained about a specific error message (then the test belongs in the relevant rule test class), or if they complained about something that can be tested with assertType - then the test belongs to NodeScopeResolverTest.
It's also possible to reference the same file from a rule test class and from NodeScopeResolverTest if you'd want.
Thank you.
|
Of course this fix is hugely valuable and also satisfying given the number of issues issue-bot found it fixes :) Thank you very much. |
83a0825 to
2cd8400
Compare
|
@ondrejmirtes Ok, thanks for the feedback. I moved the tests out of the AnalyserIntegrationTest. I kept most of them both in a rule test and in the NodeScopeResolverTest to make sure that both the |
|
Thank you! |
|
6 issues fixed with a two-line-change, nice! :) |
The goal of this PR is to fix https://phpstan.org/r/c17b503c-a789-4d90-87a7-31f3436f1b68 .
These issues were also fixed:
B|nullgets narrowed to typeBwhen calling a method that returnsmixedphpstan#9293The problem is that
TypeCombinator::containsNullreturns false forMixedType(there's even a test for that and some usages definitely rely on this behavior). That seems quite strange to me and it's possible that there are other issues due to this confusing method. But I replaced only those usages for which I could come up with a test.