[Validator] Enable auto mapping by default

[dunglas]

Q	A
License	MIT
Doc issue/PR	todo

Now that symfony/symfony#32107 has been merged, auto mapping can be on again.

[ghost]

Pull request passes validation.

[weaverryan]

I think we should add a docs PR to go along with this - it’s a really important change.

[nicolas-grekas]

(@dunglas please create a doc issue at least)

[dunglas]

Actually, it's already documented symfony/symfony-docs#11132

[weaverryan]

I like this feature! But it does have side effects. For example, in make:user, we will need to make sure that we add the "opt-out" annotation above the password property... which we can only actually do if the validator component is installed at the moment the command is run :/.

If a user doesn't have the "opt-out" annotation above password, when they create a registration form, it will always fail with the generic "this value cannot be blank" validation error - which will appear at the top of the form, not attached to any field. That's because password won't be part of the form - the user will have some sort of a plainPassword which is actually what's on the form.

That's the big WTF that we need to navigate around if we're going to enable this feature by default.

[dunglas]

Does it make sense to create a user system without validation? I mean you already need to check that the username is not taken, that the password isn’t empty, etc... isn’t it?

Cannot the command throw if the Validator component isn’t installed?

[weaverryan]

Cannot the command throw if the Validator component isn’t installed?

I was thinking about this as well. Probably we need to allow a User to be created without requiring validator (there are likely at least some cases where users are created via some system that doesn't need validation). But we could (at least) definitely add the opt-out annotation when they run make:registration-form. Basically, I think we can do a pretty good job in Maker with messaging and being smart.

Btw, there is another docs issue about this being automatically enabled, that still needs work.

[weaverryan]

Btw, I'd like to merge this as soon as we can so that it "feels" like 4.4 and 5.0 consistently have this feature enabled. But, I think we need the MakerBundle tweak and docs first. I'm working as quickly as I can on those :).

[weaverryan]

MakerBundle PR for @DisableAutoMapping: symfony/maker-bundle#514

But the feature currently doesn't work :/ - see symfony/symfony#34672

And (sorry!) I'm fighting back a little bit again on this feature - symfony/symfony#34860.

[nicolas-grekas]

(please rebase so that the new checks can run)

[github-actions]

Diff between recipe versions

Thanks for the PR 😍

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/validator

3.3 vs 4.1

diff --git a/symfony/validator/4.1/config/packages/test/validator.yaml b/symfony/validator/4.1/config/packages/test/validator.yaml
new file mode 100644
index 0000000..b18c866
--- /dev/null
+++ b/symfony/validator/4.1/config/packages/test/validator.yaml
@@ -0,0 +1,4 @@
+framework:
+    validation:
+        # As of Symfony 4.3 you can disable the NotCompromisedPassword Validator
+        # not_compromised_password: false
diff --git a/symfony/validator/4.1/config/packages/validator.yaml b/symfony/validator/4.1/config/packages/validator.yaml
new file mode 100644
index 0000000..a695e1a
--- /dev/null
+++ b/symfony/validator/4.1/config/packages/validator.yaml
@@ -0,0 +1,3 @@
+framework:
+    validation:
+        email_validation_mode: html5
diff --git a/symfony/validator/3.3/manifest.json b/symfony/validator/4.1/manifest.json
index 2a250e2..57f78dc 100644
--- a/symfony/validator/3.3/manifest.json
+++ b/symfony/validator/4.1/manifest.json
@@ -1,3 +1,6 @@
 {
+    "copy-from-recipe": {
+        "config/": "%CONFIG_DIR%/"
+    },
     "aliases": ["validation"]
 }

4.1 vs 4.3

diff --git a/symfony/validator/4.1/config/packages/test/validator.yaml b/symfony/validator/4.3/config/packages/test/validator.yaml
index b18c866..1e5ab78 100644
--- a/symfony/validator/4.1/config/packages/test/validator.yaml
+++ b/symfony/validator/4.3/config/packages/test/validator.yaml
@@ -1,4 +1,3 @@
 framework:
     validation:
-        # As of Symfony 4.3 you can disable the NotCompromisedPassword Validator
-        # not_compromised_password: false
+        not_compromised_password: false
diff --git a/symfony/validator/4.1/config/packages/validator.yaml b/symfony/validator/4.3/config/packages/validator.yaml
index a695e1a..350786a 100644
--- a/symfony/validator/4.1/config/packages/validator.yaml
+++ b/symfony/validator/4.3/config/packages/validator.yaml
@@ -1,3 +1,8 @@
 framework:
     validation:
         email_validation_mode: html5
+
+        # Enables validator auto-mapping support.
+        # For instance, basic validation constraints will be inferred from Doctrine's metadata.
+        #auto_mapping:
+        #    App\Entity\: []

4.3 vs 4.4

diff --git a/symfony/validator/4.3/config/packages/validator.yaml b/symfony/validator/4.4/config/packages/validator.yaml
index 350786a..1affc68 100644
--- a/symfony/validator/4.3/config/packages/validator.yaml
+++ b/symfony/validator/4.4/config/packages/validator.yaml
@@ -4,5 +4,5 @@ framework:
 
         # Enables validator auto-mapping support.
         # For instance, basic validation constraints will be inferred from Doctrine's metadata.
-        #auto_mapping:
-        #    App\Entity\: []
+        auto_mapping:
+            App\Entity\: []

[dunglas]

@nicolas-grekas done. The failure looks unrelated, or am I missing something?

[javiereguiluz]

I'm closing this proposal as "won't merge" because after discussing about this in the Core Team, it was decided that it's better to not enable auto_mapping by default in Symfony apps. Thanks.