Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Form] fix some type annotations #42012

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

Merged
merged 1 commit into from
Jul 19, 2021
Merged

[Form] fix some type annotations #42012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions 4 src/Symfony/Component/Form/Button.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ public function handleRequest($request = null)
/**
* Submits data to the button.
*
* @param string|null $submittedData Not used
* @param bool $clearMissing Not used
* @param array|string|null $submittedData Not used
xabbuh marked this conversation as resolved.
Show resolved Hide resolved
* @param bool $clearMissing Not used
*
* @return $this
*
Expand Down
78 changes: 0 additions & 78 deletions 78 src/Symfony/Component/Form/ButtonBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ public function create($name, $type = null, array $options = [])
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param string $name
*
* @throws BadMethodCallException
*/
public function get($name)
Expand All @@ -115,10 +111,6 @@ public function get($name)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param string $name
*
* @throws BadMethodCallException
*/
public function remove($name)
Expand All @@ -129,8 +121,6 @@ public function remove($name)
/**
* Unsupported method.
*
* @param string $name
*
* @return bool Always returns false
*/
public function has($name)
Expand Down Expand Up @@ -161,12 +151,6 @@ public function getForm()
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param string $eventName
* @param callable $listener
* @param int $priority
*
* @throws BadMethodCallException
*/
public function addEventListener($eventName, $listener, $priority = 0)
Expand All @@ -177,8 +161,6 @@ public function addEventListener($eventName, $listener, $priority = 0)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @throws BadMethodCallException
*/
public function addEventSubscriber(EventSubscriberInterface $subscriber)
Expand All @@ -189,10 +171,6 @@ public function addEventSubscriber(EventSubscriberInterface $subscriber)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $forcePrepend
*
* @throws BadMethodCallException
*/
public function addViewTransformer(DataTransformerInterface $viewTransformer, $forcePrepend = false)
Expand All @@ -203,8 +181,6 @@ public function addViewTransformer(DataTransformerInterface $viewTransformer, $f
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @throws BadMethodCallException
*/
public function resetViewTransformers()
Expand All @@ -215,10 +191,6 @@ public function resetViewTransformers()
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $forceAppend
*
* @throws BadMethodCallException
*/
public function addModelTransformer(DataTransformerInterface $modelTransformer, $forceAppend = false)
Expand All @@ -229,8 +201,6 @@ public function addModelTransformer(DataTransformerInterface $modelTransformer,
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @throws BadMethodCallException
*/
public function resetModelTransformers()
Expand Down Expand Up @@ -261,8 +231,6 @@ public function setAttributes(array $attributes)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @throws BadMethodCallException
*/
public function setDataMapper(DataMapperInterface $dataMapper = null)
Expand All @@ -287,10 +255,6 @@ public function setDisabled($disabled)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param mixed $emptyData
*
* @throws BadMethodCallException
*/
public function setEmptyData($emptyData)
Expand All @@ -301,10 +265,6 @@ public function setEmptyData($emptyData)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $errorBubbling
*
* @throws BadMethodCallException
*/
public function setErrorBubbling($errorBubbling)
Expand All @@ -315,10 +275,6 @@ public function setErrorBubbling($errorBubbling)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $required
*
* @throws BadMethodCallException
*/
public function setRequired($required)
Expand All @@ -329,10 +285,6 @@ public function setRequired($required)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param null $propertyPath
*
* @throws BadMethodCallException
*/
public function setPropertyPath($propertyPath)
Expand All @@ -343,10 +295,6 @@ public function setPropertyPath($propertyPath)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $mapped
*
* @throws BadMethodCallException
*/
public function setMapped($mapped)
Expand All @@ -357,10 +305,6 @@ public function setMapped($mapped)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $byReference
*
* @throws BadMethodCallException
*/
public function setByReference($byReference)
Expand All @@ -371,10 +315,6 @@ public function setByReference($byReference)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $compound
*
* @throws BadMethodCallException
*/
public function setCompound($compound)
Expand All @@ -397,10 +337,6 @@ public function setType(ResolvedFormTypeInterface $type)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param mixed $data
*
* @throws BadMethodCallException
*/
public function setData($data)
Expand All @@ -411,10 +347,6 @@ public function setData($data)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @param bool $locked
*
* @throws BadMethodCallException
*/
public function setDataLocked($locked)
Expand All @@ -425,8 +357,6 @@ public function setDataLocked($locked)
/**
* Unsupported method.
*
* This method should not be invoked.
*
* @throws BadMethodCallException
*/
public function setFormFactory(FormFactoryInterface $formFactory)
Expand All @@ -437,8 +367,6 @@ public function setFormFactory(FormFactoryInterface $formFactory)
/**
* Unsupported method.
*
* @param string $action
*
* @throws BadMethodCallException
*/
public function setAction($action)
Expand All @@ -449,8 +377,6 @@ public function setAction($action)
/**
* Unsupported method.
*
* @param string $method
*
* @throws BadMethodCallException
*/
public function setMethod($method)
Expand All @@ -471,8 +397,6 @@ public function setRequestHandler(RequestHandlerInterface $requestHandler)
/**
* Unsupported method.
*
* @param bool $initialize
*
* @return $this
*
* @throws BadMethodCallException
Expand All @@ -489,8 +413,6 @@ public function setAutoInitialize($initialize)
/**
* Unsupported method.
*
* @param bool $inheritData
*
* @throws BadMethodCallException
*/
public function setInheritData($inheritData)
Expand Down
38 changes: 18 additions & 20 deletions 38 src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
use Symfony\Component\PropertyAccess\PropertyPath;
use Symfony\Component\PropertyAccess\PropertyPathInterface;

/**
* Adds property path support to a choice list factory.
Expand Down Expand Up @@ -59,19 +60,17 @@ public function getDecoratedFactory()
/**
* {@inheritdoc}
*
* @param iterable $choices The choices
* @param callable|string|PropertyPath|null $value The callable or path for
* generating the choice values
* @param mixed $value
*
* @return ChoiceListInterface The choice list
* @return ChoiceListInterface
*/
public function createListFromChoices($choices, $value = null)
{
if (\is_string($value)) {
$value = new PropertyPath($value);
}

if ($value instanceof PropertyPath) {
if ($value instanceof PropertyPathInterface) {
$accessor = $this->propertyAccessor;
$value = function ($choice) use ($accessor, $value) {
// The callable may be invoked with a non-object/array value
Expand All @@ -88,18 +87,17 @@ public function createListFromChoices($choices, $value = null)
/**
* {@inheritdoc}
*
* @param callable|string|PropertyPath|null $value The callable or path for
* generating the choice values
* @param mixed $value
*
* @return ChoiceListInterface The choice list
* @return ChoiceListInterface
*/
public function createListFromLoader(ChoiceLoaderInterface $loader, $value = null)
{
if (\is_string($value)) {
$value = new PropertyPath($value);
}

if ($value instanceof PropertyPath) {
if ($value instanceof PropertyPathInterface) {
$accessor = $this->propertyAccessor;
$value = function ($choice) use ($accessor, $value) {
// The callable may be invoked with a non-object/array value
Expand All @@ -116,13 +114,13 @@ public function createListFromLoader(ChoiceLoaderInterface $loader, $value = nul
/**
* {@inheritdoc}
*
* @param array|callable|string|PropertyPath|null $preferredChoices The preferred choices
* @param callable|string|PropertyPath|null $label The callable or path generating the choice labels
* @param callable|string|PropertyPath|null $index The callable or path generating the view indices
* @param callable|string|PropertyPath|null $groupBy The callable or path generating the group names
* @param array|callable|string|PropertyPath|null $attr The callable or path generating the HTML attributes
* @param mixed $preferredChoices
* @param mixed $label
* @param mixed $index
* @param mixed $groupBy
* @param mixed $attr
*
* @return ChoiceListView The choice list view
* @return ChoiceListView
*/
public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

I mean using mixed for a decorator makes sense as the class does not know what it decorates that might have widened the range of accepted parameters types.
But in theory this applies to all arguments. So in theory you would also need to remove the type from ChoiceListInterface $list and accept mixed there. This might go to far. But it's a thought expirement.

Copy link
Member Author

Choose a reason for hiding this comment

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

This might go to far. But it's a thought experiment.

I agree with this :)

{
Expand All @@ -132,7 +130,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
$label = new PropertyPath($label);
}

if ($label instanceof PropertyPath) {
if ($label instanceof PropertyPathInterface) {
$label = function ($choice) use ($accessor, $label) {
return $accessor->getValue($choice, $label);
};
Expand All @@ -142,7 +140,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
$preferredChoices = new PropertyPath($preferredChoices);
}

if ($preferredChoices instanceof PropertyPath) {
if ($preferredChoices instanceof PropertyPathInterface) {
$preferredChoices = function ($choice) use ($accessor, $preferredChoices) {
try {
return $accessor->getValue($choice, $preferredChoices);
Expand All @@ -157,7 +155,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
$index = new PropertyPath($index);
}

if ($index instanceof PropertyPath) {
if ($index instanceof PropertyPathInterface) {
$index = function ($choice) use ($accessor, $index) {
return $accessor->getValue($choice, $index);
};
Expand All @@ -167,7 +165,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
$groupBy = new PropertyPath($groupBy);
}

if ($groupBy instanceof PropertyPath) {
if ($groupBy instanceof PropertyPathInterface) {
$groupBy = function ($choice) use ($accessor, $groupBy) {
try {
return $accessor->getValue($choice, $groupBy);
Expand All @@ -182,7 +180,7 @@ public function createView(ChoiceListInterface $list, $preferredChoices = null,
$attr = new PropertyPath($attr);
}

if ($attr instanceof PropertyPath) {
if ($attr instanceof PropertyPathInterface) {
$attr = function ($choice) use ($accessor, $attr) {
return $accessor->getValue($choice, $attr);
};
Expand Down
12 changes: 10 additions & 2 deletions 12 src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,16 @@ private static function resolveValidationGroups($groups, FormInterface $form)

private static function getConstraintsInGroups($constraints, $group)
{
return array_filter($constraints, static function (Constraint $constraint) use ($group) {
return \in_array($group, $constraint->groups, true);
$groups = (array) $group;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is coming from \Symfony\Component\Validator\Constraints\GroupSequence::$groups and according to it's type, $group itself could also be a GroupSequence again. So there is still something not right.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true, yet nobody reported this as a bug ever, so that the urgency on this is zero on my side :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Using a nested array seems to be the same as using a nested GroupSequence. If the nested group sequence is not supported, we probably should just remove the type from GroupSequence::$groups. It's also not documented in https://symfony.com/doc/current/validation/sequence_provider.html


return array_filter($constraints, static function (Constraint $constraint) use ($groups) {
foreach ($groups as $group) {
if (\in_array($group, $constraint->groups, true)) {
return true;
}
}

return false;
});
}
}
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.