Skip to content

Commit

Permalink
Fix multipleOf bugs (#114)
Browse files Browse the repository at this point in the history
* Fix bug when using multipleOf values with decimal places
* Allow resetting multipleOf back to null
* Add tests for multipleOf bugs
* Fix incorrect formatting of multipleOf in error message

- `%d` casts the multipleOf to a whole number even if its a float with decimals
- `%f` would add unnecessary decimals, e.g. `0.01` would become `0.010000`
- `%s` maintains the same number of decimals, e.g. `0.01` stays `0.01`

---------

Co-authored-by: bertramakers <[email protected]>
  • Loading branch information
bertramakers and bertramakers authored Nov 1, 2024
1 parent e913512 commit cc41c28
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
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);
}
}

0 comments on commit cc41c28

Please sign in to comment.