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

Fix multipleOf bugs #114

Merged
merged 9 commits into from
Nov 1, 2024
Merged
14 changes: 11 additions & 3 deletions src/Schema/Type/Number.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,16 @@ public function validate(mixed $value, callable $fail): void
}
}

if ($this->multipleOf !== null && $value % $this->multipleOf !== 0) {
$fail(sprintf('must be a multiple of %d', $this->multipleOf));
// Divide the value by multipleOf instead of using the modulo operator to avoid bugs when using a multipleOf
// that has decimal places. (Since the modulo operator converts the multipleOf to int)
// Note that dividing two integers returns another integer if the result is a whole number. So to make the
// comparison work at all times we need to cast the result to float. Casting both to integer will not work
// as intended since then the result of the division would also be rounded.
if (
$this->multipleOf !== null &&
(float) ($value / $this->multipleOf) !== round($value / $this->multipleOf)
) {
$fail(sprintf('must be a multiple of %s', $this->multipleOf));
}
}

Expand Down Expand Up @@ -85,7 +93,7 @@ public function maximum(?float $maximum, bool $exclusive = false): static

public function multipleOf(?float $number): static
{
if ($number <= 0) {
if ($number !== null && $number <= 0) {
throw new InvalidArgumentException('multipleOf must be a positive number');
}

Expand Down
16 changes: 16 additions & 0 deletions tests/unit/NumberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ public static function validationProvider(): array
[Number::make()->maximum(10, exclusive: true), 10, false],
[Number::make()->multipleOf(2), 1, false],
[Number::make()->multipleOf(2), 2, true],
[Number::make()->multipleOf(0.01), 100, true],
[Number::make()->multipleOf(0.01), 100.5, true],
[Number::make()->multipleOf(0.01), 100.56, true],
[Number::make()->multipleOf(0.01), 100.567, false],
];
}

Expand All @@ -56,4 +60,16 @@ public function test_validation(Type $type, mixed $value, bool $valid)

$type->validate($value, $fail);
}

public function test_multipleOf_reset(): void
{
$number = Number::make()
->multipleOf(2)
->multipleOf(null);

$fail = $this->createMock(MockedCaller::class);
$fail->expects($this->never())->method('__invoke');

$number->validate(5, $fail);
}
}
Loading