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

Shapes::keyExists should refine optional shape fields #8012

Closed
eryi opened this issue Sep 29, 2017 · 6 comments
Closed

Shapes::keyExists should refine optional shape fields #8012

eryi opened this issue Sep 29, 2017 · 6 comments
Labels

Comments

@eryi
Copy link

eryi commented Sep 29, 2017

HHVM Version

3.22.0 with enable_experimental_tc_features = optional_shape_field

Standalone code, or other way to reproduce the problem

<?hh //strict

function myfunc(shape(?'optional' => mixed) $var): void {
  if (Shapes::keyExists($var, 'optional')) {
    print $var['optional'];
  } else {
    print 'optional field does not exist';
  }
}

Expected result

hh_client - no errors!

Actual result

Invalid index operation: 'optional' is marked as an optional shape field. It may not be present in the shape. Use Shapes::idx instead. (Typing[4165])

As it is, dealing with optional fields would require at least 1 Shapes::keyExists and 1 Shapes::idx. This is really really verbose.

Even better solution

Allow the php error control operator (@) to suppress type errors in addition to runtime errors. This has the additional advantage of allowing me to null coalesce optional fields.

@fredemmott
Copy link
Contributor

The PHP error control operator is not part of hack, and we don't plan on adding it. See https://docs.hhvm.com/hack/typechecker/special for ways to suppress this.

@michaeltingley
Copy link

Seems like a reasonable request, I'll see what we can do.

@dlreeves
Copy link
Contributor

dlreeves commented Oct 4, 2017

We plan on making changes to Hack that will better define when and how type refinement will be performed. We hadn't considered this case, but we will take this use case in mind for the change. I rather focus our attention on the future, especially since we hope to get the feature out within the next 3-6 months.

If there was a pull request that made the change I would consider merging it, but not something anyone on the team will work on at the moment.

@ryangreenberg
Copy link

I think this issue can be closed with the release of 3.27:

In 3.27.1 Shapes::keyExist() now refines the type when used as a condition

@eryi
Copy link
Author

eryi commented Jul 29, 2018

For some reason, this doesn't work with invariant. Is this expected?

<?hh //strict

function myfunc(shape(?'optional' => string) $var): void {
  \invariant(Shapes::keyExists($var, 'optional'), '');

  // Invalid index operation: 'optional' is marked as an optional shape field....
  print $var['optional'];
}

function myfunc2(shape(?'optional' => string) $var): void {
  if (!Shapes::keyExists($var, 'optional')) throw new Exception();

  // OK
  print $var['optional'];
}

hh_client

Invalid index operation: 'optional' is marked as an optional shape field. It may not be present in the shape. Use the `??` operator instead. (Typing[4165])

hhvm --version

HipHop VM 3.27.1 (rel)
Compiler: 1532025848_857759529
Repo schema: 6f2a65dfa3a360d8c159fe76a8bee0e26ba4e45a

@eryi
Copy link
Author

eryi commented Jul 29, 2018

This might be asking too much of the type checker but it would be nice if this worked on more than 1 level.

<?hh //strict
function myfunc(shape(?'optional' => shape(?'optional' => string)) $var): void {
  if (!Shapes::keyExists($var, 'optional')) throw new Exception();
  if (!Shapes::keyExists($var['optional'], 'optional')) throw new Exception();

  // Invalid index operation: 'optional' is marked as an optional shape field....
  print $var['optional']['optional'];
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants