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

Either allow literal ints as shape keys, or require class constant shape keys to be strings #6035

Open
fredemmott opened this issue Aug 18, 2015 · 4 comments
Labels

Comments

@fredemmott
Copy link
Contributor

class MyClass {
  const int FOO = 123;
}

type MyShape = shape(
  MyClass::FOO => string, // this is fine
  123 => string, // expected string literal or class constant
}

Discovered while looking into why Shapes::toArray() returns array<arraykey, mixed> instead of array<string,mixed>

@JoelMarcey
Copy link
Contributor

"returns array instead of array" ???

@Orvid
Copy link
Contributor

Orvid commented Aug 19, 2015

It's there, but apparently is detected as part of the markup. What he actually said is array<string,mixed> not array<arraykey,mixed>, the markup is just hiding some of it.

@lexidor
Copy link
Collaborator

lexidor commented May 24, 2020

The reason why this rule exists (I believe) is because shape()s were backed by PHP arrays in 2015.
PHP arrays performed integral type coercion on keys.
['0' => 'zero'] would become [0 => 'zero'].
The typesystem could not model shapes like this, because it wouldn't know about this runtime "helpful" behavior.

type MyShape = shape(
  '0' => string,
  0 => int
);

This shape is impossible to create in hhvm versions back shapes with PHP arrays.

Since hhvm 4.2.0 PHP arrays (and darrays, the backing type behind shapes), nolonger perform integral key coercion.
However the typechecker still shields you from this dead runtime behavior.

type MyShape = shape(
    '1' => string,
);
Parsing[1002] Shape field name must not be an int-like string (i.e. "123")
   --> file.hack
 11 |     '1' => string,
    |     ^^^

1 error found.

The blogpost for hhvm 3.28.3 states this directly.
Because shapes are darrays under the hood, we are still disallowing user-defined shapes to define integer keys due to intish string array key coercion.

I think we can safely allow int keys in shapes now.

@fredemmott
Copy link
Contributor Author

Yeah, they’re used in regex matches without issues, but aren’t permitted in user defined types.

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

4 participants