Skip to content

[Validator] deprecate non-string constraint violation codes #32265

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 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 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions UPGRADE-4.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,8 @@ TwigBridge
Validator
---------

* Deprecated using anything else than a `string` as the code of a `ConstraintViolation`, a `string` type-hint will
be added to the constructor of the `ConstraintViolation` class and to the `ConstraintViolationBuilder::setCode()`
method in 5.0.
* Deprecated passing an `ExpressionLanguage` instance as the second argument of `ExpressionValidator::__construct()`.
Pass it as the first argument instead.
2 changes: 2 additions & 0 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ TwigBridge
Validator
--------

* Removed support for non-string codes of a `ConstraintViolation`. A `string` type-hint was added to the constructor of
the `ConstraintViolation` class and to the `ConstraintViolationBuilder::setCode()` method.
* An `ExpressionLanguage` instance or null must be passed as the first argument of `ExpressionValidator::__construct()`
* The `checkMX` and `checkHost` options of the `Email` constraint were removed
* The `Email::__construct()` 'strict' property has been removed. Use 'mode'=>"strict" instead.
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Validator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ CHANGELOG
4.4.0
-----

* using anything else than a `string` as the code of a `ConstraintViolation` is deprecated, a `string` type-hint will
be added to the constructor of the `ConstraintViolation` class and to the `ConstraintViolationBuilder::setCode()`
method in 5.0
* deprecated passing an `ExpressionLanguage` instance as the second argument of `ExpressionValidator::__construct()`. Pass it as the first argument instead.
* added the `compared_value_path` parameter in violations when using any
comparison constraint with the `propertyPath` option.
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/Validator/ConstraintViolation.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ public function __construct(?string $message, ?string $messageTemplate, array $p
$message = '';
}

if (null !== $code && !\is_string($code)) {
@trigger_error(sprintf('Not using a string as the error code in %s() is deprecated since Symfony 4.4. A type-hint will be added in 5.0.', __METHOD__), E_USER_DEPRECATED);
}

$this->message = $message;
$this->messageTemplate = $messageTemplate;
$this->parameters = $parameters;
Expand Down
16 changes: 8 additions & 8 deletions src/Symfony/Component/Validator/Constraints/FileValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,49 +65,49 @@ public function validate($value, Constraint $constraint)
$this->context->buildViolation($constraint->uploadIniSizeErrorMessage)
->setParameter('{{ limit }}', $limitAsString)
->setParameter('{{ suffix }}', $suffix)
->setCode(UPLOAD_ERR_INI_SIZE)
->setCode((string) UPLOAD_ERR_INI_SIZE)
->addViolation();

return;
case UPLOAD_ERR_FORM_SIZE:
$this->context->buildViolation($constraint->uploadFormSizeErrorMessage)
->setCode(UPLOAD_ERR_FORM_SIZE)
->setCode((string) UPLOAD_ERR_FORM_SIZE)
->addViolation();

return;
case UPLOAD_ERR_PARTIAL:
$this->context->buildViolation($constraint->uploadPartialErrorMessage)
->setCode(UPLOAD_ERR_PARTIAL)
->setCode((string) UPLOAD_ERR_PARTIAL)
->addViolation();

return;
case UPLOAD_ERR_NO_FILE:
$this->context->buildViolation($constraint->uploadNoFileErrorMessage)
->setCode(UPLOAD_ERR_NO_FILE)
->setCode((string) UPLOAD_ERR_NO_FILE)
->addViolation();

return;
case UPLOAD_ERR_NO_TMP_DIR:
$this->context->buildViolation($constraint->uploadNoTmpDirErrorMessage)
->setCode(UPLOAD_ERR_NO_TMP_DIR)
->setCode((string) UPLOAD_ERR_NO_TMP_DIR)
->addViolation();

return;
case UPLOAD_ERR_CANT_WRITE:
$this->context->buildViolation($constraint->uploadCantWriteErrorMessage)
->setCode(UPLOAD_ERR_CANT_WRITE)
->setCode((string) UPLOAD_ERR_CANT_WRITE)
->addViolation();

return;
case UPLOAD_ERR_EXTENSION:
$this->context->buildViolation($constraint->uploadExtensionErrorMessage)
->setCode(UPLOAD_ERR_EXTENSION)
->setCode((string) UPLOAD_ERR_EXTENSION)
->addViolation();

return;
default:
$this->context->buildViolation($constraint->uploadErrorMessage)
->setCode($value->getError())
->setCode((string) $value->getError())
->addViolation();

return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function testToStringHandlesCodes()
'some_value',
null,
null,
0
'0'
);

$expected = <<<'EOF'
Expand Down Expand Up @@ -108,4 +108,24 @@ public function testToStringOmitsEmptyCodes()

$this->assertSame($expected, (string) $violation);
}

/**
* @group legacy
* @expectedDeprecation Not using a string as the error code in Symfony\Component\Validator\ConstraintViolation::__construct() is deprecated since Symfony 4.4. A type-hint will be added in 5.0.
*/
public function testNonStringCode()
{
$violation = new ConstraintViolation(
'42 cannot be used here',
'this is the message template',
[],
['some_value' => 42],
'some_value',
null,
null,
42
);

self::assertSame(42, $violation->getCode());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -435,23 +435,23 @@ public function testUploadedFileError($error, $message, array $params = [], $max
public function uploadedFileErrorProvider()
{
$tests = [
[UPLOAD_ERR_FORM_SIZE, 'uploadFormSizeErrorMessage'],
[UPLOAD_ERR_PARTIAL, 'uploadPartialErrorMessage'],
[UPLOAD_ERR_NO_FILE, 'uploadNoFileErrorMessage'],
[UPLOAD_ERR_NO_TMP_DIR, 'uploadNoTmpDirErrorMessage'],
[UPLOAD_ERR_CANT_WRITE, 'uploadCantWriteErrorMessage'],
[UPLOAD_ERR_EXTENSION, 'uploadExtensionErrorMessage'],
[(string) UPLOAD_ERR_FORM_SIZE, 'uploadFormSizeErrorMessage'],
[(string) UPLOAD_ERR_PARTIAL, 'uploadPartialErrorMessage'],
[(string) UPLOAD_ERR_NO_FILE, 'uploadNoFileErrorMessage'],
[(string) UPLOAD_ERR_NO_TMP_DIR, 'uploadNoTmpDirErrorMessage'],
[(string) UPLOAD_ERR_CANT_WRITE, 'uploadCantWriteErrorMessage'],
[(string) UPLOAD_ERR_EXTENSION, 'uploadExtensionErrorMessage'],
];

if (class_exists('Symfony\Component\HttpFoundation\File\UploadedFile')) {
// when no maxSize is specified on constraint, it should use the ini value
$tests[] = [UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
$tests[] = [(string) UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
'{{ limit }}' => UploadedFile::getMaxFilesize() / 1048576,
'{{ suffix }}' => 'MiB',
]];

// it should use the smaller limitation (maxSize option in this case)
$tests[] = [UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
$tests[] = [(string) UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
'{{ limit }}' => 1,
'{{ suffix }}' => 'bytes',
], '1'];
Expand All @@ -464,14 +464,14 @@ public function uploadedFileErrorProvider()

// it correctly parses the maxSize option and not only uses simple string comparison
// 1000M should be bigger than the ini value
$tests[] = [UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
$tests[] = [(string) UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
'{{ limit }}' => $limit,
'{{ suffix }}' => $suffix,
], '1000M'];

// it correctly parses the maxSize option and not only uses simple string comparison
// 1000M should be bigger than the ini value
$tests[] = [UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
$tests[] = [(string) UPLOAD_ERR_INI_SIZE, 'uploadIniSizeErrorMessage', [
'{{ limit }}' => '0.1',
'{{ suffix }}' => 'MB',
], '100K'];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Validator\Tests\Violation;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Translation\IdentityTranslator;
use Symfony\Component\Validator\ConstraintViolationList;
use Symfony\Component\Validator\Tests\Fixtures\ConstraintA;
use Symfony\Component\Validator\Violation\ConstraintViolationBuilder;

class ConstraintViolationBuilderTest extends TestCase
{
/**
* @group legacy
* @expectedDeprecation Not using a string as the error code in Symfony\Component\Validator\Violation\ConstraintViolationBuilder::setCode() is deprecated since Symfony 4.4. A type-hint will be added in 5.0.
* @expectedDeprecation Not using a string as the error code in Symfony\Component\Validator\ConstraintViolation::__construct() is deprecated since Symfony 4.4. A type-hint will be added in 5.0.
*/
public function testNonStringCode()
{
$constraintViolationList = new ConstraintViolationList();
(new ConstraintViolationBuilder($constraintViolationList, new ConstraintA(), 'invalid message', [], null, 'foo', 'baz', new IdentityTranslator()))
->setCode(42)
->addViolation();

self::assertSame(42, $constraintViolationList->get(0)->getCode());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ public function setPlural($number)
*/
public function setCode($code)
{
if (null !== $code && !\is_string($code)) {
@trigger_error(sprintf('Not using a string as the error code in %s() is deprecated since Symfony 4.4. A type-hint will be added in 5.0.', __METHOD__), E_USER_DEPRECATED);
}

$this->code = $code;

return $this;
Expand Down