Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added error message to impossible instanceof rule to warn when comparing a trait. #1570

Merged

Conversation

mad-briller
Copy link
Contributor

@mad-briller mad-briller marked this pull request as draft July 30, 2022 12:19
@mad-briller mad-briller force-pushed the add.impossible-instanceof-trait branch 2 times, most recently from b11bbb2 to 6885b96 Compare July 30, 2022 12:28
@mad-briller mad-briller force-pushed the add.impossible-instanceof-trait branch from 6885b96 to 6c45692 Compare July 30, 2022 12:28
@@ -418,6 +419,16 @@ public function dataIsSuperTypeOf(): array
new ObjectType(ExtendsThrowable::class),
TrinaryLogic::createMaybe(),
],
59 => [
new ObjectType(DateTime::class),
new ObjectType(ConstantNumericComparisonTypeTrait::class),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure php has no built-in traits so i used one that phpstan has, i feel like theres a better way to do this though

@mad-briller mad-briller marked this pull request as ready for review July 30, 2022 12:32
@ondrejmirtes
Copy link
Member

@mad-briller
Copy link
Contributor Author

i think either location works, though i think "existing class" is a bit more indirect than "impossible instanceof" when it comes to comparing a trait in instanceof as a trait is not a class
happy to move if it you think the other location is better!

@ondrejmirtes
Copy link
Member

On the other hand there's already ReflectionProvider in the "existing class" rule, and to me it makes more sense there because trait is not an existing class 😊 I just forgot that rule exists, sorry 😊

@mad-briller
Copy link
Contributor Author

no worries mate, i've moved the check to the ExistingClassInInstanceOf rule

@ondrejmirtes ondrejmirtes merged commit d0c048c into phpstan:1.8.x Jul 30, 2022
@ondrejmirtes
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants