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

support for Optional[`a] #4

Merged
merged 1 commit into from Jul 5, 2018
Merged

Conversation

tobyink
Copy link
Contributor

@tobyink tobyink commented Jul 5, 2018

Adds support for:

  has foo => Optional[Int];

@zoffixznet zoffixznet merged commit bf68187 into zoffixznet:master Jul 5, 2018
@zoffixznet
Copy link
Owner

👍 Thanks!

zoffixznet added a commit that referenced this pull request Jul 5, 2018
  - [tobyink] Implement support for Optional[`a] PR #4
@zoffixznet
Copy link
Owner

zoffixznet commented Jul 29, 2018

Looks like this broke support for Maybe, like in this piece of code

I'm now getting this for it:

$ perl bin/build-project-list.pl
Must be a positive number (in $args->{"limit"}) at (eval 347) line 118
    "PositiveNum" is a subtype of "Num"
    "Num" is a subtype of "LaxNum"
    "LaxNum" is a subtype of "Str"
    "Str" is a subtype of "Value"
    "Value" is a subtype of "Defined"
    Undef did not pass type constraint "Defined" (in $args->{"limit"})
    "Defined" is defined as: (defined($_))

Any ideas for how to fix that?

@tobyink
Copy link
Contributor Author

tobyink commented Jul 31, 2018

Hmmm, try changing this:

if ($mew_type and $mew_type->is_parameterized and $mew_type->parent == Types::Standard::Optional()) {

to this:

if ($mew_type and $mew_type->is_parameterized and $mew_type->parent->strictly_equals(Types::Standard::Optional)) {

And this:

elsif ($mew_type and $mew_type == Types::Standard::Optional()) {

To this:

elsif ($mew_type and $mew_type->strictly_equals(Types::Standard::Optional)) {

The overloaded == uses a looser version of the equality check which thinks Maybe and Optional are the same (because they do actually inline to the same thing in most cases).

@zoffixznet
Copy link
Owner

👍 That did the trick. Thanks!

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.

None yet

2 participants