Skip to content

Commit 6ee3efa

Browse files
fabpotfabpot
committed
minor #32066 Fine tune constructor types (Tobion)
This PR was merged into the 4.4 branch. Discussion ---------- Fine tune constructor types | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Fine tunes some constructor types that have been added in #24722 Form names as integer was only a workaround as forms names are used as array keys which get transformed to int. So it was added as a workaround in #6355 (comment) With typehints added in #24722 those were mostly auto-cast anyway, e.g. in FormBuilder. There are only a few integer form names remaining documented, in the main entry points of the Form component (`\Symfony\Component\Form\FormInterface::add`). Internally it's always a string now. So I could remove some int docs which also fixes #30032 what @xabbuh tried to do. Some of these changes we're just not done before because of broken tests. It's mainly a missing explicit mock for `TranslationInterface::trans` which returned null. If we had return types hints in interfaces, this wouldn't happen. Commits ------- 507794a Fine tune constructor types
2 parents a0aa941 + 507794a commit 6ee3efa

File tree

17 files changed

+39
-41
lines changed

17 files changed

+39
-41
lines changed

‎src/Symfony/Component/Config/Definition/BaseNode.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ abstract class BaseNode implements NodeInterface
4747
*/
4848
public function __construct(?string $name, NodeInterface $parent = null, string $pathSeparator = self::DEFAULT_PATH_SEPARATOR)
4949
{
50-
if (false !== strpos($name = (string) $name, $pathSeparator)) {
50+
if (null === $name) {
51+
$name = '';
52+
}
53+
if (false !== strpos($name, $pathSeparator)) {
5154
throw new \InvalidArgumentException('The name must not contain "'.$pathSeparator.'".');
5255
}
5356

‎src/Symfony/Component/Config/Definition/Builder/NodeDefinition.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ abstract class NodeDefinition implements NodeParentInterface
4141
public function __construct(?string $name, NodeParentInterface $parent = null)
4242
{
4343
$this->parent = $parent;
44-
$this->name = $name;
44+
$this->name = $name ?? '';
4545
}
4646

4747
/**

‎src/Symfony/Component/Config/Resource/GlobResource.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface
3939
*
4040
* @throws \InvalidArgumentException
4141
*/
42-
public function __construct(?string $prefix, string $pattern, bool $recursive, bool $forExclusion = false, array $excludedPrefixes = [])
42+
public function __construct(string $prefix, string $pattern, bool $recursive, bool $forExclusion = false, array $excludedPrefixes = [])
4343
{
4444
$this->prefix = realpath($prefix) ?: (file_exists($prefix) ? $prefix : false);
4545
$this->pattern = $pattern;

‎src/Symfony/Component/Config/Tests/Loader/FileLoaderTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public function testImportWithFileLocatorDelegation()
7171
public function testImportWithGlobLikeResource()
7272
{
7373
$locatorMock = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
74+
$locatorMock->expects($this->once())->method('locate')->willReturn('');
7475
$loader = new TestFileLoader($locatorMock);
7576

7677
$this->assertSame('[foo]', $loader->import('[foo]'));
@@ -79,6 +80,7 @@ public function testImportWithGlobLikeResource()
7980
public function testImportWithNoGlobMatch()
8081
{
8182
$locatorMock = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
83+
$locatorMock->expects($this->once())->method('locate')->willReturn('');
8284
$loader = new TestFileLoader($locatorMock);
8385

8486
$this->assertNull($loader->import('./*.abc'));

‎src/Symfony/Component/DomCrawler/AbstractUriElement.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ abstract class AbstractUriElement
2424
protected $node;
2525

2626
/**
27-
* @var string The method to use for the element
27+
* @var string|null The method to use for the element
2828
*/
2929
protected $method;
3030

@@ -36,7 +36,7 @@ abstract class AbstractUriElement
3636
/**
3737
* @param \DOMElement $node A \DOMElement instance
3838
* @param string $currentUri The URI of the page where the link is embedded (or the base href)
39-
* @param string $method The method to use for the link (GET by default)
39+
* @param string|null $method The method to use for the link (GET by default)
4040
*
4141
* @throws \InvalidArgumentException if the node is not a link
4242
*/
@@ -70,7 +70,7 @@ public function getNode()
7070
*/
7171
public function getMethod()
7272
{
73-
return $this->method;
73+
return $this->method ?? 'GET';
7474
}
7575

7676
/**

‎src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public function validate($form, Constraint $formConstraint)
137137
// Mark the form with an error if it contains extra fields
138138
if (!$config->getOption('allow_extra_fields') && \count($form->getExtraData()) > 0) {
139139
$this->context->setConstraint($formConstraint);
140-
$this->context->buildViolation($config->getOption('extra_fields_message'))
140+
$this->context->buildViolation($config->getOption('extra_fields_message', ''))
141141
->setParameter('{{ extra_fields }}', '"'.implode('", "', array_keys($form->getExtraData())).'"')
142142
->setInvalidValue($form->getExtraData())
143143
->setCode(Form::NO_SUCH_FIELD_ERROR)

‎src/Symfony/Component/Form/Form.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class Form implements \IteratorAggregate, FormInterface, ClearableErrorsInterfac
146146
private $lockSetData = false;
147147

148148
/**
149-
* @var string|int|null
149+
* @var string|null
150150
*/
151151
private $name;
152152

@@ -847,6 +847,8 @@ public function add($child, $type = null, array $options = [])
847847
throw new UnexpectedTypeException($child, 'string, integer or Symfony\Component\Form\FormInterface');
848848
}
849849

850+
$child = (string) $child;
851+
850852
if (null !== $type && !\is_string($type) && !$type instanceof FormTypeInterface) {
851853
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
852854
}

‎src/Symfony/Component/Form/FormConfigBuilder.php

+9-11
Original file line numberDiff line numberDiff line change
@@ -107,23 +107,23 @@ class FormConfigBuilder implements FormConfigBuilderInterface
107107
/**
108108
* Creates an empty form configuration.
109109
*
110-
* @param string|int $name The form name
110+
* @param string|null $name The form name
111111
* @param string|null $dataClass The class of the form's data
112112
* @param EventDispatcherInterface $dispatcher The event dispatcher
113113
* @param array $options The form options
114114
*
115115
* @throws InvalidArgumentException if the data class is not a valid class or if
116116
* the name contains invalid characters
117117
*/
118-
public function __construct($name, ?string $dataClass, EventDispatcherInterface $dispatcher, array $options = [])
118+
public function __construct(?string $name, ?string $dataClass, EventDispatcherInterface $dispatcher, array $options = [])
119119
{
120120
self::validateName($name);
121121

122122
if (null !== $dataClass && !class_exists($dataClass) && !interface_exists($dataClass, false)) {
123123
throw new InvalidArgumentException(sprintf('Class "%s" not found. Is the "data_class" form option set correctly?', $dataClass));
124124
}
125125

126-
$this->name = (string) $name;
126+
$this->name = $name ?? '';
127127
$this->dataClass = $dataClass;
128128
$this->dispatcher = $dispatcher;
129129
$this->options = $options;
@@ -767,15 +767,17 @@ public function getFormConfig()
767767
/**
768768
* Validates whether the given variable is a valid form name.
769769
*
770-
* @param string|int|null $name The tested form name
770+
* @param string|null $name The tested form name
771771
*
772772
* @throws UnexpectedTypeException if the name is not a string or an integer
773773
* @throws InvalidArgumentException if the name contains invalid characters
774+
*
775+
* @internal since Symfony 4.4
774776
*/
775777
public static function validateName($name)
776778
{
777-
if (null !== $name && !\is_string($name) && !\is_int($name)) {
778-
throw new UnexpectedTypeException($name, 'string, integer or null');
779+
if (null !== $name && !\is_string($name)) {
780+
throw new UnexpectedTypeException($name, 'string or null');
779781
}
780782

781783
if (!self::isValidName($name)) {
@@ -792,12 +794,8 @@ public static function validateName($name)
792794
* * starts with a letter, digit or underscore
793795
* * contains only letters, digits, numbers, underscores ("_"),
794796
* hyphens ("-") and colons (":")
795-
*
796-
* @param string|null $name The tested form name
797-
*
798-
* @return bool Whether the name is valid
799797
*/
800-
public static function isValidName($name)
798+
final public static function isValidName(?string $name): bool
801799
{
802800
return '' === $name || null === $name || preg_match('/^[a-zA-Z0-9_][a-zA-Z0-9_\-:]*$/D', $name);
803801
}

‎src/Symfony/Component/Form/FormError.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ class FormError
4747
*
4848
* @see \Symfony\Component\Translation\Translator
4949
*/
50-
public function __construct(?string $message, string $messageTemplate = null, array $messageParameters = [], int $messagePluralization = null, $cause = null)
50+
public function __construct(string $message, string $messageTemplate = null, array $messageParameters = [], int $messagePluralization = null, $cause = null)
5151
{
52-
$this->message = (string) $message;
52+
$this->message = $message;
5353
$this->messageTemplate = $messageTemplate ?: $message;
5454
$this->messageParameters = $messageParameters;
5555
$this->messagePluralization = $messagePluralization;

‎src/Symfony/Component/Form/FormFactory.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function createNamedBuilder($name, $type = 'Symfony\Component\Form\Extens
7373

7474
$type = $this->registry->getType($type);
7575

76-
$builder = $type->createBuilder($this, $name, $options);
76+
$builder = $type->createBuilder($this, (string) $name, $options);
7777

7878
// Explicitly call buildForm() in order to be able to override either
7979
// createBuilder() or buildForm() in the resolved form type

‎src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php

+2-6
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ protected function setUp()
4646
{
4747
$this->tokenManager = $this->getMockBuilder(CsrfTokenManagerInterface::class)->getMock();
4848
$this->translator = $this->getMockBuilder(TranslatorInterface::class)->getMock();
49+
$this->translator->expects($this->any())->method('trans')->willReturnArgument(0);
4950

5051
parent::setUp();
5152
}
@@ -371,16 +372,11 @@ public function testsTranslateCustomErrorMessage()
371372
->with($csrfToken)
372373
->willReturn(false);
373374

374-
$this->translator->expects($this->once())
375-
->method('trans')
376-
->with('Foobar')
377-
->willReturn('[trans]Foobar[/trans]');
378-
379375
$form = $this->factory
380376
->createBuilder('Symfony\Component\Form\Extension\Core\Type\FormType', null, [
381377
'csrf_field_name' => 'csrf',
382378
'csrf_token_manager' => $this->tokenManager,
383-
'csrf_message' => 'Foobar',
379+
'csrf_message' => '[trans]Foobar[/trans]',
384380
'csrf_token_id' => 'TOKEN_ID',
385381
'compound' => true,
386382
])

‎src/Symfony/Component/Form/Tests/FormConfigTest.php

-5
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,6 @@ public function getHtml4Ids()
5757
[123],
5858
// NULL is allowed
5959
[null],
60-
// Other types are not
61-
[1.23, 'Symfony\Component\Form\Exception\UnexpectedTypeException'],
62-
[5., 'Symfony\Component\Form\Exception\UnexpectedTypeException'],
63-
[true, 'Symfony\Component\Form\Exception\UnexpectedTypeException'],
64-
[new \stdClass(), 'Symfony\Component\Form\Exception\UnexpectedTypeException'],
6560
];
6661
}
6762

‎src/Symfony/Component/HttpFoundation/RedirectResponse.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class RedirectResponse extends Response
3232
*
3333
* @see http://tools.ietf.org/html/rfc2616#section-10.3
3434
*/
35-
public function __construct(?string $url, int $status = 302, array $headers = [])
35+
public function __construct(string $url, int $status = 302, array $headers = [])
3636
{
3737
parent::__construct('', $status, $headers);
3838

@@ -82,7 +82,7 @@ public function getTargetUrl()
8282
*/
8383
public function setTargetUrl($url)
8484
{
85-
if (empty($url)) {
85+
if ('' == $url) {
8686
throw new \InvalidArgumentException('Cannot redirect to an empty URL.');
8787
}
8888

‎src/Symfony/Component/HttpFoundation/Tests/RedirectResponseTest.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ public function testGenerateMetaRedirect()
2828

2929
/**
3030
* @expectedException \InvalidArgumentException
31+
* @expectedExceptionMessage Cannot redirect to an empty URL.
3132
*/
32-
public function testRedirectResponseConstructorNullUrl()
33+
public function testRedirectResponseConstructorEmptyUrl()
3334
{
34-
$response = new RedirectResponse(null);
35+
$response = new RedirectResponse('');
3536
}
3637

3738
/**

‎src/Symfony/Component/Validator/ConstraintViolation.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ class ConstraintViolation implements ConstraintViolationInterface
4444
* violation
4545
* @param int|null $plural The number for determining the plural
4646
* form when translating the message
47-
* @param mixed $code The error code of the violation
47+
* @param string|null $code The error code of the violation
4848
* @param Constraint|null $constraint The constraint whose validation
4949
* caused the violation
5050
* @param mixed $cause The cause of the violation
5151
*/
52-
public function __construct(?string $message, ?string $messageTemplate, array $parameters, $root, ?string $propertyPath, $invalidValue, int $plural = null, $code = null, Constraint $constraint = null, $cause = null)
52+
public function __construct(string $message, ?string $messageTemplate, array $parameters, $root, ?string $propertyPath, $invalidValue, int $plural = null, $code = null, Constraint $constraint = null, $cause = null)
5353
{
5454
$this->message = $message;
5555
$this->messageTemplate = $messageTemplate;

‎src/Symfony/Component/Validator/Test/ConstraintValidatorTestCase.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ protected function restoreDefaultTimezone()
9999
protected function createContext()
100100
{
101101
$translator = $this->getMockBuilder(TranslatorInterface::class)->getMock();
102+
$translator->expects($this->any())->method('trans')->willReturnArgument(0);
102103
$validator = $this->getMockBuilder('Symfony\Component\Validator\Validator\ValidatorInterface')->getMock();
103104
$contextualValidator = $this->getMockBuilder('Symfony\Component\Validator\Validator\ContextualValidatorInterface')->getMock();
104105

@@ -330,7 +331,7 @@ public function assertRaised()
330331
private function getViolation()
331332
{
332333
return new ConstraintViolation(
333-
null,
334+
$this->message,
334335
$this->message,
335336
$this->parameters,
336337
$this->context->getRoot(),

‎src/Symfony/Component/Validator/Violation/ConstraintViolationBuilder.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class ConstraintViolationBuilder implements ConstraintViolationBuilderInterface
4747
/**
4848
* @param TranslatorInterface $translator
4949
*/
50-
public function __construct(ConstraintViolationList $violations, Constraint $constraint, $message, array $parameters, $root, $propertyPath, $invalidValue, $translator, $translationDomain = null)
50+
public function __construct(ConstraintViolationList $violations, Constraint $constraint, string $message, array $parameters, $root, string $propertyPath, $invalidValue, $translator, string $translationDomain = null)
5151
{
5252
if (!$translator instanceof LegacyTranslatorInterface && !$translator instanceof TranslatorInterface) {
5353
throw new \TypeError(sprintf('Argument 8 passed to %s() must be an instance of %s, %s given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));

0 commit comments

Comments
 (0)