Skip to content

Implement Type->looseCompare(type)#2216

Draft
staabm wants to merge 37 commits intophpstan:1.10.xfrom
staabm:loose-compare-new
Draft

Implement Type->looseCompare(type)#2216
staabm wants to merge 37 commits intophpstan:1.10.xfrom
staabm:loose-compare-new

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Feb 2, 2023

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Feb 2, 2023 •
edited
Loading

for now I have implemented the looseCompare methods for all types.
now I need to have a look at the failling tests and afterwards type narrowing

@staabm staabm force-pushed the loose-compare-new branch 9 times, most recently from 1337c88 to 8db02be Compare February 2, 2023 22:44
@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Feb 3, 2023

Type narrowing is a separate concern, I wouldn't do that here.

ondrejmirtes
ondrejmirtes requested changes Feb 3, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

I don't see the table of == comparison described anywhere in this PR https://www.php.net/manual/en/types.comparisons.php.

All types need to take into account being compared to any other type. For example this implementation in NullType is too naive:

return $type->isNull()->toBooleanType();

Because all of the following examples are true:

null == []
null == false
null == 0
null == ""

@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Feb 3, 2023

Also an idea for a preliminary or a future PR - NanType. See the last comment here: https://www.php.net/manual/en/function.is-nan.php

@herndlm herndlm mentioned this pull request Feb 3, 2023
Closed
@staabm staabm force-pushed the loose-compare-new branch 12 times, most recently from 0cff8e4 to 16fb924 Compare February 4, 2023 13:17
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Feb 4, 2023 •
edited
Loading

I have implemented a few basic types and added tests for them in loose-compare.php. please have a look whether this is done how you imagined it.

btw: this PR will get really big if we implement all combinations for all types here.
do you think we could split this, e.g. doing basic and simple object type in one PR, and all more specialized types in separate PRs?

thank you

@staabm staabm force-pushed the loose-compare-new branch 2 times, most recently from 3fe288d to 9aed5d1 Compare February 4, 2023 14:41
ondrejmirtes
ondrejmirtes requested changes Feb 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

I like StringType::looseCompare but otherwise it's still gonna be a lot of work 😅 And we're not even diving into type narrowing now. I think it's obvious why I was avoiding this topic until now 😂

staabm and others added 27 commits February 12, 2023 08:18
@staabm
Restore src/Type/Accessory/HasOffsetValueType.php
b85344c
@staabm
Restore src/Type/Accessory/NonEmptyArrayType.php
cb6d2e0
@staabm
Restore src/Type/Accessory/OversizedArrayType.php
e342edf
@staabm
Fix
5492b86
@staabm
Update InitializerExprTypeResolver.php
e7654a6
@staabm
test never
ef65e67
@staabm
loose array tests
adf88a8
@staabm
Update loose-comparisons.php
00e1f1b
@staabm
fix
7b1f9bb
@staabm
Restore src/Type/IterableType.php
295895f
@staabm
Update loose-comparisons.php
4e6c8d7
@staabm
Restore src/Type/ResourceType.php
6d8f55f
@staabm
Update loose-comparisons.php
f63648d
@staabm
fix
621c385
@staabm
fix
d3e3088
@staabm
Review fixes
2177d57
@staabm
list type
03829b9
@staabm
non falsey string
24675fc
@staabm
numeric strings
0061751
@staabm
add class-string type
03630c1
@staabm
fix deprecations
be1cdc1
@staabm
compare class-strings to non-class-strings
b1eb301
@staabm
fix falsy union types
63edc44
@staabm
fix enums
8843bbe
@staabm
fix build
d884a4a
@staabm
fix
83a87f9
@staabm
fix php7 expectations
018d83e
@ondrejmirtes ondrejmirtes mentioned this pull request Jan 25, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

+1 more reviewer

@herndlm herndlm herndlm left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

implement loose comparison semantics

3 participants

@staabm @ondrejmirtes @herndlm