8000 [FrameworkBundle] Execute the PhpDocExtractor earlier · Issue #21367 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[FrameworkBundle] Execute the PhpDocExtractor earlier #21367

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.

Sign up for GitHub

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

Jump to bottom
Closed
GuilhemN opened this issue Jan 20, 2017 · 9 comments
Closed

[FrameworkBundle] Execute the PhpDocExtractor earlier #21367

GuilhemN opened this issue Jan 20, 2017 · 9 comments
Labels
Bug FrameworkBundle Status: Needs Review

Comments

@GuilhemN
Copy link
Contributor
GuilhemN commented Jan 20, 2017 •
edited
Loading
Q A
Bug report? yes, kind of
Feature request? no
BC Break report? no
RFC? no
Symfony version 2.8 -> master

I wonder if this is logical to execute the PhpDocExtractor after the ReflectionExtractor: when you use phpdocs it's because they are more precise than php type hints.
This causes an issue in NelmioApiDocBundle, for example you can't use int[] with a setter as the type mixed[] will be returned instead of int[].

Would you accept bumping its priority to -999?

cc @dunglas
@GuilhemN GuilhemN mentioned this issue Jan 20, 2017
Closed
@ogizanagi
Copy link
Contributor

👍 I agree it's more logical and the order I personally rely on in my own projects. This is also the order described on the documentation page: http://symfony.com/doc/current/components/property_info.html

@GuilhemN
Copy link
Contributor Author

Apparently the DoctrineExtractor already has a priority of -999, so I'd say it's safer to change the priority of the ReflectionExtractor to -1002 or lower.

@dunglas
Copy link
Member
dunglas commented Jan 21, 2017

It was done on purpose because parsing PHPDoc is extremely slow. On the other hand using reflection is fast.
But we can reconsider it.

@GuilhemN GuilhemN mentioned this issue Jan 21, 2017
Closed
@GuilhemN
Copy link
Contributor Author

see #21370

@magarzon
Copy link

And if you could choose the priority/order and/or enable/disable each extractor in the FrameworkBundle configuration, it'd be a blast.

My use case: I make calls to an API and deserialize the response into model objects using the Serializer (and PropertyInfo component). So I don't need DoctrineExtractor at all, but it's instantiated and added as first option anyway.

Second issue: I have a class with a boolean field "active" and its accesor "isActive", but the response from the API is 0 or 1. The ReflectionExtractor only try to get the type using ReflectionMethod::getReturnType if you use PHP 7.x, but I'm using PHP 5.7, so it fallbacks to check if the accesor starts with "is" to decide that it's a boolean, and I get and exception (because is_bool(1) returns false). With the PhpDocExtractor I could use int|bool as expected types.

Yes, I can change the name of the accesor to getActive, but I'd prefer if I could decide which extractors use

@GuilhemN
Copy link
Contributor Author

@magarzon you can remove the extractors you don't need using $container->removeDefinition('the_extractor_i_dont_want').

fabpot added a commit that referenced this issue Jan 30, 2017
@fabpot
bug #21370 [FrameworkBundle] Execute the PhpDocExtractor earlier (Gui…
f4693be 
…lhemN)

This PR was submitted for the master branch but it was merged into the 2.8 branch instead (closes #21370).

Discussion
----------

[FrameworkBundle] Execute the PhpDocExtractor earlier

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes, but safer to apply on master
| New feature?  | no
| BC breaks?    | is changing a priority a bc break?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21367
| License       | MIT
| Doc PR        |

Fixes #21367.

> I wonder if this is logical to execute the PhpDocExtractor after the ReflectionExtractor: when you use phpdocs it's because they are more precise than php type hints.
This causes an issue in NelmioApiDocBundle, for example you can't use int[] with a setter as the type mixed[] will be returned instead of int[].
>
> ~~Would you accept bumping its priority to -999?~~

This PR changes the priority of the `ReflectionExtractor` to `-1002` to make sure it is executed after the `PhpDocExtractor`.

Commits
-------

0425e05 [FrameworkBundle] Execute the PhpDocExtractor earlier
@fabpot fabpot closed this as completed Jan 30, 2017
@teohhanhui
Copy link
Contributor

This has caused BC break in my app. 😞

@xabbuh
Copy link
Member
xabbuh commented Mar 29, 2017

@teohhanhui Please open a new issue describing the BC break you experience. Here your comment will likely get lost.

@magnetik magnetik mentioned this issue Jul 16, 2018
Open
@faizanakram99
Copy link
Contributor

@magarzon you can remove the extractors you don't need using $container->removeDefinition('the_extractor_i_dont_want').

it doesn't work, without doctrine extractor, property info breaks, it somehow still references it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug FrameworkBundle Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[FrameworkBundle] Execute the PhpDocExtractor earlier GuilhemN/symfony
10 participants
@fabpot @dunglas @javiereguiluz @magarzon @teohhanhui @xabbuh @ogizanagi @GuilhemN @faizanakram99 @carsonbot
0