Skip to content

Commit 3b43d7e

Browse files
ianhoffmanianhoffman
committed
Improve type inference for UntypedBuilder
1 parent 6c0934b commit 3b43d7e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1939
-934
lines changed

‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ lint:
1919
docker run -v `pwd`:/app -it slack/hack-json-schema ./vendor/bin/hhast-lint
2020

2121
format:
22-
docker run -v `pwd`:/app -it slack/hack-json-schema find {src,tests} -type f -name "*.hack" -o -name "*.php" -exec hackfmt -i {} \;
22+
docker run -v `pwd`:/app -it slack/hack-json-schema find {src,tests} -type f \( -name "*.hack" -o -name "*.php" \) -exec hackfmt -i {} \;
2323

2424
typecheck:
2525
docker run -v `pwd`:/app -it slack/hack-json-schema /bin/bash -c './vendor/bin/hh-autoload && hh_server --check .'

‎src/Codegen/Codegen.php

+5
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ public function build(): this {
263263
$this->file->addBeforeType(
264264
$this->getHackCodegenFactory()->codegenType($this->getType())->setType($this->builder->getType()),
265265
);
266+
Typing\TypeSystem::registerAlias($this->getType(), $this->builder->getTypeInfo());
266267
}
267268

268269
$this->class = $this->class
@@ -329,6 +330,10 @@ public function isArrayKeyType(): bool {
329330
return $this->builder->isArrayKeyType();
330331
}
331332

333+
public function getTypeInfo(): Typing\Type {
334+
return $this->builder->getTypeInfo();
335+
}
336+
332337
public function setSuffix(string $_suffix): void {
333338
}
334339
}

‎src/Codegen/Constraints/ArrayBuilder.php

+14
Original file line numberDiff line numberDiff line change
@@ -268,4 +268,18 @@ private function determineHackArrayType(): void {
268268
$this->hackArrayType = HackArrayType::KEYSET;
269269
}
270270
}
271+
272+
<<__Override>>
273+
public function getTypeInfo(): Typing\Type {
274+
if ($this->singleItemSchemaBuilder) {
275+
$inner = $this->singleItemSchemaBuilder->getTypeInfo();
276+
} else {
277+
$inner = Typing\TypeSystem::mixed();
278+
}
279+
if ($this->hackArrayType === HackArrayType::KEYSET) {
280+
return Typing\TypeSystem::keyset($inner);
281+
} else {
282+
return Typing\TypeSystem::vec($inner);
283+
}
284+
}
271285
}

‎src/Codegen/Constraints/BooleanBuilder.php

+4
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,8 @@ protected function getCheckMethod(): CodegenMethod {
5151
->setReturnType($this->getType());
5252
}
5353

54+
<<__Override>>
55+
public function getTypeInfo(): Typing\Type {
56+
return Typing\TypeSystem::bool();
57+
}
5458
}

‎src/Codegen/Constraints/IBuilder.php

+1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ public function getType(): string;
88
public function isArrayKeyType(): bool;
99
public function build(): this;
1010
public function setSuffix(string $suffix): void;
11+
public function getTypeInfo(): Typing\Type;
1112
}

‎src/Codegen/Constraints/NullBuilder.php

+4
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,8 @@ public function getType(): string {
4444
return 'null';
4545
}
4646

47+
<<__Override>>
48+
public function getTypeInfo(): Typing\Type {
49+
return Typing\TypeSystem::null();
50+
}
4751
}

‎src/Codegen/Constraints/NumberBuilder.php

+11
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,15 @@ public function getType(): string {
120120
return 'num';
121121
}
122122

123+
<<__Override>>
124+
public function getTypeInfo(): Typing\Type {
125+
if ($this->getType() === 'int') {
126+
return Typing\TypeSystem::int();
127+
} else if ($this->getType() === 'num') {
128+
return Typing\TypeSystem::num();
129+
} else {
130+
// TODO: Handle hackEnum
131+
return Typing\TypeSystem::mixed();
132+
}
133+
}
123134
}

‎src/Codegen/Constraints/ObjectBuilder.php

+6
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ public function build(): this {
109109

110110
// Generate a type based on the specified properties
111111
$type = $this->codegenType($property_classes, $pattern_properties_classes);
112+
// TODO: Register objects as shapes or dicts
113+
Typing\TypeSystem::registerAlias($this->getType(), Typing\TypeSystem::nonnull());
112114
$this->ctx->getFile()->addBeforeType($type);
113115
return $this;
114116
}
@@ -560,4 +562,8 @@ private function codegenType(
560562
}
561563
}
562564

565+
<<__Override>>
566+
public function getTypeInfo(): Typing\Type {
567+
return Typing\TypeSystem::alias($this->getType());
568+
}
563569
}

‎src/Codegen/Constraints/SchemaBuilder.php

+4
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ public function getResolvedContext(): Context {
172172
return $this->resolvedContext;
173173
}
174174

175+
public function getTypeInfo(): Typing\Type {
176+
return $this->builder->getTypeInfo();
177+
}
178+
175179
public function setSuffix(string $suffix): void {
176180
$this->builder->setSuffix($suffix);
177181
}

