Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PropertyInfo] Handle collection in PhpStan same as PhpDoc #57617

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Jul 1, 2024

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #52699 (comment)
License MIT

On #52699, a comment suggested to support classes that inherit from collection classes (such as doctrine collections for example).
This has been done for PHPDoc, but not in PHPStan.

This PR adds this missing behavior to PHPStan (and adds the missing PHPDoc related test).

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

I am not completely convinced that allowing subclasses in #52699 was a good decision as it rather looks like introducing a new feature in a patch release. But now that we did it for the PhpDocTypeHelper updating the PhpStanTypeHelper in the same way looks reasonable to me.

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

once CS fixed

@mtarld mtarld force-pushed the fix/property-info-phpstan-collection branch from 80a81f6 to f098683 Compare July 5, 2024 07:59
@xabbuh
Copy link
Member

xabbuh commented Jul 6, 2024

Thank you @mtarld.

@xabbuh xabbuh merged commit e67bfc1 into symfony:5.4 Jul 6, 2024
12 checks passed
@mtarld mtarld deleted the fix/property-info-phpstan-collection branch July 6, 2024 08:37
xabbuh added a commit to xabbuh/symfony that referenced this pull request Jul 9, 2024
This change ensures that the tests that have been added in symfony#57617 are
also applied when the types from the TypeInfo component are used instead
of the Type class that is built into the PropertyInfo component.
fabpot added a commit that referenced this pull request Jul 10, 2024
This PR was merged into the 7.1 branch.

Discussion
----------

[PropertyInfo]  add tests for TypeInfo types

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

This change ensures that the tests that have been added in #57617 are also applied when the types from the TypeInfo component are used instead of the Type class that is built into the PropertyInfo component.

Commits
-------

93d3209 add tests for TypeInfo types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants