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

Implement multipleOf #29

Merged
merged 6 commits into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Codegen/Constraints/NumberBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
?'maximum' => int,
?'minimum' => int,
?'coerce' => bool,
?'multipleOf' => num,
...
);

Expand Down Expand Up @@ -39,6 +40,14 @@ public function build(): this {
->setValue($minimum, HackBuilderValues::export());
}

$devisor = $this->typed_schema['multipleOf'] ?? null;
if ($devisor is nonnull) {
$properties[] = $this->codegenProperty('devisorForMultipleOf')
->setType('num')
->setValue($devisor, HackBuilderValues::export());
}


$enum = $this->getEnumCodegenProperty();
if ($enum is nonnull) {
$properties[] = $enum;
Expand Down Expand Up @@ -90,6 +99,13 @@ protected function getCheckMethod(): CodegenMethod {
);
}

if (($this->typed_schema['multipleOf'] ?? null) is nonnull) {
$hb->addMultilineCall(
'Constraints\NumberMultipleOfConstraint::check',
vec['$typed', 'self::$devisorForMultipleOf', '$pointer'],
);
}

$hb->addReturn('$typed', HackBuilderValues::literal());

return $this->codegenCheckMethod()
Expand Down
49 changes: 49 additions & 0 deletions src/Constraints/NumberMultipleOfConstraint.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?hh // strict

namespace Slack\Hack\JsonSchema\Constraints;

use namespace HH\Lib\Math;
use namespace Slack\Hack\JsonSchema;

// Because comparing floating point numbers using `===` will have undesierable results,
// we will compare with a little bit of leeway.
// We are looking for a remainer of 0 after the modulo.
// `30 % 3` is 0, so 30 is a multiple of 3.
// `fmod(6., .6)` is not 0, because of floating point rounding errors.
// It is 2.220...E-16, but this is almost zero, so we pass the test.
const float COMPARISON_LEEWAY = 10. ** -6;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do.


class NumberMultipleOfConstraint {
public static function check(num $dividend, num $devisor, string $pointer): void {
invariant($devisor > 0, 'multipleOf 0 or a negative number does not make sense. Use a positive non-zero number.');

$remainer = Math\abs(
$dividend is int && $devisor is int ? $dividend % $devisor : \fmod((float)$dividend, (float)$devisor),
);

$error = shape(
'code' => JsonSchema\FieldErrorCode::FAILED_CONSTRAINT,
'constraint' => shape(
'type' => JsonSchema\FieldErrorConstraint::MULTIPLE_OF,
'expected' => $devisor,
'got' => $dividend,
),
'message' => "must be a multiple of {$devisor}",
);

if ($remainer is int && $remainer !== 0) {
throw new JsonSchema\InvalidFieldException($pointer, vec[$error]);
}
if ($remainer is float) {
// If we are closer to 0 than to the devisor, we check assert that our remainer
// is less than our COMPARISON_LEEWAY.
if ($remainer < $devisor / 2 && $remainer > COMPARISON_LEEWAY) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a comment about this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is a good plan.

throw new JsonSchema\InvalidFieldException($pointer, vec[$error]);
// However, sometimes the remainer is very close to the devisor.
// That could also indicate a multiple of the devisor.
} else if ($remainer > $devisor / 2 && ($devisor - $remainer) > COMPARISON_LEEWAY) {
throw new JsonSchema\InvalidFieldException($pointer, vec[$error]);
}
}
}
}
1 change: 1 addition & 0 deletions src/Exceptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enum FieldErrorConstraint: string {
MIN_LENGTH = 'min_length';
MAXIMUM = 'maximum';
MINIMUM = 'minimum';
MULTIPLE_OF = 'multiple_of';
ENUM = 'enum';
PATTERN = 'pattern';
FORMAT = 'format';
Expand Down
63 changes: 63 additions & 0 deletions tests/MultipleOfConstraintTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?hh // partial

namespace Slack\Hack\JsonSchema\Tests;

use function Facebook\FBExpect\expect;
use type Facebook\HackTest\DataProvider;
use namespace Slack\Hack\JsonSchema;

final class MultipleOfConstraintTest extends BaseCodegenTestCase {

<<__Override>>
public static async function beforeFirstTestAsync(): Awaitable<void> {
$ret = self::getBuilder('multiple-of.json', 'MultipleOfValidator');
$ret['codegen']->build();
require_once($ret['path']);
}

public function provideCasesForIsValid(
): dict<
string,
(shape(?'a_multiple_of_five_int' => int, ?'a_multiple_of_1_point_one_repeating_number' => num), bool),
> {
return dict[
'zero is a multiple of 5' => tuple(shape('a_multiple_of_five_int' => 0), true),
'one is not a multiple of 5' => tuple(shape('a_multiple_of_five_int' => 1), false),
'five is a multiple of 5' => tuple(shape('a_multiple_of_five_int' => 5), true),
'fifty is a multiple of 5' => tuple(shape('a_multiple_of_five_int' => 50), true),
'zero is a mutliple of 1.1...' => tuple(shape('a_multiple_of_1_point_one_repeating_number' => 0), true),
'one is not a mutliple of 1.1...' => tuple(shape('a_multiple_of_1_point_one_repeating_number' => 1), false),
'5.5... is a multiple of 1.1...' =>
// This will have an modulus of 1.1111111111056, testing if I can deal with it being slightly below the devidor.
tuple(shape('a_multiple_of_1_point_one_repeating_number' => 5.55555555555), true),
'5.5...6 is a multiple of 1.1... if you place the 6 far enough back' =>
// This will have an modulus of 4.4444449986969E-7, testing if I can deal with it being slightly above zero.
tuple(shape('a_multiple_of_1_point_one_repeating_number' => 5.555556), true),
'5555555.5... is a multiple of 1.1...' =>
tuple(shape('a_multiple_of_1_point_one_repeating_number' => 55555555.55555555555), true),
// I arbitrarily choose to check for 6 digits. These tests need updating if we choose a different number of digits.
'1.11111 <- 5 times a one behind the period is a multiple of 1.1...' =>
tuple(shape('a_multiple_of_1_point_one_repeating_number' => 1.11111), false),
'1.111111 <- 6 times a one behind the period is a multiple of 1.1...' =>
tuple(shape('a_multiple_of_1_point_one_repeating_number' => 1.111111), true),
];
}

<<DataProvider('provideCasesForIsValid')>>
public function testIsValid(shape(...) $value, bool $is_valid): void {
$schema = new Generated\MultipleOfValidator($value);
$schema->validate();
expect($schema->isValid())->toBeSame($is_valid);
}

public function testForError(): void {
$schema = new Generated\MultipleOfValidator(shape('a_multiple_of_five_int' => 1));
$schema->validate();
$err = $schema->getErrors()[0];
expect(Shapes::idx($err, 'constraint')as nonnull['type'])->toBeSame(JsonSchema\FieldErrorConstraint::MULTIPLE_OF);
expect(Shapes::idx($err, 'constraint') |> Shapes::idx($$ as nonnull, 'got'))->toBeSame(1);
expect(Shapes::idx($err, 'constraint') |> Shapes::idx($$ as nonnull, 'expected'))->toBeSame(5);
expect($err['message'])->toBeSame('must be a multiple of 5');
}

}
109 changes: 109 additions & 0 deletions tests/examples/codegen/MultipleOfValidator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?hh // strict
/**
* This file is generated. Do not modify it manually!
*
* To re-generate this file run `make test`
*
*
* @generated SignedSource<<2a4a4e3e9c399de1de58724be9cf4435>>
*/
namespace Slack\Hack\JsonSchema\Tests\Generated;
use namespace Slack\Hack\JsonSchema;
use namespace Slack\Hack\JsonSchema\Constraints;

type TMultipleOfValidator = shape(
?'a_multiple_of_five_int' => int,
?'a_multiple_of_1_point_one_repeating_number' => num,
...
);

final class MultipleOfValidatorPropertiesAMultipleOfFiveInt {

private static num $devisorForMultipleOf = 5;
private static bool $coerce = false;

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

Constraints\NumberMultipleOfConstraint::check(
$typed,
self::$devisorForMultipleOf,
$pointer,
);
return $typed;
}
}

final class MultipleOfValidatorPropertiesAMultipleOf1PointOneRepeatingNumber {

private static num $devisorForMultipleOf = 1.1111111111111;
private static bool $coerce = false;

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

Constraints\NumberMultipleOfConstraint::check(
$typed,
self::$devisorForMultipleOf,
$pointer,
);
return $typed;
}
}

final class MultipleOfValidator
extends JsonSchema\BaseValidator<TMultipleOfValidator> {

private static bool $coerce = false;

public static function check(
mixed $input,
string $pointer,
): TMultipleOfValidator {
$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) {
/* HH_IGNORE_ERROR[4051] allow dynamic access to preserve input. See comment in the codegen lib for reasoning and alternatives if needed. */
$output[$key] = $value;
}

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

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

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

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

<<__Override>>
protected function process(): TMultipleOfValidator {
return self::check($this->input, $this->pointer);
}
}
13 changes: 13 additions & 0 deletions tests/examples/multiple-of.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"type": "object",
"properties": {
"a_multiple_of_five_int": {
"type": "integer",
"multipleOf": 5
},
"a_multiple_of_1_point_one_repeating_number": {
"type": "number",
"multipleOf": 1.1111111111111111
}
}
}