Skip to content

object type: accomodate to properly resolve template type bound#85

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
hrach:object_template_type
Jan 11, 2020
Merged

object type: accomodate to properly resolve template type bound#85
ondrejmirtes merged 1 commit intophpstan:masterfrom
hrach:object_template_type

Conversation

@hrach
Copy link
Copy Markdown
Contributor

@hrach hrach commented Jan 4, 2020

@hrach hrach force-pushed the object_template_type branch 7 times, most recently from 2450ce9 to 7ab804a Compare January 5, 2020 20:46
@hrach
Copy link
Copy Markdown
Contributor Author

hrach commented Jan 5, 2020

Finally. I was missguided about hundred times. Wrong fixes, fixes that broken something elsewhere later, etc. etc. Also, another (major) codechange will arrive in another PR... Please take a look, sometimes it's a magic for me.

@ondrejmirtes
Copy link
Copy Markdown
Member

Yeah, I like this much better 👍 Although the if ($type instanceof TemplateType) as first thing in isSuperTypeOf implementation feels dirty. Why instanceof CompoundType with isSubTypeOf call later doesn't work?

@hrach hrach force-pushed the object_template_type branch from 7ab804a to 83b0857 Compare January 5, 2020 21:20
@hrach
Copy link
Copy Markdown
Contributor Author

hrach commented Jan 5, 2020

Why instanceof CompoundType with isSubTypeOf call later doesn't work?

Because I didn't know what is it exactly :D Ok, I've tried to rework it. Actually, many interfaces and theirs methods would benefit of documenting their meaning.

@ondrejmirtes
Copy link
Copy Markdown
Member

I'd like the number of if ($something instanceof SomeType) to go down, especially in Type::isSuperTypeOf implementations. Because with them, adding a new type becomes exponentially more work...

I'm really grateful for your contribution :) I'd say that after 22 hours, you're an expert now in this area :D

@ondrejmirtes
Copy link
Copy Markdown
Member

Look at CompoundType implementations - it's a hack used to when accepts or isSuperTypeOf needs special handling for some types... Since TemplateType is one of them, it should work without an extra if...

@ondrejmirtes
Copy link
Copy Markdown
Member

Oh, you’ve already changed the code. Doesn’t something in TemplateObjectWithoutClassType also need to change? (I’m on mobile.)

@hrach
Copy link
Copy Markdown
Contributor Author

hrach commented Jan 6, 2020

Doesn’t something in TemplateObjectWithoutClassType also need to change? (I’m on mobile.)

Need investigation, obviously we are missing more tests, changing it doesn't fix/break anything.

@ondrejmirtes ondrejmirtes merged commit 65cd6cb into phpstan:master Jan 11, 2020
@ondrejmirtes
Copy link
Copy Markdown
Member

Works great, thank you! Not sure what's going on in TemplateObjectWithoutClassType and why it doesn't need the changes...

@hrach hrach deleted the object_template_type branch February 12, 2020 22:58
@hrach hrach mentioned this pull request May 17, 2020
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.

(new (class-string<T : U>)) !instanceOf U

2 participants