‎src/Codegen/Constraints/StringBuilder.php

+9
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,13 @@ public function getType(): string {
151151

152152
return 'string';
153153
}
154+
155+
<<__Override>>
156+
public function getTypeInfo(): Typing\Type {
157+
if ($this->getType() === 'string') {
158+
return Typing\TypeSystem::string();
159+
}
160+
// TODO: Type resolution for hackEnum
161+
return Typing\TypeSystem::nonnull();
162+
}
154163
}

‎src/Codegen/Constraints/UniqueRefBuilder.php

+4
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ public function setSuffix(string $_suffix): void {
9696
// noop because we don't use suffixes for unique refs
9797
}
9898

99+
public function getTypeInfo(): Typing\Type {
100+
return Typing\TypeSystem::alias($this->getType());
101+
}
102+
99103
/**
100104
* Static function for generating class and file names based off the unique
101105
* ref config. This is static because we use it in `Codegen::forPaths`

‎src/Codegen/Constraints/UntypedBuilder.php

+45-39
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@
33
namespace Slack\Hack\JsonSchema\Codegen;
44

55
use namespace HH\Lib\{C, Math, Str, Vec};
6-
use type Facebook\HackCodegen\{CodegenMethod, CodegenType, HackBuilder, HackBuilderKeys, HackBuilderValues};
6+
use type Facebook\HackCodegen\{
7+
CodegenClass,
8+
CodegenMethod,
9+
CodegenType,
10+
HackBuilder,
11+
HackBuilderKeys,
12+
HackBuilderValues,
13+
};
714

815
type TUntypedSchema = shape(
916
?'anyOf' => vec<TSchema>,
@@ -20,7 +27,12 @@
2027

2128
class UntypedBuilder extends BaseBuilder<TUntypedSchema> {
2229
protected static string $schema_name = 'Slack\hack\JsonSchema\Codegen\TUntypedSchema';
23-
private ?string $current_type = null;
30+
private Typing\Type $type_info;
31+
32+
public function __construct(Context $ctx, string $suffix, TSchema $schema, ?CodegenClass $class = null) {
33+
parent::__construct($ctx, $suffix, $schema, $class);
34+
$this->type_info = Typing\TypeSystem::mixed();
35+
}
2436

2537
<<__Override>>
2638
public function build(): this {
@@ -31,6 +43,7 @@ public function build(): this {
3143

3244
$type = $this->codegenType();
3345
$this->ctx->getFile()->addBeforeType($type);
46+
Typing\TypeSystem::registerAlias($this->getType(), $this->type_info);
3447

3548
return $this;
3649
}
@@ -128,6 +141,7 @@ private function generateNotChecks(vec<TSchema> $schemas, HackBuilder $hb): void
128141

129142
}
130143

144+
// TODO: Determine Lowest Upper Bound for oneOf constraint.
131145
private function generateOneOfChecks(vec<TSchema> $schemas, HackBuilder $hb): void {
132146
$constraints = vec[];
133147
foreach ($schemas as $index => $schema) {
@@ -184,6 +198,7 @@ private function generateOneOfChecks(vec<TSchema> $schemas, HackBuilder $hb): vo
184198

185199
}
186200

201+
// TODO: Determine Greatest Lower Bound for allOf constraint.
187202
private function generateAllOfChecks(vec<TSchema> $schemas, HackBuilder $hb): void {
188203
$merged_schema = $this->getMergedAllOfChecks();
189204
if ($merged_schema) {
@@ -322,7 +337,6 @@ public function getMergedAllOfChecks(): ?TSchema {
322337
private function generateMergedAllOfChecks(TSchema $schema, HackBuilder $hb): void {
323338
$schema_builder = new SchemaBuilder($this->ctx, $this->generateClassName($this->suffix, 'allOf'), $schema);
324339
$schema_builder->build();
325-
$this->current_type = $schema_builder->getType();
326340
$hb->addReturnf('%s::check($input, $pointer)', $schema_builder->getClassName());
327341
}
328342

@@ -469,47 +483,34 @@ private function getOptimizedAnyOfTypes(vec<SchemaBuilder> $schema_builders): ?T
469483
}
470484

471485
private function generateGenericAnyOfChecks(vec<SchemaBuilder> $schema_builders, HackBuilder $hb): void {
472-
$constraints = vec[];
486+
$present_types = vec[];
487+
$nonnull_builders = vec[];
473488
foreach ($schema_builders as $schema_builder) {
474-
$constraints[] = "{$schema_builder->getClassName()}::check<>";
475-
}
476-
477-
// Checks for the special case of one null and one non-null type in order to
478-
// form a nullable unified type.
479-
//
480-
// For unique references, we require that the schemas be built before
481-
// accessing the type. However, we want to filter out the `NullBuilder` in
482-
// this special case before it gets built. If we run into this case, we'll
483-
// store a reference to the `non_null_schema_builder` that we can reference
484-
// after we've filtered out the `NullBuilder` and have built all the
485-
// schemas. This lets us adjust the current type to be nullable
486-
// (`?<whatever>`)
487-
$non_null_schema_builder = null;
488-
if (C\count($schema_builders) === 1) {
489-
$this->current_type = $schema_builders[0]->getType();
490-
} else if (C\count($schema_builders) === 2) {
491-
$without_null = Vec\filter($schema_builders, $sb ==> !($sb->getBuilder() is NullBuilder));
492-
493-
if (C\count($without_null) === 1) {
494-
$non_null_schema_builder = $without_null[0];
495-
496-
$constraints = vec["{$non_null_schema_builder->getClassName()}::check<>"];
497-
$schema_builders = vec[$non_null_schema_builder];
498-
499-
$hb
500-
->startIfBlock('$input === null')
501-
->addReturn(null, HackBuilderValues::export())
502-
->endIfBlock()
503-
->ensureEmptyLine();
489+
// Avoid building null validators as we're going to instead
490+
// bail out using a guard.
491+
if ($schema_builder->getBuilder() is NullBuilder) {
492+
$present_types[] = Typing\TypeSystem::null();
493+
} else {
494+
$schema_builder->build();
495+
$nonnull_builders[] = $schema_builder;
496+
$present_types[] = $schema_builder->getTypeInfo();
504497
}
505498
}
506499

507-
foreach ($schema_builders as $sb) {
508-
$sb->build();
500+
$this->type_info = Typing\TypeSystem::union($present_types);
501+
if ($this->type_info->isOptional()) {
502+
// If the type is optional, don't bother building null builders
503+
// and instead just short-circuit.
504+
$hb
505+
->startIfBlock('$input === null')
506+
->addReturn(null, HackBuilderValues::export())
507+
->endIfBlock()
508+
->ensureEmptyLine();
509509
}
510510

511-
if ($non_null_schema_builder is nonnull) {
512-
$this->current_type = '?'.$non_null_schema_builder->getType();
511+
$constraints = vec[];
512+
foreach ($nonnull_builders as $schema_builder) {
513+
$constraints[] = "{$schema_builder->getClassName()}::check<>";
513514
}
514515

515516
$hb
@@ -628,7 +629,12 @@ private function codegenType(): CodegenType {
628629
return $this->ctx
629630
->getHackCodegenFactory()
630631
->codegenType($this->getType())
631-
->setType($this->current_type ?? 'mixed');
632+
->setType($this->type_info->render());
632633
}
633634

635+
<<__Override>>
636+
public function getTypeInfo(): Typing\Type {
637+
invariant($this->type_info is nonnull, 'must call `build` method before accessing type info');
638+
return $this->type_info;
639+
}
634640
}

‎src/Codegen/Typing/ConcreteType.hack

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
namespace Slack\Hack\JsonSchema\Codegen\Typing;
2+
3+
use namespace HH\Lib\{Str, Vec};
4+
5+
/**
6+
* Represents a concrete type in the Hack type system,
7+
* i.e., a type which cannot be satisfied by `null`
8+
* and which is not a type alias.
9+
*
10+
* Concrete types may contain generics. It may be possible to also
11+
* represent optional types as concrete types containing a single
12+
* generic, but using a separate type for optionals seemed like a simpler
13+
* approach for now.
14+
*/
15+
final class ConcreteType extends Type {
16+
public function __construct(private ConcreteTypeName $name, private vec<Type> $generics = vec[]) {}
17+
18+
<<__Override>>
19+
public function getConcreteTypeName(): ConcreteTypeName {
20+
return $this->name;
21+
}
22+
23+
<<__Override>>
24+
public function getGenerics(): vec<Type> {
25+
return $this->generics;
26+
}
27+
28+
<<__Override>>
29+
public function hasAlias(): bool {
30+
return false;
31+
}
32+
33+
<<__Override>>
34+
public function isOptional(): bool {
35+
return false;
36+
}
37+
38+
<<__Override>>
39+
public function render(): string {
40+
$out = (string)$this->name;
41+
$generics = $this->getGenerics();
42+
if ($generics) {
43+
$out = $out.'<'.Str\join(Vec\map($generics, $generic ==> $generic->render()), ', ').'>';
44+
}
45+
// TODO: Handle shape fields
46+
return $out;
47+
}
48+
}

‎src/Codegen/Typing/ConcreteTypeName.hack

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
namespace Slack\Hack\JsonSchema\Codegen\Typing;
2+
3+
/**
4+
* The various types which have names in the Hack type system.
5+
*
6+
* In reality, this isn't everything, but it's enough for our purposes.
7+
*/
8+
enum ConcreteTypeName: string {
9+
ARRAYKEY = 'arraykey';
10+
BOOL = 'bool';
11+
DICT = 'dict';
12+
INT = 'int';
13+
KEYSET = 'keyset';
14+
NONNULL = 'nonnull';
15+
NOTHING = 'nothing';
16+
NUM = 'num';
17+
SHAPE = 'shape';
18+
STRING = 'string';
19+
VEC = 'vec';
20+
}

0 commit comments

Comments
 (0)