Skip to content

Add support for stubs to declare intersection type class properties #8751

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

Merged
merged 3 commits into from
Jul 22, 2022

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jun 11, 2022

Haven't figured out yet how to do return types and function parameter types, but this is a start.

Copy link
Member

@kocsismate kocsismate left a comment

Choose a reason for hiding this comment

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

I noticed that proper intersection type support is still needed in the below methods:

  • Type::fromString()
  • Type::toOptimizerTypeMask() and Type::toOptimizerTypeMaskForArrayValue()
  • Type::getTypeForDoc() (+ phd support should also be added)
  • Type::equals()
  • Type::toString()

@Girgias Girgias force-pushed the stubs-intersection-support branch from 37f08dc to ccc23ce Compare June 18, 2022 12:17
@Girgias
Copy link
Member Author

Girgias commented Jun 18, 2022

I noticed that proper intersection type support is still needed in the below methods:

* `Type::fromString()`

Done

* `Type::toOptimizerTypeMask()` and `Type::toOptimizerTypeMaskForArrayValue()`

Maybe I'm wrong, but this seems unapplicable because intersection types can't have scalars types?

* `Type::getTypeForDoc()` (+ phd support should also be added)

PhD support has been added a while ago: php/phd@094794e

* `Type::equals()`

I'm not sure any changes need to be made to that as the logic looks sound to me for both cases

* `Type::toString()`

Done

@@ -17,6 +17,7 @@ class _ZendTestClass implements _ZendTestInterface {
public int $intProp = 123;
public ?stdClass $classProp = null;
public stdClass|Iterator|null $classUnionProp = null;
public Traversable&Countable $classIntersectionProp;
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it's worthwhile to also add a function with intersection param and return types?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how to do the check with ZPP or is it possible to them manually?

Copy link
Member

@kocsismate kocsismate Jul 4, 2022

Choose a reason for hiding this comment

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

Yeah, I think you should rely on manual param parsing for now, but we could add a ZPP when we will have a real use-case for an internal intersection param type.

@kocsismate
Copy link
Member

Maybe I'm wrong, but this seems unapplicable because intersection types can't have scalars types?

Even though optimizer type masks support the object type, you are right in the sense that we cannot really specify the type mask further now.

The only small issue I can imagine with the current implementation (and probably it's an existing problem) is that MAY_BE_OBJECT is added multiple times to the type mask if the type includes multiple classes.

@Girgias
Copy link
Member Author

Girgias commented Jun 30, 2022

Maybe I'm wrong, but this seems unapplicable because intersection types can't have scalars types?

Even though optimizer type masks support the object type, you are right in the sense that we cannot really specify the type mask further now.

The only small issue I can imagine with the current implementation (and probably it's an existing problem) is that MAY_BE_OBJECT is added multiple times to the type mask if the type includes multiple classes.

Isn't it a bitmask? So adding it multiple times shouldn't change anything other than being somewhat redundant?

@kocsismate
Copy link
Member

Isn't it a bitmask? So adding it multiple times shouldn't change anything other than being somewhat redundant?

Yes, I was only referring to the redundancy. :)

@Girgias Girgias force-pushed the stubs-intersection-support branch from ccc23ce to 9aaa860 Compare July 22, 2022 09:50
@Girgias Girgias merged commit 4457dba into php:master Jul 22, 2022
@Girgias Girgias deleted the stubs-intersection-support branch July 22, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants