fix(#473): add public getter methods to Range value objects
#477
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdded five public accessor methods to the Range value object to expose internal state (lower/upper bounds, bracket inclusivity, explicit empty flag), corresponding PHPUnit tests to validate these accessors across range types, and phpstan-ignore annotations in a few tests to silence static analysis for intentionally invalid test inputs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-08-24T23:39:18.782ZApplied to files:
📚 Learning: 2025-03-11T12:32:10.726ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (4)
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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php(2 hunks)tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.php(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the point[] array type should be documented as "point[]" not "_point" in the AVAILABLE-TYPES.md table, to be consistent with all other array types like text[], jsonb[], inet[], etc.
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
📚 Learning: 2025-08-24T23:39:18.782Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/LtreeInterface.php:7-7
Timestamp: 2025-08-24T23:39:18.782Z
Learning: This repository follows a concrete value object pattern without interfaces. All value objects (Point, DateRange, Int4Range, NumericRange, etc.) are concrete classes that directly implement required interfaces like \Stringable and \JsonSerializable. The codebase uses abstract base classes for shared functionality rather than interfaces for abstraction. Adding interfaces for value objects violates the established architectural patterns.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.phpsrc/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
📚 Learning: 2025-04-11T11:23:44.192Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 340
File: tests/MartinGeorgiev/Doctrine/DBAL/Types/InetArrayTest.php:145-145
Timestamp: 2025-04-11T11:23:44.192Z
Learning: In the PostgreSQL for Doctrine test cases, methods that test database-to-PHP conversions should use `mixed` type for parameter and include non-string test cases in their data providers, following the pattern in classes like InetTest, CidrTest, and MacaddrTest.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.phptests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
📚 Learning: 2025-08-24T16:52:32.488Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-24T16:52:32.488Z
Learning: This repository uses BaseType extension pattern for custom Doctrine DBAL types with PostgreSQL platform assertions, comprehensive unit and integration testing with data providers, and dedicated exception classes for type conversion errors.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.php
🧬 Code graph analysis (1)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.php (2)
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php (16)
Test(139-147)Test(149-156)Test(158-165)Test(231-239)Test(241-249)Test(251-263)Test(265-272)Test(274-282)Test(284-299)Test(301-323)Test(325-330)Test(351-356)createSimpleRange(18-24)createInfiniteRange(36-39)createInclusiveRange(41-49)createEmptyRange(31-34)src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.php (5)
getLower(151-154)getUpper(159-162)isLowerBracketInclusive(164-167)isUpperBracketInclusive(169-172)isExplicitlyEmpty(174-177)
🪛 GitHub Actions: CI
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.php
[error] 116-116: phpdoc_to_comment: convert /** @phpstan-ignore-next-line ... / to / @phpstan-ignore-next-line ... */.
[error] 126-126: phpdoc_to_comment: convert /** @phpstan-ignore-next-line ... / to / @phpstan-ignore-next-line ... */.
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.php
[error] 148-156: PHP CS Fixer would modify return type declarations: getLower()/getUpper() return types changed from 'int|float|\DateTimeInterface|null' to '\DateTimeInterface|float|int|null'.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
[error] 234-234: phpdoc_to_comment: convert /** @phpstan-ignore-next-line ... / to / @phpstan-ignore-next-line ... */.
[error] 244-244: phpdoc_to_comment: convert /** @phpstan-ignore-next-line ... / to / @phpstan-ignore-next-line ... */.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PHP 8.5 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.5 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.5 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (2)
src/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/Range.php (1)
26-36: LGTM: Constructor documentation added.The constructor parameter docblock provides helpful documentation for the generic type bounds.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/BaseRangeTestCase.php (1)
113-166: LGTM: Comprehensive test coverage for new getter methods.The new test methods thoroughly verify all five getter methods added to the Range class:
- Lower/upper bound retrieval for simple ranges
- Null bounds for infinite ranges
- Bracket inclusivity states for both simple and inclusive ranges
- Explicit empty state distinction
The test implementations are clear, well-structured, and provide appropriate coverage.
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/DateRangeTest.php
Outdated
Show resolved
Hide resolved
tests/Unit/MartinGeorgiev/Doctrine/DBAL/Types/ValueObject/NumericRangeTest.php
Outdated
Show resolved
Hide resolved
Contributor
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests