Skip to content

Commit

Permalink
Fix codegen for empty shapes
Browse files Browse the repository at this point in the history
We were returning `dict[]` from functions typed as returning empty
shapes. Sort of surprising this only just came up, but here's the fix.
  • Loading branch information
ianhoffman committed Mar 8, 2023
1 parent 8fa6864 commit d1209ae
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Codegen/Constraints/ObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ protected function getCheckMethodCode(
$hb->addInlineComment('Hack to prevent us from having to change the params names when we are not using them.');
$hb->addAssignment('$_', '$input', HackBuilderValues::literal());
$hb->addAssignment('$_', '$pointer', HackBuilderValues::literal());
$hb->addReturn('dict[]', HackBuilderValues::literal());
$hb->addReturn('shape()', HackBuilderValues::literal());

return $hb;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/ObjectSchemaValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -692,4 +692,29 @@ public function testInvalidMinPropertiesWithNoAdditionalProperties(): void {
expect($constraint['got'] ?? null)->toEqual('a');
}

public function testEmptyClosedShape(): void {
$validator = new ObjectSchemaValidator(dict[
'empty_closed_shape' => dict[]
]);
$validator->validate();
expect($validator->isValid())->toBeTrue();

$validator = new ObjectSchemaValidator(dict[
'empty_closed_shape' => dict['foo' => 'bar']
]);
$validator->validate();
expect($validator->isValid())->toBeFalse();

$errors = $validator->getErrors();
expect(C\count($errors))->toEqual(1);

$error = C\firstx($errors);
expect($error['code'])->toEqual(FieldErrorCode::FAILED_CONSTRAINT);
expect($error['message'])->toEqual('invalid additional property: foo');

$constraint = Shapes::at($error, 'constraint');
expect($constraint['type'])->toEqual(FieldErrorConstraint::ADDITIONAL_PROPERTIES);
expect($constraint['got'] ?? null)->toEqual('foo');
}

}
57 changes: 56 additions & 1 deletion tests/examples/codegen/ObjectSchemaValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* To re-generate this file run `make test`
*
*
* @generated SignedSource<<7fa7d1f5ae7e84bf34ab173568728f05>>
* @generated SignedSource<<5d8158e850af0f6d550037bc4003da67>>
*/
namespace Slack\Hack\JsonSchema\Tests\Generated;
use namespace Slack\Hack\JsonSchema;
Expand Down Expand Up @@ -97,6 +97,9 @@

type TObjectSchemaValidatorPropertiesInvalidMinPropertiesWithNoAdditionalProperties = dict<string, mixed>;

type TObjectSchemaValidatorPropertiesEmptyClosedShape = shape(
);

type TObjectSchemaValidator = shape(
?'only_additional_properties' => TObjectSchemaValidatorPropertiesOnlyAdditionalProperties,
?'only_no_additional_properties' => TObjectSchemaValidatorPropertiesOnlyNoAdditionalProperties,
Expand All @@ -119,6 +122,7 @@
?'only_max_properties' => TObjectSchemaValidatorPropertiesOnlyMaxProperties,
?'min_and_max_properties' => TObjectSchemaValidatorPropertiesMinAndMaxProperties,
?'invalid_min_properties_with_no_additional_properties' => TObjectSchemaValidatorPropertiesInvalidMinPropertiesWithNoAdditionalProperties,
?'empty_closed_shape' => TObjectSchemaValidatorPropertiesEmptyClosedShape,
...
);

Expand Down Expand Up @@ -1648,6 +1652,46 @@ public static function check(
}
}

final class ObjectSchemaValidatorPropertiesEmptyClosedShape {

private static bool $coerce = false;
private static keyset<string> $properties = keyset[
];

public static function check(
mixed $input,
string $pointer,
): TObjectSchemaValidatorPropertiesEmptyClosedShape {
$typed = Constraints\ObjectConstraint::check($input, $pointer, self::$coerce);

$errors = vec[];
$output = shape();

/*HHAST_IGNORE_ERROR[UnusedVariable] Some loops generated with this statement do not use their $value*/
foreach ($typed as $key => $value) {
if (\HH\Lib\C\contains_key(self::$properties, $key)) {
continue;
}

$errors[] = shape(
'code' => JsonSchema\FieldErrorCode::FAILED_CONSTRAINT,
'message' => "invalid additional property: {$key}",
'constraint' => shape(
'type' => JsonSchema\FieldErrorConstraint::ADDITIONAL_PROPERTIES,
'got' => $key,
),
);
}

if (\HH\Lib\C\count($errors)) {
throw new JsonSchema\InvalidFieldException($pointer, $errors);
}

/* HH_IGNORE_ERROR[4163] */
return $output;
}
}

final class ObjectSchemaValidator
extends JsonSchema\BaseValidator<TObjectSchemaValidator> {

Expand Down Expand Up @@ -1899,6 +1943,17 @@ public static function check(
}
}

if (\HH\Lib\C\contains_key($typed, 'empty_closed_shape')) {
try {
$output['empty_closed_shape'] = ObjectSchemaValidatorPropertiesEmptyClosedShape::check(
$typed['empty_closed_shape'],
JsonSchema\get_pointer($pointer, 'empty_closed_shape'),
);
} catch (JsonSchema\InvalidFieldException $e) {
$errors = \HH\Lib\Vec\concat($errors, $e->errors);
}
}

if (\HH\Lib\C\count($errors)) {
throw new JsonSchema\InvalidFieldException($pointer, $errors);
}
Expand Down
6 changes: 6 additions & 0 deletions tests/examples/object-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@
"type": "object",
"minProperties": 1,
"additionalProperties": false
},
"empty_closed_shape": {
"type": "object",
"additionalProperties": false,
"discardAdditionalProperties": false,
"properties": {}
}
}
}

0 comments on commit d1209ae

Please sign in to comment.