Skip to content

Constraint REGEX htmlPattern cannot be a boolean. #53807

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

Closed
cavasinf opened this issue Feb 6, 2024 · 5 comments
Closed

Constraint REGEX htmlPattern cannot be a boolean. #53807

cavasinf opened this issue Feb 6, 2024 · 5 comments

Comments

@cavasinf
Copy link

cavasinf commented Feb 6, 2024

Symfony version(s) affected

6.4.3

Description

Doc says that we can pass false to htmlPattern parameter, but in reality this is not the case.

Setting htmlPattern to false will disable client side validation.

https://symfony.com/doc/6.4/reference/constraints/Regex.html#htmlpattern

Class:

https://github.com/symfony/validator/blob/03b0c75d7d3df1ef9a0fd9fb8db1e86f83ffa2bb/Constraints/Regex.php#L39

How to reproduce

  1. Add assert constraint to an entity:
use Symfony\Component\Validator\Constraints as Assert;

// ...

#[Assert\Regex(
    pattern: '/^[a-z]+(-[a-z]+)*$/',
    htmlPattern: false,
)]
private ?string $slug = null;
  1. Run phpstan
  2. Error
------ --------------------------------------------------------------------------------------------------------------------------------------- 
  Line   App/Entity/MyEntityphp                                                                                             
 ------ --------------------------------------------------------------------------------------------------------------------------------------- 
  33     Parameter $htmlPattern of attribute class Symfony\Component\Validator\Constraints\Regex constructor expects string|null, false given.  
 ------ ---------------------------------------------------------------------------------------------------------------------------------------

Possible Solution

Allow boolean or false type on htmlPattern parameter.

Additional Context

No response

@stof
Copy link
Member

stof commented Feb 6, 2024

Looking at the code, you can also set it to an empty string for that

I'm not sure how we want to solve this inconsistency between the type that has been added in the code and the documentation.

Looking in the history, support for passing false to disable the HTML pattern was covered by tests when it was introduced in 6f9eda9 but this case was lost when refactoring the tests in bf006f5

@cavasinf
Copy link
Author

cavasinf commented Feb 7, 2024

you can also set it to an empty string for that

Ok for a "workaround", but I don't think it's a "normal way" to think if you want this behavior.
IMO the false option as recommended by the doc is a good configuration.

@alexandre-daubois
Copy link
Member

The problem is Regex is not final and htmlPattern is public so its type cannot be changed without breaking backward compatibility. Sometimes BC breaks are accepted between minor versions but this is pretty rare. I think the best thing to do here is to address this to the documentation. I indeed agree with you that providing an empty string is not the best option and being able to pass false would be better, but I'm afraid this has to wait for Symfony 8.0 😄

@javiereguiluz
Copy link
Member

The related docs have been fixed in symfony/symfony-docs#20089

I reopen the issue because I don't know if there are still some code changes that need to be made to fix this. Thanks.

@xabbuh
Copy link
Member

xabbuh commented Aug 2, 2024

Let’s close here. Since the constraint class isn’t final changing the property type isn’t easily possible and thus not worth the effort IMO.

@xabbuh xabbuh closed this as completed Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants