Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PropertyInfo] Use iterators for PropertyInfoExtractor#21654
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| * @param array $extractors | ||
| * @param string $method | ||
| * @param array $arguments | ||
| * @param array|\Traversable $extractors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
iterable would describe it 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
can we use it in php docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, and wealready did :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
great! updated
stof commentedFeb 17, 2017
👍 |
stof commentedFeb 17, 2017
Actually, you need to add a conflict rule in FrameworkBundle, to forbid it to use an older property-info component (which would have an array typehint), if we don't have the conflict already |
GuilhemN commentedFeb 17, 2017
Indeed, thanks! |
dunglas commentedFeb 17, 2017
👍 |
nicolas-grekas commentedFeb 18, 2017
Thank you@GuilhemN. |
… (GuilhemN)This PR was merged into the 3.3-dev branch.Discussion----------[PropertyInfo] Use iterators for PropertyInfoExtractor| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Most of the time, when using the cache, the property info extractors are not used: the new iterator feature looks perfect to prevent their instantiation.Commits-------38523a9 [PropertyInfo] Use iterators for PropertyInfoExtractor
| * @param PropertyTypeExtractorInterface[] $typeExtractors | ||
| * @param PropertyDescriptionExtractorInterface[] $descriptionExtractors | ||
| * @param PropertyAccessExtractorInterface[] $accessExtractors | ||
| * @paramiterable|PropertyListExtractorInterface[] $listExtractors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
iterable is not a valid keyword, even according to the (still)draft PSR-5. Seehttps://github.com/phpDocumentor/fig-standards/issues/141
But anyway I think it's redundant to write it like this, it should be simplyiterable<PropertyListExtractorInterface>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nor is@final, its support will come when it will be adopted so imo it's not a problem.
About the redundancy I agree but we would lose all completion until editors add generics support, I don't think it's acceptable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Anyway, the RFC for introducing aniterable keyword has been accepted and PHP 7.1 supports this type:https://wiki.php.net/rfc/iterable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@dunglas That's for PHP, not PHPDoc 😄
Most of the time, when using the cache, the property info extractors are not used: the new iterator feature looks perfect to prevent their instantiation.