Skip to content

Redesign Collection as immutable value object, simplify DSL#29

Merged
alganet merged 1 commit intoRespect:masterfrom
alganet:immutable
Apr 2, 2026
Merged

Redesign Collection as immutable value object, simplify DSL#29
alganet merged 1 commit intoRespect:masterfrom
alganet:immutable

Conversation

@alganet
Copy link
Copy Markdown
Member

@alganet alganet commented Apr 2, 2026

Replace the broad DSL surface (chaining, __get, ArrayAccess, proxy methods) with a single immutable constructor: name, with, filter, required. Remove Filtered, CollectionNotBound, filterColumns, and all mapper coupling from Collection.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.07%. Comparing base (9663392) to head (c46deeb).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #29      +/-   ##
============================================
+ Coverage     97.64%   98.07%   +0.42%     
+ Complexity      293      253      -40     
============================================
  Files            17       15       -2     
  Lines           595      520      -75     
============================================
- Hits            581      510      -71     
+ Misses           14       10       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Replace the broad DSL surface (chaining, __get, ArrayAccess, proxy
methods) with a single immutable constructor: name, with, filter,
required. Remove Filtered, CollectionNotBound, filterColumns, and
all mapper coupling from Collection.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Collection into an immutable value object and simplifies the public DSL by removing chaining/ArrayAccess/proxy methods and moving operations to AbstractMapper methods that take an explicit Collection spec.

Changes:

  • Replaced chained/ArrayAccess-based collection building with explicit immutable constructors (name, with, filter, required) and derive().
  • Removed Filtered collections and related column-filtering / mapper coupling, updating hydrators and iterators to traverse with.
  • Updated integration/unit tests and PHPStan config to match the new DSL and immutability model.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/Styles/Sakila/SakilaIntegrationTest.php Updates integration tests to build collections via mapper calls and pass them into fetch/fetchAll/persist.
tests/Styles/Plural/PluralIntegrationTest.php Same DSL migration for pluralized style fixtures.
tests/Styles/NorthWind/NorthWindIntegrationTest.php Same DSL migration for NorthWind naming conventions.
tests/Styles/CakePHP/CakePHPIntegrationTest.php Same DSL migration for CakePHP naming conventions.
tests/InMemoryMapper.php Aligns in-memory mapper to new filter/with model and removes filtered-column behavior.
tests/Hydrators/PrestyledAssocTest.php Updates collection construction to with arrays; removes Filtered-specific test coverage.
tests/Hydrators/NestedTest.php Updates nested hydrator tests to the new immutable with array composition.
tests/Collections/TypedTest.php Updates typed collections to the new constructor/derive model and adds derive preservation tests.
tests/Collections/FilteredTest.php Removes tests for the deleted Filtered collection type.
tests/Collections/CompositeTest.php Updates composite collections to new constructor/derive model and adds derive preservation tests.
tests/Collections/CollectionTest.php Rewrites tests around immutability, with, filter, derive, and deep cloning.
tests/CollectionIteratorTest.php Updates iterator traversal expectations to use with (no chaining/next links).
tests/AbstractMapperTest.php Updates mapper tests to use explicit collection specs and registered collection cloning/derivation.
src/Hydrators/PrestyledAssoc.php Removes Filtered handling and relies on CollectionIterator + name !== null.
src/Hydrators/Nested.php Traverses with children instead of connectsTo/children.
src/Collections/Typed.php Adds immutable constructor params and deriveArgs() so derive() preserves type.
src/Collections/Filtered.php Removes the Filtered collection implementation.
src/Collections/Composite.php Adds immutable constructor params and deriveArgs() so derive() preserves compositions.
src/Collections/Collection.php Replaces prior DSL surface with immutable with/filter/required model, derive(), and deep cloning.
src/CollectionNotBound.php Removes exception tied to the old “bound collection” proxy model.
src/CollectionIterator.php Iterates only over with children (no connectsTo concept).
src/AbstractMapper.php Removes mapper-binding/proxy collection APIs and implements registered-collection clone/derive via __call.
phpstan.neon.dist Updates ignore patterns for removed types/APIs and adds a broader suppression for test diagnostics.
Comments suppressed due to low confidence (2)

src/Collections/Composite.php:45

  • Composite::__callStatic() phpdoc restricts $arguments to array<int, array<string, list<string>>>, but Composite can now be called with additional args (with/filter/required) and named arguments. Update the phpdoc to reflect the actual accepted shapes (including string keys for named args) so static analysis matches runtime behavior.
    /** @param array<int, array<string, list<string>>> $arguments */
    public static function __callStatic(string $name, array $arguments): static
    {
        return new static($name, ...$arguments);
    }

src/Collections/Typed.php:59

  • Typed::__callStatic() phpdoc restricts $arguments to array<int, string>, but Typed can now be called with additional args (with/filter/required) and named arguments. Update the phpdoc to include these possibilities (and string keys for named args) to avoid misleading static analysis.
    /** @param array<int, string> $arguments */
    public static function __callStatic(string $name, array $arguments): static
    {
        return new static($name, ...$arguments);
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 83 to 86
@@ -148,56 +86,9 @@ public static function __callStatic(string $name, array $arguments): static
return new static($name, ...$arguments);
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

The __callStatic() phpdoc says $arguments is array<int, mixed>, but named arguments (e.g. Collection::foo(filter: 1)) arrive as string keys. Updating this to array<int|string, mixed> (and/or documenting expected keys like with, filter, required) will make static analysis and IDE hints accurate.

Copilot uses AI. Check for mistakes.
@alganet alganet marked this pull request as ready for review April 2, 2026 22:16
@alganet alganet merged commit c7ab409 into Respect:master Apr 2, 2026
7 checks passed
@alganet alganet deleted the immutable branch April 2, 2026 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants