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

Type safety for dicts with mixed value types - and the future of json_decode #8653

Closed
jacobconley opened this issue Jan 29, 2020 · 4 comments

Comments

@jacobconley
Copy link

jacobconley commented Jan 29, 2020

Is your feature request related to a problem? Please describe.

Often, it is necessary to map keys to values of mixed types - the legacy json_decode function most often returns a legacy array as a mapping from a string to a mixed type (at least in the PHP documentation - I could not find anything referring to the Hack adaptation specifically). Similarly, I've written a parser in Hack for TOML, which canonically represents a mapping between strings and mixed values.

Since the legacy PHP object is being replaced with the new type-safe vec and dict types, there is a clear need for a concise type-safe way to extract values from such dicts.

Describe the solution you'd like

As a temporary solution for my parser, I made a class that wraps around a dict<string, nonnull> to provide the required methods. They're essentially all different versions of this function, which returns a string, for each of the other primitive types:

    public function string(string $key) : string { 
		if(! \array_key_exists($key, $this->dict)) throw DictAccessException::DNE($key);
		$x = $this->dict[$key];
		if($x is string) return $x; 
		else throw DictAccessException::WrongType($key, 'string'); 
    }

I also included functions whose name begins with an underscore, such as function _string(string $key) : ?string, which return NULL instead of throwing an exception for a non-existent key.

If the team agrees that this type of solution would work best, I'd be happy to close this issue, tidy up that code and submit a pull request to HSL through the hsl-experimental repo.

Describe alternatives you've considered

Of course, the Hack team could have other plans for this issue, such as directly implementing this functionality in the dict class itself.

Regardless, the biggest question I have moving forward is: what are the current plans for the future of json_decode and similar functions, that are currently documented to use the legacy array type? I think there should be a clear precedent for projects such as my Hack TOML parser to follow when it comes to handling data access like this, and an elegant solution will be very powerful and time-saving in the long run.

Thanks and cheers.

@fredemmott
Copy link
Contributor

dict<string, mixed>

This type almost always indicates that a shape sshould be used; for json_decode (and presumably TOML too), the usual pattern is to define a shape with the expected structure, then use https://github.com/hhvm/type-assert to convert the value to that shape

fail-null type tests

$foo['bar'] ?as string

If potentially unset, ($foo['bar'] ?? null) ?as string

fail-hard type tests

$foo['bar'] as string

what are the current plans for the future of json_decode and similar functions

Leave this until varray/darray/varray_or_darray/array no longer exist; doing any improvements now:

  • is arguably impossible to do 'correctly'
  • makes removal of the legacy array types much harder, delaying actually doing it well

Closing as:

  • there are existing solutions
  • we're not going to add anything dict-specific
  • no action to be taken for now

@fredemmott
Copy link
Contributor

fredemmott commented Jan 29, 2020

json_decode does have a \JSON_FB_HACK_ARRAYS flag, which makes it return vec/dict; the problem is that vecs are not valid tuples, and dicts are not valid shapes; for JSON, we generally decode with that flag, and TypeAssert knows to convert vecs to to tuples and dicts to shapes if they're expected

@fredemmott
Copy link
Contributor

fredemmott commented Jan 29, 2020

The long-term dream (that we've not thought through properly, and I don't think can't be implemented correctly yet) is JSON\decode<reified T>(string $json): T

@jacobconley
Copy link
Author

Ah, I see, this sheds light onto the purpose of Shapes, I hadn't used them before; I was also unaware of reified generics and the TypeAssert library, this is all interesting stuff. It seems like TypeAssert is a more robust version of my class, so I'll definitely migrate my code to use it.

Cheers for all of the great work.

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

No branches or pull requests

2 participants