Skip to content

fix: handle ChildrenAssociationField with self::class reference#35

Closed
Soner (shyim) wants to merge 1 commit into
mainfrom
fix/children-association-field-self-class
Closed

fix: handle ChildrenAssociationField with self::class reference#35
Soner (shyim) wants to merge 1 commit into
mainfrom
fix/children-association-field-self-class

Conversation

@shyim
Copy link
Copy Markdown
Member

Summary

  • DALDefinitionCollector threw RuntimeException: Cannot handle field ...ChildrenAssociationField with arguments [...] when an entity definition used new ChildrenAssociationField(self::class) (or any non-string reference argument).
  • Root cause: the collector tried to derive the field's property name from the constructor's first arg, but ChildrenAssociationField's property name is always 'children' (default), and self::class is a ClassConstFetch node without a value property — so it fell through to the generic branch and blew up.
  • Fix: hoist the ChildrenAssociationField check into the early special-case block alongside ChildCountField, ParentAssociationField, etc. The unreachable inner branch was removed.

Test plan

  • Added regression fixture children-association-self-class.php exercising new ChildrenAssociationField(self::class)
  • Verified the fixture reproduces the original error without the fix
  • vendor/bin/phpunit tests/Rule/BestPractise/DALDefinitionRuleTest.php — all 4 tests pass

The DAL definition collector threw "Cannot handle field" when a
ChildrenAssociationField was constructed with a non-string reference
argument like self::class. Hoist the field to the early special-case
block since its property name is always "children".
@lacknere
Copy link
Copy Markdown
Contributor

Soner (@shyim) the propertyName could be something else than children. The propertyName can optionally be passed as second parameter.
See #36

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.

2 participants