Fixed '$this instanceof X will always be false' in traits by staabm · Pull Request #2045 · phpstan/phpstan-src

staabm · 2022-12-03T17:30:55Z

test fails without the fix

1) PHPStan\Rules\Classes\ImpossibleInstanceOfRuleTest::testBug3632
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'10: Instanceof between *NEVER* and Bug3632\OtherClass will always evaluate to false.
 '

src/Analyser/MutatingScope.php

staabm · 2023-01-15T15:37:59Z

I have approached the problem from a few different angles. All worked more or less.

At the end I get the feeling maybe this should be fixed at the core by e.g. treating $this just like a simple ObjectType when analysing in trait context

src/Rules/Classes/ImpossibleInstanceOfRule.php

tests/PHPStan/Rules/Classes/data/trait-instance-of.php

staabm · 2023-01-15T20:35:10Z

regarding the build: I can see the same errors in other PRs so I think these are unrelated

phpstan-bot · 2023-01-15T20:35:31Z

This pull request has been marked as ready for review.

ondrejmirtes

The underyling issues we need to fix is: https://phpstan.org/r/d8b183e1-7099-478e-af58-fdd5e8ab7d21

So it's not sufficient to try to if-else our way out of this problem in a single rule, we need to fix the typesystem here.

So something needs to happen to handling of Instanceof_ in TypeSpecifier.

staabm · 2023-01-16T09:38:16Z

So something needs to happen to handling of Instanceof_ in TypeSpecifier.

that was a great hint, thank you. I feel dumb for searching the solution in a lot of other phpstan components, but not the TypeSpecifier :)

ondrejmirtes · 2023-01-19T12:04:19Z

I decided this still doesn't work and we need a different approach. ThisType in a trait context needs to behave a bit differently than in a class. It needs to allow intersection with other classes, almost like an interface.

staabm · 2023-01-19T12:46:11Z

So the bottom line is, that we need a new type?

ondrejmirtes · 2023-01-19T12:49:22Z

Some boolean arg in ThisType would suffice. But maybe TraitThisType would work too.

tests/PHPStan/Analyser/AnalyserIntegrationTest.php

tests/PHPStan/Analyser/data/trait-instance-of.php

ondrejmirtes

I see some problems with this PR.

TypeSpecifier shouldn't be touched. Instead, MutatingScope::enterTrait should handle ThisType.
Any changes to isSuperTypeOf logic needs to be heavily covered by tests in TypeCombinatorTest - in dataUnion and dataIntersection.
There are some cases the method should return no: a) for a final class that does not use the trait. b) For an ObjectType with a trait name c) For a type that's already subtracted in ThisType. This influences how unions and intersections are gonna end up looking like.

staabm · 2023-01-31T10:18:51Z

tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

for some reason this error is not reported. I get a error though, when running phpstan on this file in isolation.

otherwise I think this looks fine. I will add new TypeCombinator Tests now

That's ExistingClassInInstanceOfRule rule, not ImpossibleInstanceOfRule rule.

phpstan-bot · 2023-01-31T12:50:55Z

This pull request has been marked as ready for review.

ondrejmirtes · 2023-01-31T13:43:22Z

src/Analyser/MutatingScope.php

