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

Add support for value-of<BackedEnum> #537

Merged
merged 7 commits into from
Jul 19, 2024
Merged

Conversation

robchett
Copy link
Contributor

@robchett robchett commented May 2, 2024

No description provided.

@robchett
Copy link
Contributor Author

robchett commented May 3, 2024

Added Integration tests too, raised an issue with my original work.

Copy link
Member

@romm romm left a comment

Choose a reason for hiding this comment

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

Excellent work, thanks!

Just a few minor comments; I'll try to address these during the train trip I'm about to make. 😁

throw new ClosingBracketMissing($this->symbol());
}

if ($subType instanceof UnionType && count($subType->types()) === 1) {
Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary, a union type always has at least 2 elements.

}
}

return new UnionType(...$list);
Copy link
Member

Choose a reason for hiding this comment

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

As said earlier, a union type must have at least two elements. We must handle the case where a backed enum has only one case (in which case we can directly return the first element of the list, I guess). This would also require a specific test for it!

use RuntimeException;

/** @internal */
final class NonArrayOf extends RuntimeException implements InvalidType
Copy link
Member

Choose a reason for hiding this comment

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

This class seems not to be used.

@romm romm enabled auto-merge (squash) July 19, 2024 14:49
@romm
Copy link
Member

romm commented Jul 19, 2024

Thank you for you work @robchett! This will be available in next release.

@romm romm merged commit b1017ce into CuyZ:master Jul 19, 2024
11 checks passed
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