Skip to content

Commit

Permalink
Never generate structs as keys for Hack
Browse files Browse the repository at this point in the history 
Summary: hack containers only accept arraykeys as their key type. Anything else throws at runtime. Have thrift just generate arraykey in these places, as we're currently definitely not getting anything else anyway

Reviewed By: yfeldblum, stevegury

Differential Revision: D14111392

fbshipit-source-id: 35f29dc82ca69e4f6b818d24eb76c287c7b7fd66
  • Loading branch information
@DavidSnider @facebook-github-bot
DavidSnider authored and facebook-github-bot committed Feb 16, 2019
1 parent 66b50e6 commit 433b732
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 18 deletions.
15 changes: 10 additions & 5 deletions thrift/compiler/generate/t_hack_generator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3747,6 +3747,8 @@ t_hack_generator::type_to_typehint(t_type* ttype, bool nullable, bool shape) {
type_to_typehint(((t_map*)ttype)->get_key_type(), false, shape);
if (shape && shape_arraykeys_ && key_type == "string") {
key_type = "arraykey";
} else if (((t_map*)ttype)->get_key_type()->is_struct()) {
key_type = "arraykey";
}
return prefix + "<" + key_type + ", " +
type_to_typehint(((t_map*)ttype)->get_val_type(), false, shape) + ">";
Expand All @@ -3762,9 +3764,10 @@ t_hack_generator::type_to_typehint(t_type* ttype, bool nullable, bool shape) {
prefix = const_collections_ ? "ConstSet" : "Set";
}
string suffix = (arraysets_ || (shape && !arrays_)) ? ", bool>" : ">";
return prefix + "<" +
type_to_typehint(((t_set*)ttype)->get_elem_type(), false, shape) +
suffix;
string key_type = ((t_map*)ttype)->get_key_type()->is_struct()
? "arraykey"
: type_to_typehint(((t_set*)ttype)->get_elem_type(), false, shape);
return prefix + "<" + key_type + suffix;
} else {
return "mixed";
}
Expand All @@ -3787,9 +3790,11 @@ string t_hack_generator::type_to_param_typehint(t_type* ttype, bool nullable) {
if (strict_types_) {
return type_to_typehint(ttype, nullable);
} else {
auto key_type = ((t_map*)ttype)->get_key_type();
return "\\HH\\KeyedContainer<" +
type_to_param_typehint(((t_map*)ttype)->get_key_type()) + ", " +
type_to_param_typehint(((t_map*)ttype)->get_val_type()) + ">";
(key_type->is_struct() ? "arraykey"
: type_to_param_typehint(key_type)) +
", " + type_to_param_typehint(((t_map*)ttype)->get_val_type()) + ">";
}
} else {
return type_to_typehint(ttype, nullable);
Expand Down
20 changes: 10 additions & 10 deletions thrift/compiler/test/fixtures/hack-struct-keys/gen-hack/Baz.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface BazAsyncIf extends \IThriftAsyncIf {
* 2: list<Bar> b,
* 3: map<Foo, string> c);
*/
public function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): Awaitable<string>;
public function qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): Awaitable<string>;
}

/**
Expand All @@ -33,7 +33,7 @@ interface BazIf extends \IThriftSyncIf {
* 2: list<Bar> b,
* 3: map<Foo, string> c);
*/
public function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): string;
public function qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): string;
}

/**
Expand All @@ -43,7 +43,7 @@ public function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedConta
trait BazClientBase {
require extends \ThriftClientBase;

protected function sendImpl_qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): int {
protected function sendImpl_qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): int {
$currentseqid = $this->getNextSequenceID();
$args = new Baz_qux_args(
$a,
Expand Down Expand Up @@ -154,7 +154,7 @@ class BazAsyncClient extends \ThriftClientBase implements BazAsyncIf {
* 2: list<Bar> b,
* 3: map<Foo, string> c);
*/
public async function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): Awaitable<string> {
public async function qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): Awaitable<string> {
$currentseqid = $this->sendImpl_qux($a, $b, $c);
await $this->asyncHandler_->genWait($currentseqid);
return $this->recvImpl_qux($currentseqid);
Expand All @@ -166,7 +166,7 @@ class BazClient extends \ThriftClientBase implements BazIf {
use BazClientBase;

<<__Deprecated('use gen_qux()')>>
public function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): string {
public function qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): string {
$currentseqid = $this->sendImpl_qux($a, $b, $c);
return $this->recvImpl_qux($currentseqid);
}
Expand All @@ -178,14 +178,14 @@ public function qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedConta
* 2: list<Bar> b,
* 3: map<Foo, string> c);
*/
public async function gen_qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): Awaitable<string> {
public async function gen_qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): Awaitable<string> {
$currentseqid = $this->sendImpl_qux($a, $b, $c);
await $this->asyncHandler_->genWait($currentseqid);
return $this->recvImpl_qux($currentseqid);
}

/* send and recv functions */
public function send_qux(Set<Foo> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<Foo, string> $c): int {
public function send_qux(Set<arraykey> $a, \HH\KeyedContainer<int, Bar> $b, \HH\KeyedContainer<arraykey, string> $c): int {
return $this->sendImpl_qux($a, $b, $c);
}
public function recv_qux(?int $expectedsequenceid = null): string {
Expand Down Expand Up @@ -240,11 +240,11 @@ class Baz_qux_args implements \IThriftStruct {
'c' => 3,
};
const int STRUCTURAL_ID = 1160460043765273624;
public Set<Foo> $a;
public Set<arraykey> $a;
public Vector<Bar> $b;
public Map<Foo, string> $c;
public Map<arraykey, string> $c;

public function __construct(?Set<Foo> $a = null, ?Vector<Bar> $b = null, ?Map<Foo, string> $c = null ) {
public function __construct(?Set<arraykey> $a = null, ?Vector<Bar> $b = null, ?Map<arraykey, string> $c = null ) {
if ($a === null) {
$this->a = Set {};
} else {
Expand Down
6 changes: 3 additions & 3 deletions thrift/compiler/test/fixtures/hack-struct-keys/gen-hack/module_types.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ class Bar implements \IThriftStruct {
* Original thrift field:-
* 1: set<struct module.Foo> a
*/
public Set<Foo> $a;
public Set<arraykey> $a;
/**
* Original thrift field:-
* 2: map<struct module.Foo, i32> b
*/
public Map<Foo, int> $b;
public Map<arraykey, int> $b;

public function __construct(?Set<Foo> $a = null, ?Map<Foo, int> $b = null ) {
public function __construct(?Set<arraykey> $a = null, ?Map<arraykey, int> $b = null ) {
if ($a === null) {
$this->a = Set {};
} else {
Expand Down

0 comments on commit 433b732

Please sign in to comment.