Add bit size and signed/unsigned support to Integer validator #5
Conversation
- Add configurable bit sizes (8, 16, 32, 64) with 32-bit as default - Add signed/unsigned option with signed as default - Add range validation based on bit size and signedness - Add getBits(), isUnsigned(), and getFormat() methods - Update getDescription() to reflect specific constraints - Add comprehensive tests for new functionality
WalkthroughThe Integer validator constructor now is Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/Validator/Integer.php (2)
58-67: Consider extracting range calculation to avoid duplication.The min/max calculation logic is duplicated between
getDescription()andisValid(). Consider extracting this into a private helper method or caching the values as properties set during construction.♻️ Suggested refactor
class Integer extends Validator { protected bool $loose = false; protected int $bits = 32; protected bool $unsigned = false; + protected int $min; + protected int $max; public function __construct(bool $loose = false, int $bits = 32, bool $unsigned = false) { if (!\in_array($bits, [8, 16, 32, 64])) { throw new \InvalidArgumentException('Bits must be 8, 16, 32, or 64'); } $this->loose = $loose; $this->bits = $bits; $this->unsigned = $unsigned; + + if ($this->unsigned) { + $this->min = 0; + $this->max = (2 ** $this->bits) - 1; + } else { + $this->min = -(2 ** ($this->bits - 1)); + $this->max = (2 ** ($this->bits - 1)) - 1; + } }Then use
$this->minand$this->maxin bothgetDescription()andisValid().Also applies to: 158-165
133-136:getFormat()doesn't distinguish unsigned integers.OpenAPI and JSON Schema use formats like
uint8,uint16, etc. for unsigned integers. The current implementation returnsint32for both signed and unsigned 32-bit integers, which may lead to incorrect schema generation.♻️ Suggested fix
public function getFormat(): string { - return 'int' . $this->bits; + return ($this->unsigned ? 'uint' : 'int') . $this->bits; }tests/Validator/IntegerTest.php (1)
37-67: Test coverage looks good but could be more comprehensive.The tests cover the core functionality well. Consider adding tests for:
getDescription()output format verification- 32-bit signed boundary values (
-2147483648to2147483647)- Loose mode combined with bit-size validation (e.g.,
new Integer(true, 8)with string inputs)💡 Additional test suggestions
public function testGetDescription() { $validator = new Integer(false, 8); $this->assertStringContainsString('signed', $validator->getDescription()); $this->assertStringContainsString('8-bit', $validator->getDescription()); $validatorU = new Integer(false, 8, true); $this->assertStringContainsString('unsigned', $validatorU->getDescription()); } public function testLooseModeWithBitSize() { $validator = new Integer(true, 8); $this->assertTrue($validator->isValid('127')); $this->assertFalse($validator->isValid('128')); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Validator/Integer.phptests/Validator/IntegerTest.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Validator/IntegerTest.php (1)
src/Validator/Integer.php (5)
Integer(12-174)getBits(109-112)isUnsigned(121-124)getFormat(133-136)isValid(146-173)
🔇 Additional comments (3)
src/Validator/Integer.php (2)
19-27: LGTM!New properties for bit size and signedness are well-documented and have sensible defaults that maintain backward compatibility.
38-47: LGTM!Constructor properly validates the
$bitsparameter against allowed values and throws a clear exception message. The default values ensure backward compatibility with existing code.tests/Validator/IntegerTest.php (1)
69-74: LGTM!Exception testing is properly implemented using PHPUnit's
expectExceptionandexpectExceptionMessagemethods.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/Validator/Integer.php:
- Around line 138-141: getFormat() currently always returns 'int' + $this->bits,
ignoring the unsigned flag; update it to check isUnsigned() (or $unsigned) and
return 'uint' + $this->bits when true (e.g.,
'uint8','uint16','uint32','uint64'), otherwise return 'int' + $this->bits so the
format matches OpenAPI's unsigned integer formats; modify the getFormat() method
accordingly and keep the existing $bits and isUnsigned() usage for determining
the returned string.
🧹 Nitpick comments (2)
tests/Validator/IntegerTest.php (1)
37-67: Consider adding test coverage for edge cases and 32-bit signed range.The test covers several bit sizes but could benefit from additional coverage:
- 32-bit signed range validation: The default 32-bit signed validator is created but its range (
-2,147,483,648to2,147,483,647) is not tested.- 64-bit signed boundary values: Only
getFormat()is tested for 64-bit, but not actual range validation.getDescription()output: Not tested for any configuration.📝 Suggested additional test coverage
// 64-bit signed $validator64 = new Integer(false, 64); $this->assertSame('int64', $validator64->getFormat()); + $this->assertTrue($validator64->isValid(PHP_INT_MAX)); + $this->assertTrue($validator64->isValid(PHP_INT_MIN)); + + // 32-bit signed range validation + $validator32 = new Integer(false, 32); + $this->assertTrue($validator32->isValid(-2147483648)); + $this->assertTrue($validator32->isValid(2147483647)); + $this->assertFalse($validator32->isValid(-2147483649)); + $this->assertFalse($validator32->isValid(2147483648)); + + // Test getDescription output + $this->assertStringContainsString('signed', $validator->getDescription()); + $this->assertStringContainsString('32-bit', $validator->getDescription()); }src/Validator/Integer.php (1)
63-80: Consider extracting range calculation to reduce duplication.The min/max calculation logic is duplicated in both
getDescription()andisValid(). Extracting this to a private method would improve maintainability.♻️ Suggested refactor to extract range calculation
+ /** + * Get Min/Max Range + * + * Returns the min and max values based on bit size and signedness. + * + * @return array{min: int, max: int} + */ + private function getRange(): array + { + if ($this->unsigned) { + return ['min' => 0, 'max' => (2 ** $this->bits) - 1]; + } + return [ + 'min' => -(2 ** ($this->bits - 1)), + 'max' => (2 ** ($this->bits - 1)) - 1 + ]; + } + public function getDescription(): string { $signedness = $this->unsigned ? 'unsigned' : 'signed'; - - // Calculate min and max values based on bit size and signed/unsigned - if ($this->unsigned) { - $min = 0; - $max = (2 ** $this->bits) - 1; - } else { - $min = -(2 ** ($this->bits - 1)); - $max = (2 ** ($this->bits - 1)) - 1; - } + $range = $this->getRange(); return \sprintf( 'Value must be a valid %s %d-bit integer between %s and %s', $signedness, $this->bits, - \number_format($min), - \number_format($max) + \number_format($range['min']), + \number_format($range['max']) ); }Then similarly update
isValid()to use$this->getRange().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Validator/Integer.phptests/Validator/IntegerTest.php
🧰 Additional context used
🧬 Code graph analysis (2)
tests/Validator/IntegerTest.php (1)
src/Validator/Integer.php (5)
Integer(12-179)getBits(114-117)isUnsigned(126-129)getFormat(138-141)isValid(151-178)
src/Validator/Integer.php (1)
src/Validator/Range.php (2)
__construct(34-39)getFormat(66-69)
🔇 Additional comments (4)
tests/Validator/IntegerTest.php (1)
69-81: LGTM!Exception tests properly verify both invalid bit size and 64-bit unsigned rejection using PHPUnit's
expectExceptionandexpectExceptionMessage.src/Validator/Integer.php (3)
19-27: LGTM!New properties for bit size and signedness are well-documented and have sensible defaults that maintain backward compatibility.
38-52: LGTM!Constructor validation is solid:
- Correctly restricts bit sizes to valid values (8, 16, 32, 64)
- Appropriately rejects 64-bit unsigned due to PHP's signed integer representation
- Maintains backward compatibility with existing code using
new Integer()ornew Integer(true)
151-178: LGTM!The validation logic is correct:
- Properly handles loose mode by converting numeric strings to numbers
- Correctly validates that the value is an integer type
- Range checking correctly uses the calculated min/max based on bit size and signedness
Summary
Adds configurable bit size and signed/unsigned support to the Integer validator, making it more flexible for different integer type requirements.
Changes
getBits(): Returns the configured bit sizeisUnsigned(): Returns whether the integer is unsignedgetFormat(): Returns OpenAPI/JSON Schema format string (e.g., "int8", "int32", "int64")getDescription()to reflect specific bit size and range constraintsUsage Examples
Backwards Compatibility
Fully backwards compatible - all existing uses of
new Integer()andnew Integer(true)continue to work with the default 32-bit signed behavior.Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.