symfony: NotBlank validator should not invalidate null values
Description
In most validators, there’s a failsafe mecanism that prevent the validator from validating if it’s a null value, as there’s a validator for that (NotNull). I think NotBlank shouldn’t bother on null values, especially as NotBlank and NotNull are used together to prevent null values too.
Not really a feature request, but not really a bug too… more like a RFC but for all versions. But I guess this would be a BC break though. 😕 So maybe add a deprecated on the validator if it’s a null value or something, and remove the null invalidation from the NotBlank on 5.0 ?
Currently, we need to reimplement the validator just to skip null values…
Possible Solution
Either deprecate the validation on null value in NotBlank, or do as in most validators, (if null value, then skip). But as I mentionned, this would probably be a bc break.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 19
- Comments: 23 (23 by maintainers)
Commits related to this issue
- feature #29641 [Validator] NotBlank: add a new option to allow null values (dunglas) This PR was squashed before being merged into the 4.3-dev branch (closes #29641). Discussion ---------- [Validat... — committed to symfony/symfony by fabpot 5 years ago
- feature #31528 [Validator] Add a Length::$allowEmptyString option to reject empty strings (ogizanagi) This PR was merged into the 4.4 branch. Discussion ---------- [Validator] Add a Length::$allowE... — committed to symfony/doctrine-bridge by fabpot 5 years ago
- feature #31528 [Validator] Add a Length::$allowEmptyString option to reject empty strings (ogizanagi) This PR was merged into the 4.4 branch. Discussion ---------- [Validator] Add a Length::$allowE... — committed to symfony/symfony by fabpot 5 years ago
- feature #31528 [Validator] Add a Length::$allowEmptyString option to reject empty strings (ogizanagi) This PR was merged into the 4.4 branch. Discussion ---------- [Validator] Add a Length::$allowE... — committed to symfony/form by fabpot 5 years ago
- feature #31528 [Validator] Add a Length::$allowEmptyString option to reject empty strings (ogizanagi) This PR was merged into the 4.4 branch. Discussion ---------- [Validator] Add a Length::$allowE... — committed to symfony/validator by fabpot 5 years ago
Adding an option
allowNull(or something like that), defaulting tofalsewould be a good BC imo, and would solve the current problem (and why not remove it on 5.0 ?). As @Neirda24 said, anullvalue is “no value” rather than “blank value”…Which makes no sense to me neither tbh…
That would be a huge BC break. And due to the form component setting properties to
nullwhen submitting an empty field, it might even be confusing to change the behavior ofNotBlank. So I’m not sure it is worth it.Adding an
allow_nulloption is indeed a quick fix, and would help a lot when creating APIs. 👍 on my side.Can you provide a source for this? We always use just NotBlank. Usage for NotNull is really rare in projects I’ve seen.
How is “not blank” value ever null value? Blank includes null.
For what use case?
Maybe we could add a
NullOrNotBlankand potentially renameNotBlanktoNotNullNotBlank. I’m not too happy with the names but IMO it’s the original name that is messed up and we can’t change it without BC breaks.On a side note, I witnessed at least 10 Symfony projects where they declared a
NotBlankignoringnullvalues, so I also strongly feel there is an issue here.Again, we are not arguing we cannot achieve what we want. There is several workarounds available for them and we already discussed about them here. Our point is that the default behaviour is confusing and intuitive. That you consider it to be a loud minority and not a design/UX issue is another story and up for debate and why this issue has been created.
I don’t agree for one. You can’t deprecate that without majority agreeing. All I see is loud minority. What you guys want can be achieved with other constraints like Length and NotEqualTo, what NotBlank does currently can’t be replaced with any other single constraint, only with multiple.
@ostrolucky there is workarounds, we are not arguying otherwise. We would like however to make those constraints more intuitive
@xabbuh : The issue with the
Lengthconstraint is that it ignores empty strings: https://github.com/symfony/symfony/blob/b226859e8a949c0b39071f61ffcc0b4c10e3ba46/src/Symfony/Component/Validator/Constraints/LengthValidator.php#L32-L34@ostrolucky because there is a need to check for non null empty values.
nullis often given either as “reset” or “not provided” value.You can always argue that a form (for an API or not) - and I agree on paper. But in practice for a lot of front-ends omitting non provided fields as opposed to sending null requires too much work for what it’s worth, hence this need in the back-end.
I know this can be a huge debate but for me “the absence of value” does not mean “a blank value”. Also
emptyin PHP also applies to arrays. So[]is empty but is not blank… (another reason why I also find it not intuitive).Definitely agreeing with @theofidry . When using any Constraints in symfony the first rule is “Do not validate what you are not supposed to”, so I don’t understand why there is a
NotBlankvalidating null and aNotNull. maybe add something likeallowNullin theNotBlankconstraints but set it to deprecated…We can always find a way to go around this of course. But to stay consistent with any other constraints I feel like
nullshould not be tested byNotBlank…Well, my point is I don’t see a use case for the
NotBlankconstraint when you are not validating a string. For other types there are better constraints IMO. Based on this assumption we only need to discuss whether or not we need to distinguishnullfrom the empty string which brings us back to the question if we need a shortcut for theLengthconstraint with a minimum length of 1.