Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Form] Add a TypeGuesser that use typed property reflection#47450
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
base:7.4
Are you sure you want to change the base?
Conversation
e9ab555 to448b249Comparecarsonbot commentedSep 2, 2022
Hey! I think@alamirault has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
| ['string', new TypeGuess(TextType::class, [], Guess::LOW_CONFIDENCE)], | ||
| ['nullable', new TypeGuess(TextType::class, [], Guess::LOW_CONFIDENCE)], | ||
| ['suit', new TypeGuess(EnumType::class, ['class' => Suit::class], Guess::MEDIUM_CONFIDENCE)], | ||
| ['date', new TypeGuess(DateTimeType::class, ['input' => 'datetime_immutable'], Guess::LOW_CONFIDENCE)], |
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.
we should also add a test case for a type where the guess would benull
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.
I've added one case with a property typed with an unknown class, and one with an untyped property.
Uh oh!
There was an error while loading.Please reload this page.
448b249 to884d0a3Comparesrc/Symfony/Component/Form/Tests/Extension/Core/ReflectionTypeGuesserTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Form/Tests/Extension/Core/ReflectionTypeGuesserTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
5d2411a toac8a91dCompare| { | ||
| public function guessType(string $class, string $property): ?TypeGuess | ||
| { | ||
| $type = $this->getReflectionType($class, $property); |
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.
Shouldn't we use PropertyInfo instead of using reflection directly? This will allow supporting Reflection but also PHPDoc/PHPStan types as well as other metadata sources.
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.
I thought about using PropertyInfo but decided not to because I wanted the guesser to be the simplest possible and not depend on another component (this could be important when not using the full stack framework).
3f14eac tofbf29b9Comparefbf29b9 toa1b6252Compare
When using form on objects that are not doctrine entities, you have to explicitly define all types that are used on forms.
This PR adds a type guesser that uses reflection information from typed properties to guess form types.