Skip to content

Modernize properties with PHP 8.4+ features and tighten visibility - #15

Merged
alganet merged 1 commit into
Respect:masterfrom
alganet:modernize
Mar 19, 2026
Merged

Modernize properties with PHP 8.4+ features and tighten visibility#15
alganet merged 1 commit into
Respect:masterfrom
alganet:modernize

Conversation

@alganet

@alganet alganet commented Mar 19, 2026

Copy link
Copy Markdown
Member
  • Replace trivial getters/setters with asymmetric visibility (public
    private(set)) on Collection, Filtered, Typed, Composite properties
  • Add virtual property hooks: Collection::$hasChildren, $more,
    Filtered::$identifierOnly, AbstractMapper::$style
  • Use constructor promotion with readonly for Typed::$type,
    Composite::$compositions, Filtered::$filters
  • Make NotPersistable a final readonly class
  • Cache tail node in Collection::$last for O(1) chain stacking
  • Tighten visibility: AbstractMapper::$collections and tryHydration()
    to private, Collection::findMapper()/resolveMapper() to private
  • Remove all trivial getters (getName, getCondition, getNext, getParent,
    getChildren, isRequired, getFilters, getType, getCompositions),
    setters (setMapper, setCondition, setParent, setRequired), and boolean
    methods (hasChildren, hasMore, isIdentifierOnly, getStyle)
  • Inline single-use variables and convert verbose conditionals to
    ternaries across Style classes
  • Exclude PSR2.Classes.PropertyDeclaration from PHPCS (4.0.x cannot
    parse property hooks)
  • Update all callers in src and tests to use direct property access

@alganet
alganet requested a review from Copilot March 19, 2026 02:48
@codecov-commenter

codecov-commenter commented Mar 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.83%. Comparing base (3f39ede) to head (cb6241b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/AbstractMapper.php 73.68% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master      #15      +/-   ##
============================================
- Coverage     96.13%   94.83%   -1.31%     
+ Complexity      157      144      -13     
============================================
  Files            13       13              
  Lines           388      329      -59     
============================================
- Hits            373      312      -61     
- Misses           15       17       +2     

☔ 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.

Copilot AI left a comment

Copy link
Copy Markdown

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 the collections/mapper API to use public read + restricted write properties (property hooks) instead of getter/setter methods, and updates tests and supporting utilities accordingly.

Changes:

  • Replace Collection/Typed/Filtered/Composite getter/setter methods with property access + constructors.
  • Update AbstractMapper, CollectionIterator, and the in-memory test mapper to use the new property-based API.
  • Simplify Style implementations and adjust PHPCS config to tolerate property hook syntax.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/InMemoryMapper.php Updates test mapper to use property access (name, condition, more, etc.) and $this->style.
tests/Collections/TypedTest.php Updates tests to assert against new properties (next, children, type, etc.).
tests/Collections/FilteredTest.php Updates tests to use new properties (filters, identifierOnly, etc.).
tests/Collections/CompositeTest.php Updates tests to use new properties (compositions, children, etc.).
tests/Collections/CollectionTest.php Updates tests for property-based API and removes reflection-based assertions tied to removed internals.
tests/CollectionIteratorTest.php Updates iterator-related tests to use children property.
tests/AbstractMapperTest.php Updates tests for collection registration, mapper/style access via properties.
src/Styles/Standard.php Refactors to more concise return expressions.
src/Styles/Plural.php Refactors to more concise return expressions.
src/Styles/NorthWind.php Refactors to more concise return expressions.
src/Styles/CakePHP.php Refactors to more concise return expressions.
src/NotPersistable.php Marks attribute class as readonly.
src/Collections/Typed.php Moves type into constructor + readonly property; updates entity resolution to use name property.
src/Collections/Filtered.php Moves filters into constructor + readonly property; adds computed identifierOnly property.
src/Collections/Composite.php Moves compositions into constructor + readonly property.
src/Collections/Collection.php Major refactor: property hooks, mapper resolution via parent chain, computed more/hasChildren, and tail traversal.
src/CollectionIterator.php Tightens types and switches traversal logic to new properties (more, children, next, name).
src/AbstractMapper.php Switches to property-based API (style, next, etc.), adjusts tracking storage typing, and refactors hydration helper.
phpcs.xml.dist Excludes PHPCS sniffs that can’t parse property hooks.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/Collections/Collection.php
Comment thread src/AbstractMapper.php
- Replace trivial getters/setters with asymmetric visibility (public
  private(set)) on Collection, Filtered, Typed, Composite properties
- Add virtual property hooks: Collection::$hasChildren, $more,
  Filtered::$identifierOnly, AbstractMapper::$style
- Use constructor promotion with readonly for Typed::$type,
  Composite::$compositions, Filtered::$filters
- Make NotPersistable a final readonly class
- Cache tail node in Collection::$last for O(1) chain stacking
- Tighten visibility: AbstractMapper::$collections and tryHydration()
  to private, Collection::findMapper()/resolveMapper() to private
- Remove all trivial getters (getName, getCondition, getNext, getParent,
  getChildren, isRequired, getFilters, getType, getCompositions),
  setters (setMapper, setCondition, setParent, setRequired), and boolean
  methods (hasChildren, hasMore, isIdentifierOnly, getStyle)
- Inline single-use variables and convert verbose conditionals to
  ternaries across Style classes
- Exclude PSR2.Classes.PropertyDeclaration from PHPCS (4.0.x cannot
  parse property hooks)
- Update all callers in src and tests to use direct property access
@alganet alganet changed the title WIP Modernize properties with PHP 8.4+ features and tighten visibility Mar 19, 2026
@alganet
alganet marked this pull request as ready for review March 19, 2026 03:17
@alganet
alganet merged commit 133ea72 into Respect:master Mar 19, 2026
3 checks passed
@alganet
alganet deleted the modernize branch March 19, 2026 03:18
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