Skip to content

[RFC][Validator] Change Validator auto_mapping to opt-in #34860

Closed
@weaverryan

Description

@weaverryan

Hi!

I think we should change how Validation auto_mapping works. I've argued for this before... I need to again :).

The Problem

Validation auto_mapping works by activating it config/packages/validator.yaml for specific namespaces:

framework:
    validation:
        auto_mapping:
            App\Entity\: []

I think this is a mistake for 3 reasons:

  1. When some "unexpected" validation happens, a user might not even know that an auto-mapping validation feature exists. They will rely on years of Stack Overflow posts they tell them to look for annotation constraints in the class... but they will have none.

  2. If we merged [Validator] Enable auto mapping by default recipes#664 to activate this feature for new project, then we split the community. Some users will suddenly have auto-mapping validation working automatically everywhere while other users (existing projects) will not. That's especially difficult because there's no annotation inside the class to help see this distinction. It makes docs & debugging hard... and we will activate this feature, not on an exact release... but whatever random day we merge that PR.

  3. With the approach of activating auto_mapping globally with YAML config, it makes it much harder for existing projects to "migrate" to this approach. Just adding the config is dangerous: they could have some forms they don't think about where new validation rules suddenly show up, unwanted.

Proposal

For that reason, I propose the following change. This ignores BC for the moment - just for the sake of explaining the idea clearly.

A) Make auto_mapping ONLY be activated when the @EnableAutoMapping annotation is added to a class (or, sure, you could activate it just for a property). Users could then opt into the feature on a class-by-class basis and it makes the feature more obvious & debuggable.

B) The YAML config would no longer be needed. Basically, auto_mapping would now be a true/false boolean (it could default to true) that would simply load/don't load the auto-mapping related services. AND, this means that auto_mapping is instantly useable for any class we validate. Currently, if you put a class in App\Model, it won't have auto-mapping (you need to know to tweak some config). Heck, if some users still want to be able to automatically-activate this feature, we could still have an option to do that for some namespaces.

For BC, we would probably need to deprecate the old auto_mapping option in favor of a new option (auto_validation?) that has the new behavior. Or, we could just deprecate passing auto_mapping an array - in favor of making it a boolean. The boolean would activate the new behavior.

Thanks!

Metadata

Metadata

Assignees

No one assigned

    Labels

    RFCRFC = Request For Comments (proposals about features that you want to be discussed)StalledValidator

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions