Skip to content

Support PHP8's Union and Mixed types#495

Merged
ciaranmcnulty merged 15 commits intophpspec:masterfrom
ciaranmcnulty:union-types
Sep 26, 2020
Merged

Support PHP8's Union and Mixed types#495
ciaranmcnulty merged 15 commits intophpspec:masterfrom
ciaranmcnulty:union-types

Conversation

@ciaranmcnulty
Copy link
Member

@ciaranmcnulty ciaranmcnulty commented Aug 12, 2020

Resolves #482

This pushes the types logic from the ArgumentNode and MethodNode (and the TypeHintReference) down into smaller value objects for ArgumentTypeNode and ReturnTypeNodes, and then expands it to support Unions

@ciaranmcnulty ciaranmcnulty marked this pull request as draft August 12, 2020 14:03
@ciaranmcnulty ciaranmcnulty changed the title Support PHP8's Union types (#482) Support PHP8's Union types Aug 12, 2020
@ciaranmcnulty ciaranmcnulty marked this pull request as ready for review September 16, 2020 14:25
@ciaranmcnulty
Copy link
Member Author

I believe this is now feature-complete

@ciaranmcnulty ciaranmcnulty force-pushed the union-types branch 2 times, most recently from d2ef7ef to 7d55492 Compare September 18, 2020 13:53
@ciaranmcnulty
Copy link
Member Author

With the recent CI changes this is now green - once any review comments are addressed it can be merged and then we'll be able to update composer.json for PHP8 I think?

@dereuromark
Copy link

dereuromark commented Sep 18, 2020

I ran into the same issue today with https://travis-ci.org/github/milesj/decoda/jobs/728420774 and PHP8 not being testable right now.

//EDIT: using --ignore-platform-reqs works as a workaround for now.

@ciaranmcnulty ciaranmcnulty changed the title Support PHP8's Union types Support PHP8's Union and Mixed types Sep 23, 2020
@ciaranmcnulty
Copy link
Member Author

I realised I wasn't providing an appropriate default promise for union types so have added it

$types,
function(string $type) {
if(class_exists($type) || interface_exists($type)) {
static function(string $type1, string $type2) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we:

  1. Prefer to return a scalar or object promise rather than a null (existing behaviour)
  2. Prefer to return an object over a scalar (on the basis that the typical Obj|false case is similar to the above)
  3. If there are more than one object type in a union, or no objects and more than one scalar, we are just picking one of them essentially at random

3 here is maybe problematic, or at least counter-intuitive. We could sort by alphabetical order or something, but it doesn't seem to make sense. We could in theory use the order of the union except that ReflectionUnionType::getTypes 'has no guaranteed order'

@ciaranmcnulty ciaranmcnulty merged commit 7cb9cd4 into phpspec:master Sep 26, 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.

Cannot double a method with union type

4 participants