-			$this->expressionTypes,
-			$this->nativeExpressionTypes,
+			array_merge($this->expressionTypes, [
+				'$this' => $thisHolder,


No need for array_merge. Simply do this before:

$expressionTypes = $this->expressionTypes; $expressionType['$this'] = $thisHolder;

And the same thing for nativeExpressionTypes.

I see. I was inspired by another place where it was done this way. fixed both in 749fbab

ondrejmirtes · 2023-01-31T13:48:08Z

tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

That's ExistingClassInInstanceOfRule rule, not ImpossibleInstanceOfRule rule.

ondrejmirtes · 2023-01-31T15:19:08Z

Thank you.

staabm · 2023-01-31T15:20:16Z

thanks for the directions. I couldn't have finished this one without your feedback <3.

staabm commented Dec 3, 2022

View reviewed changes

src/Analyser/MutatingScope.php Outdated Show resolved Hide resolved

ondrejmirtes force-pushed the 1.9.x branch from a5d90c1 to a69c7f0 Compare January 5, 2023 14:23

staabm force-pushed the trait-instance-of branch from 45f4b29 to e5ebe9d Compare January 15, 2023 10:56

staabm marked this pull request as ready for review January 15, 2023 11:56

staabm marked this pull request as draft January 15, 2023 12:55

staabm force-pushed the trait-instance-of branch from c6c3cfc to d052ca0 Compare January 15, 2023 14:33

staabm force-pushed the trait-instance-of branch from d052ca0 to d75abea Compare January 15, 2023 20:23

staabm commented Jan 15, 2023

View reviewed changes

src/Rules/Classes/ImpossibleInstanceOfRule.php Outdated Show resolved Hide resolved

staabm commented Jan 15, 2023

View reviewed changes

tests/PHPStan/Rules/Classes/data/trait-instance-of.php Outdated Show resolved Hide resolved

staabm marked this pull request as ready for review January 15, 2023 20:35

ondrejmirtes requested changes Jan 15, 2023

View reviewed changes

clxmstaab force-pushed the trait-instance-of branch 3 times, most recently from 9b059b0 to e571a81 Compare January 16, 2023 09:31

staabm force-pushed the trait-instance-of branch 2 times, most recently from 8389601 to 5e8963b Compare January 18, 2023 19:47

staabm force-pushed the trait-instance-of branch from f2180b4 to 9f41729 Compare January 21, 2023 10:43

staabm commented Jan 21, 2023

View reviewed changes

tests/PHPStan/Analyser/AnalyserIntegrationTest.php Outdated Show resolved Hide resolved

staabm force-pushed the trait-instance-of branch from 9f41729 to e8b28a3 Compare January 21, 2023 10:44

staabm commented Jan 21, 2023

View reviewed changes

tests/PHPStan/Analyser/data/trait-instance-of.php Outdated Show resolved Hide resolved

staabm force-pushed the trait-instance-of branch from e8b28a3 to 9fc0b5a Compare January 24, 2023 18:50

clxmstaab force-pushed the trait-instance-of branch from 9fc0b5a to 0e92cb2 Compare January 30, 2023 13:32

ondrejmirtes requested changes Jan 30, 2023

View reviewed changes

staabm marked this pull request as draft January 31, 2023 09:15

staabm added 3 commits January 31, 2023 10:28

Fixed '$this instanceof X will always be false' in traits

ae164cd

try fix

ad9ee68

Restore src/Analyser/TypeSpecifier.php

6159a0d

clxmstaab force-pushed the trait-instance-of branch 2 times, most recently from 041c226 to e53cd2d Compare January 31, 2023 10:09

staabm commented Jan 31, 2023

View reviewed changes

clxmstaab force-pushed the trait-instance-of branch 5 times, most recently from 695c257 to 79bf7ef Compare January 31, 2023 12:50

staabm marked this pull request as ready for review January 31, 2023 12:50

clxmstaab force-pushed the trait-instance-of branch 2 times, most recently from a2d4993 to 1c526dc Compare January 31, 2023 12:59

incorporate review feedback

005b6f3

clxmstaab force-pushed the trait-instance-of branch from 1c526dc to 005b6f3 Compare January 31, 2023 13:06

ondrejmirtes requested changes Jan 31, 2023

View reviewed changes

Fix

749fbab

ondrejmirtes merged commit e1c8380 into phpstan:1.9.x Jan 31, 2023

ondrejmirtes mentioned this pull request Jan 31, 2023

Instanceof between *NEVER* and XXX will always evaluate to false phpstan/phpstan#3632

Closed

staabm deleted the trait-instance-of branch January 31, 2023 15:19

staabm mentioned this pull request Feb 14, 2023

False positive: Method invoked with 1 parameter, 2 required phpstan/phpstan#8888

Closed

Conversation

staabm commented Dec 3, 2022

Uh oh!

Uh oh!

staabm commented Jan 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staabm commented Jan 15, 2023

Uh oh!

phpstan-bot commented Jan 15, 2023

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

staabm commented Jan 16, 2023

Uh oh!

ondrejmirtes commented Jan 19, 2023

Uh oh!

staabm commented Jan 19, 2023

Uh oh!

ondrejmirtes commented Jan 19, 2023

Uh oh!

Uh oh!

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

staabm Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Jan 31, 2023

Uh oh!

ondrejmirtes Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

staabm Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes commented Jan 31, 2023

Uh oh!

staabm commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

staabm commented Jan 15, 2023 •

edited

Loading

staabm Jan 31, 2023 •

edited

Loading

staabm Jan 31, 2023 •

edited

Loading