Merge pull request #57 from slackhq/ih_hackEnum_constraint

Support easy coercion to Hack enums

Author: ianhoffman

README.md

@@ -82,10 +82,10 @@ By defining the `source_root` and `output_root` we can generate unique validator
 ## Developing
 
 ### Installing Dependencies
-We use composer to install our dependencies. Running the following will install composer (if it doesn't exist already) and install the dependencies:
+We handle all dependencies through Docker. It's as simple as:
 
 ```console
-make composer-install
+make install
 ```
 
 ### Running Tests

src/Codegen/Constraints/BaseBuilder.php

@@ -2,6 +2,7 @@
 
 namespace Slack\Hack\JsonSchema\Codegen;
 
+use namespace HH\Lib\{C, Str};
 use type Facebook\HackCodegen\{CodegenClass, CodegenMethod, CodegenProperty, HackBuilder, HackBuilderValues};
 
 <<__ConsistentConstruct>>
@@ -92,6 +93,66 @@ protected function addEnumConstraintCheck(HackBuilder $hb): void {
     }
   }
 
+  protected function addHackEnumConstraintCheck(HackBuilder $hb): void {
+    $schema = type_assert_type($this->typed_schema, TSchema::class);
+    if (!Shapes::keyExists($schema, 'hackEnum')) {
+      return;
+    }
+
+    try {
+      $rc = new \ReflectionClass($schema['hackEnum']);
+    } catch (\ReflectionException $e) {
+      throw new \Exception(Str\format("Hack enum '%s' does not exist", $schema['hackEnum']));
+    }
+
+    invariant($rc->isEnum(), "'%s' is not an enum", $schema['hackEnum']);
+
+    $schema_type = $schema['type'] ?? null;
+    $hack_enum_values = keyset[];
+    foreach ($rc->getConstants() as $hack_enum_value) {
+      if ($schema_type === TSchemaType::INTEGER_T) {
+        $hack_enum_value = $hack_enum_value ?as int;
+      } else {
+        $hack_enum_value = $hack_enum_value ?as string;
+      }
+      invariant(
+        $hack_enum_value is nonnull,
+        "'%s' must contain only values of type %s",
+        $rc->getName(),
+        $schema_type === TSchemaType::INTEGER_T ? 'int' : 'string'
+      );
+      $hack_enum_values[] = $hack_enum_value;
+    }
+
+    if (Shapes::keyExists($schema, 'enum')) {
+      // If both `enum` and `hackEnum` are specified, assert that `enum` is a subset of
+      // `hackEnum` values. Any value not also in `hackEnum` can't be valid.
+      foreach ($schema['enum'] as $enum_value) {
+        invariant(
+          $enum_value is string,
+          "Enum value '%s' is not a valid value for '%s'",
+          \print_r($enum_value, true),
+          $rc->getName()
+        );
+        invariant(
+          C\contains_key($hack_enum_values, $enum_value),
+          "Enum value '%s' is unexpectedly not present in '%s'",
+          \print_r($enum_value, true),
+          $rc->getName()
+        );
+      }
+    }
+
+    $hb->addMultilineCall(
+      '$typed = Constraints\HackEnumConstraint::check',
+      vec[
+        '$typed',
+        Str\format('\%s::class', $rc->getName()),
+        '$pointer'
+      ]
+    );
+  }
+
   public function addBuilderClass(CodegenClass $class): void {
     if ($this->class) {
       return;

src/Codegen/Constraints/NumberBuilder.php

@@ -2,6 +2,7 @@
 
 namespace Slack\Hack\JsonSchema\Codegen;
 
+use namespace HH\Lib\Str;
 use type Facebook\HackCodegen\{CodegenMethod, HackBuilderValues};
 
 type TNumberSchema = shape(
@@ -10,6 +11,7 @@
   ?'minimum' => int,
   ?'coerce' => bool,
   ?'multipleOf' => num,
+  ?'hackEnum' => string,
   ...
 );
 
@@ -63,12 +65,11 @@ public function build(): this {
   protected function getCheckMethod(): CodegenMethod {
     $hb = $this->getHackBuilder();
 
+    $return_type = $this->getType();
     if ($this->typed_schema['type'] === TSchemaType::INTEGER_T) {
       $type_constraint = 'Constraints\IntegerConstraint';
-      $return_type = 'int';
     } else {
       $type_constraint = 'Constraints\NumberConstraint';
-      $return_type = 'num';
     }
 
     $hb
@@ -96,6 +97,10 @@ protected function getCheckMethod(): CodegenMethod {
       );
     }
 
+    if ($this->typed_schema['type'] === TSchemaType::INTEGER_T) {
+      $this->addHackEnumConstraintCheck($hb);
+    }
+
     $hb->addReturn('$typed', HackBuilderValues::literal());
 
     return $this->codegenCheckMethod()
@@ -107,6 +112,9 @@ protected function getCheckMethod(): CodegenMethod {
   <<__Override>>
   public function getType(): string {
     if ($this->typed_schema['type'] === TSchemaType::INTEGER_T) {
+      if (Shapes::keyExists($this->typed_schema, 'hackEnum')) {
+        return Str\format('\%s', $this->typed_schema['hackEnum']);
+      }
       return 'int';
     }
     return 'num';

src/Codegen/Constraints/SchemaBuilder.php

@@ -24,6 +24,7 @@ enum TSchemaType: string {
   ?'$ref' => string,
   ?'default' => mixed,
   ?'enum' => vec<mixed>,
+  ?'hackEnum' => string,
   ...
 );
 

src/Codegen/Constraints/StringBuilder.php

@@ -3,14 +3,15 @@
 namespace Slack\Hack\JsonSchema\Codegen;
 
 use function Slack\Hack\JsonSchema\get_function_name_from_function;
-
+use namespace HH\Lib\Str;
 use type Facebook\HackCodegen\{CodegenMethod, HackBuilderValues};
 
 type TStringSchema = shape(
   'type' => TSchemaType,
   ?'maxLength' => int,
   ?'minLength' => int,
   ?'enum' => vec<string>,
+  ?'hackEnum' => string,
   ?'pattern' => string,
   ?'format' => string,
   ?'sanitize' => shape(
@@ -132,16 +133,22 @@ protected function getCheckMethod(): CodegenMethod {
       );
     }
 
+    $this->addHackEnumConstraintCheck($hb);
+
     $hb->addReturn('$typed', HackBuilderValues::literal());
 
     return $this->codegenCheckMethod()
       ->addParameters(vec['mixed $input', 'string $pointer'])
       ->setBody($hb->getCode())
-      ->setReturnType('string');
+      ->setReturnType($this->getType());
   }
 
   <<__Override>>
   public function getType(): string {
+    if (Shapes::keyExists($this->typed_schema, 'hackEnum')) {
+      return Str\format('\%s', $this->typed_schema['hackEnum']);
+    }
+
     return 'string';
   }
 }

src/Codegen/Utils.php

@@ -12,6 +12,12 @@ function _type_assert_get_type_structure(string $type) {
   return $s->getResolvedTypeStructure();
 }
 
+function type_assert_type<T>(mixed $var, typename<T> $type): T {
+  $ts = _type_assert_get_type_structure($type);
+  $result = TypeAssert\matches_type_structure($ts, $var);
+  return $result;
+}
+
 function type_assert_shape<T>(mixed $var, string $shape): T {
   $ts = _type_assert_get_type_structure($shape);
   $result = TypeAssert\matches_type_structure($ts, $var);

src/Constraints/HackEnumConstraint.php

@@ -0,0 +1,34 @@
+<?hh // strict
+
+namespace Slack\Hack\JsonSchema\Constraints;
+
+use namespace Slack\Hack\JsonSchema;
+
+/**
+ * Assert the input is a valid value for a Hack enum.
+ *
+ * This is ideally *not* a replacement for the `enum` constraint, but should instead
+ * be used alongside it: that way, generated code in different languages (i.e, not
+ * in Hack) will still pass valid enum values. However, there are instances when an
+ * enum simply has so many values that leveraging the `enum` constraint becomes unwieldy,
+ * and `hackEnum` may still be preferable to manually-written validation in those cases.
+ */
+final class HackEnumConstraint {
+    public static function check<T as arraykey>(mixed $input, \HH\enumname<T> $enum_class, string $pointer): T {
+        $typed = $enum_class::coerce($input);
+        if ($typed is nonnull) {
+            return $typed;
+        }
+
+        $error = shape(
+            'code' => JsonSchema\FieldErrorCode::FAILED_CONSTRAINT,
+            'constraint' => shape(
+                'type' => JsonSchema\FieldErrorConstraint::ENUM,
+                'expected' => keyset($enum_class::getNames()),
+                'got' => $input,
+            ),
+            'message' => 'must be a valid enum value',
+        );
+        throw new JsonSchema\InvalidFieldException($pointer, vec[$error]);
+    }
+}

tests/ArraySchemaValidatorTest.php

@@ -216,4 +216,16 @@ public function testUnsupportedUniqueItemsConstraint(): void {
     expect($validated)->toEqual(shape('unsupported_unique_items' => vec[shape('foo' => 'a'), shape('foo' => 'a')]));
   }
 
+  public function testHackEnumItems(): void {
+    $input = vec['foo', 'bar'];
+
+    $validator = new ArraySchemaValidator(dict['hack_enum_items' => $input]);
+    $validator->validate();
+
+    expect($validator->isValid())->toBeTrue();
+    $validated = $validator->getValidatedInput();
+
+    expect($validated)->toEqual(shape('hack_enum_items' => vec[TestStringEnum::ABC, TestStringEnum::DEF]));
+  }
+
 }

tests/BaseCodegenTestCase.php

@@ -5,14 +5,22 @@
 use namespace HH\Lib\{C, Math, Str};
 use function Facebook\FBExpect\expect;
 use type Slack\Hack\JsonSchema\Validator;
-use type Slack\Hack\JsonSchema\Codegen\{Codegen, IJsonSchemaCodegenConfig};
+use type Slack\Hack\JsonSchema\Codegen\{Codegen, IJsonSchemaCodegenConfig, TSchema};
 use type Facebook\HackTest\HackTest;
 
 abstract class BaseCodegenTestCase extends HackTest {
 
   const string CODEGEN_ROOT = __DIR__.'/examples/codegen';
   const string CODEGEN_NS = 'Slack\\Hack\\JsonSchema\\Tests\\Generated';
 
+  const type TOptions = shape(
+    ?'sanitize_string' => Codegen::TSanitizeStringConfig,
+    ?'json_schema_codegen_config' => IJsonSchemaCodegenConfig,
+    ?'refs' => Codegen::TValidatorRefsConfig,
+    ?'defaults' => Codegen::TValidatorDefaultsConfig,
+    ?'discard_additional_properties' => bool,
+  );
+
   public function assertUnchanged(string $_value, ?string $_token = null): void {
     self::markTestSkipped("assertUnchanged doesn't work in hacktest yet");
 
@@ -61,17 +69,38 @@ public static function getCodeGenPath(string $file): string {
   public static function getBuilder(
     string $json_filename,
     string $name,
-    shape(
-      ?'sanitize_string' => Codegen::TSanitizeStringConfig,
-      ?'json_schema_codegen_config' => IJsonSchemaCodegenConfig,
-      ?'refs' => Codegen::TValidatorRefsConfig,
-      ?'defaults' => Codegen::TValidatorDefaultsConfig,
-      ?'discard_additional_properties' => bool,
-    ) $options = shape(),
+    this::TOptions $options = shape(),
+  ): shape(
+    'path' => string,
+    'codegen' => Codegen,
+  ) {
+    $codegen_config = self::getConfig($name, $options);
+    $codegen = Codegen::forPath(__DIR__."/examples/{$json_filename}", $codegen_config);
+
+    return shape(
+      'path' => $codegen_config['validator']['file'],
+      'codegen' => $codegen,
+    );
+  }
+
+  public static function getBuilderForSchema(
+    TSchema $schema,
+    string $name,
+    this::TOptions $options = shape(),
   ): shape(
     'path' => string,
     'codegen' => Codegen,
   ) {
+    $codegen_config = self::getConfig($name, $options);
+    $codegen = Codegen::forSchema(Shapes::toDict($schema), $codegen_config, __DIR__.'/examples/');
+
+    return shape(
+      'path' => $codegen_config['validator']['file'],
+      'codegen' => $codegen,
+    );
+  }
+
+  private static function getConfig(string $name, this::TOptions $options): Codegen::TCodegenConfig {
     $path = self::getCodeGenPath("{$name}.php");
     $validator_config = shape(
       'namespace' => self::CODEGEN_NS,
@@ -108,12 +137,7 @@ public static function getBuilder(
       $codegen_config['jsonSchemaCodegenConfig'] = $json_schema_codegen_config;
     }
 
-    $codegen = Codegen::forPath(__DIR__."/examples/{$json_filename}", $codegen_config);
-
-    return shape(
-      'path' => $path,
-      'codegen' => $codegen,
-    );
+    return $codegen_config;
   }
 
   public static function benchmark(string $label, (function(): void) $callback): void {

tests/InvalidEnumTest.php

@@ -0,0 +1,44 @@
+<?hh // strict
+
+namespace Slack\Hack\JsonSchema\Tests;
+
+use function Facebook\FBExpect\expect;
+
+final class NotAnEnum {}
+
+final class InvalidEnumTest extends BaseCodegenTestCase {
+
+  public function testNotAnEnum(): void {
+    $ret = self::getBuilderForSchema(
+        shape(
+            'type' => 'string',
+            'hackEnum' => NotAnEnum::class
+        ),
+        'InvalidEnumValidator'
+    );
+    expect(() ==> $ret['codegen']->build())->toThrow(\HH\InvariantException::class);
+  }
+
+  public function testEnumAndHackEnum(): void {
+    $ret = self::getBuilderForSchema(
+        shape(
+            'type' => 'string',
+            'hackEnum' => TestStringEnum::class,
+            'enum' => vec['qux']
+        ),
+        'InvalidEnumValidator'
+    );
+    expect(() ==> $ret['codegen']->build())->toThrow(\HH\InvariantException::class);
+  }
+
+  public function testMismatchedEnumType(): void {
+    $ret = self::getBuilderForSchema(
+        shape(
+            'type' => 'string',
+            'hackEnum' => TestIntEnum::class,
+        ),
+        'InvalidEnumValidator'
+    );
+    expect(() ==> $ret['codegen']->build())->toThrow(\HH\InvariantException::class);
+  }
+}