fix: validate default answers

Author: sisp

copier/_main.py

@@ -555,11 +555,9 @@ def _ask(self) -> None:  # noqa: C901
             if var_name in self.answers.last:
                 try:
                     answer = question.parse_answer(self.answers.last[var_name])
+                    question.validate_answer(answer)
                 except Exception:
                     del self.answers.last[var_name]
-                else:
-                    if question.validate_answer(answer):
-                        del self.answers.last[var_name]
             # Skip a question when the skip condition is met.
             if not question.get_when():
                 # Omit its answer from the answers file.
@@ -573,14 +571,10 @@ def _ask(self) -> None:  # noqa: C901
                 if question.default is MISSING:
                     continue
             if var_name in self.answers.init:
-                # Try to parse the answer value.
+                # Try to parse and validate (if the question has a validator)
+                # the answer value.
                 answer = question.parse_answer(self.answers.init[var_name])
-                # Try to validate the answer value if the question has a
-                # validator.
-                if err_msg := question.validate_answer(answer):
-                    raise ValueError(
-                        f"Validation error for question '{var_name}': {err_msg}"
-                    )
+                question.validate_answer(answer)
                 # At this point, the answer value is valid. Do not ask the
                 # question again, but set answer as the user's answer instead.
                 self.answers.user[var_name] = answer

copier/_user_data.py

@@ -6,6 +6,7 @@
 import warnings
 from collections import ChainMap
 from collections.abc import Mapping, Sequence
+from copy import deepcopy
 from dataclasses import field
 from datetime import datetime
 from functools import cached_property
@@ -275,7 +276,13 @@ def get_default(self) -> Any:
                     result = self.render_value(
                         self.settings.defaults.get(self.var_name, self.default)
                     )
-        result = self.cast_answer(result)
+        result = self.parse_answer(result)
+        # Computed values (i.e., `when: false`) are intentionally not validated
+        # at the moment.
+        # https://github.com/copier-org/copier/issues/1779#issuecomment-2365006990
+        # https://github.com/copier-org/copier/pull/1785
+        if self.get_when():
+            self.validate_answer(result)
         return result
 
     def get_default_rendered(self) -> bool | str | Choice | None | MissingType:
@@ -319,7 +326,6 @@ def _formatted_choices(self) -> Sequence[Choice]:
         """Obtain choices rendered and properly formatted."""
         result = []
         choices = self.choices
-        default = self.get_default()
         if isinstance(choices, str):
             choices = parse_yaml_string(self.render_value(self.choices))
         if isinstance(choices, dict):
@@ -344,15 +350,7 @@ def _formatted_choices(self) -> Sequence[Choice]:
 
                 disabled = self.render_value(value.get("validator", ""))
                 value = value["value"]
-            # The value can be templated
-            value = self.render_value(value)
-            checked = (
-                self.multiselect
-                and isinstance(default, list)
-                and self.cast_answer(value) in default
-                or None
-            )
-            c = Choice(name, value, disabled=disabled, checked=checked)
+            c = Choice(name, self.render_value(value), disabled=disabled)
             # Try to cast the value according to the question's type to raise
             # an error in case the value is incompatible.
             self.cast_answer(c.value)
@@ -382,7 +380,11 @@ def _validate(answer: str) -> str | Literal[True]:
                 ans = self.parse_answer(answer)
             except Exception:
                 return "Invalid input"
-            return self.validate_answer(ans) or True
+            try:
+                self.validate_answer(ans)
+            except Exception as exc:
+                return str(exc)
+            return True
 
         lexer = None
         result: AnyByStrDict = {
@@ -405,7 +407,14 @@ def _validate(answer: str) -> str | Literal[True]:
                 result["default"] = False
         if self.choices:
             questionary_type = "checkbox" if self.multiselect else "select"
-            result["choices"] = self._formatted_choices
+            choices = self._formatted_choices
+            # Select default choices for a multiselect question.
+            if self.multiselect and isinstance(
+                default_choices := self.get_default(), list
+            ):
+                for choice in (choices := deepcopy(choices)):
+                    choice.checked = self.cast_answer(choice.value) in default_choices
+            result["choices"] = choices
         if questionary_type == "input":
             if self.secret:
                 questionary_type = "password"
@@ -436,15 +445,16 @@ def get_multiline(self) -> bool:
         """Get the value for multiline."""
         return cast_to_bool(self.render_value(self.multiline))
 
-    def validate_answer(self, answer: Any) -> str:
+    def validate_answer(self, answer: Any) -> None:
         """Validate user answer."""
         try:
             err_msg = self.render_value(self.validator, {self.var_name: answer}).strip()
         except Exception as error:
-            return str(error)
+            err_msg = str(error)
         if err_msg:
-            return err_msg
-        return ""
+            raise ValueError(
+                f"Validation error for question '{self.var_name}': {err_msg}"
+            )
 
     def get_when(self) -> bool:
         """Get skip condition for question."""

tests/test_copy.py

@@ -852,93 +852,89 @@ def test_required_choice_question_without_data(
 
 
 @pytest.mark.parametrize(
-    "type_name, default, expected",
+    "spec, expected",
     [
-        ("str", "string", does_not_raise()),
-        ("str", "1.0", does_not_raise()),
-        ("str", 1.0, does_not_raise()),
-        ("str", None, pytest.raises(TypeError)),
+        ({"type": "str", "default": "string"}, does_not_raise()),
+        ({"type": "str", "default": "1.0"}, does_not_raise()),
+        ({"type": "str", "default": 1.0}, does_not_raise()),
+        ({"type": "str", "default": None}, pytest.raises(TypeError)),
         (
             {
                 "type": "str",
+                "default": "",
+                "validator": "[% if q|length < 3 %]too short[% endif %]",
+            },
+            pytest.raises(ValueError),
+        ),
+        (
+            {
+                "type": "str",
+                "default": "",
                 "secret": True,
                 "validator": "[% if q|length < 3 %]too short[% endif %]",
             },
-            "",
             pytest.raises(ValueError),
         ),
-        ("int", 1, does_not_raise()),
-        ("int", 1.0, does_not_raise()),
-        ("int", "1", does_not_raise()),
-        ("int", "1.0", pytest.raises(ValueError)),
-        ("int", "no-int", pytest.raises(ValueError)),
-        ("int", None, pytest.raises(TypeError)),
-        ("int", {}, pytest.raises(TypeError)),
-        ("int", [], pytest.raises(TypeError)),
-        ("float", 1.1, does_not_raise()),
-        ("float", 1, does_not_raise()),
-        ("float", "1.1", does_not_raise()),
-        ("float", "no-float", pytest.raises(ValueError)),
-        ("float", None, pytest.raises(TypeError)),
-        ("float", {}, pytest.raises(TypeError)),
-        ("float", [], pytest.raises(TypeError)),
-        ("bool", True, does_not_raise()),
-        ("bool", False, does_not_raise()),
-        ("bool", "y", does_not_raise()),
-        ("bool", "n", does_not_raise()),
-        ("bool", None, pytest.raises(TypeError)),
-        ("json", '"string"', does_not_raise()),
-        ("json", "1", does_not_raise()),
-        ("json", 1, does_not_raise()),
-        ("json", "1.1", does_not_raise()),
-        ("json", 1.1, does_not_raise()),
-        ("json", "true", does_not_raise()),
-        ("json", True, does_not_raise()),
-        ("json", "false", does_not_raise()),
-        ("json", False, does_not_raise()),
-        ("json", "{}", does_not_raise()),
-        ("json", {}, does_not_raise()),
-        ("json", "[]", does_not_raise()),
-        ("json", [], does_not_raise()),
-        ("json", "null", does_not_raise()),
-        ("json", None, does_not_raise()),
-        ("yaml", '"string"', does_not_raise()),
-        ("yaml", "string", does_not_raise()),
-        ("yaml", "1", does_not_raise()),
-        ("yaml", 1, does_not_raise()),
-        ("yaml", "1.1", does_not_raise()),
-        ("yaml", 1.1, does_not_raise()),
-        ("yaml", "true", does_not_raise()),
-        ("yaml", True, does_not_raise()),
-        ("yaml", "false", does_not_raise()),
-        ("yaml", False, does_not_raise()),
-        ("yaml", "{}", does_not_raise()),
-        ("yaml", {}, does_not_raise()),
-        ("yaml", "[]", does_not_raise()),
-        ("yaml", [], does_not_raise()),
-        ("yaml", "null", does_not_raise()),
-        ("yaml", None, does_not_raise()),
+        ({"type": "int", "default": 1}, does_not_raise()),
+        ({"type": "int", "default": 1.0}, does_not_raise()),
+        ({"type": "int", "default": "1"}, does_not_raise()),
+        ({"type": "int", "default": "1.0"}, pytest.raises(ValueError)),
+        ({"type": "int", "default": "no-int"}, pytest.raises(ValueError)),
+        ({"type": "int", "default": None}, pytest.raises(TypeError)),
+        ({"type": "int", "default": {}}, pytest.raises(TypeError)),
+        ({"type": "int", "default": []}, pytest.raises(TypeError)),
+        ({"type": "float", "default": 1.1}, does_not_raise()),
+        ({"type": "float", "default": 1}, does_not_raise()),
+        ({"type": "float", "default": "1.1"}, does_not_raise()),
+        ({"type": "float", "default": "no-float"}, pytest.raises(ValueError)),
+        ({"type": "float", "default": None}, pytest.raises(TypeError)),
+        ({"type": "float", "default": {}}, pytest.raises(TypeError)),
+        ({"type": "float", "default": []}, pytest.raises(TypeError)),
+        ({"type": "bool", "default": True}, does_not_raise()),
+        ({"type": "bool", "default": False}, does_not_raise()),
+        ({"type": "bool", "default": "y"}, does_not_raise()),
+        ({"type": "bool", "default": "n"}, does_not_raise()),
+        ({"type": "bool", "default": None}, pytest.raises(TypeError)),
+        ({"type": "json", "default": '"string"'}, does_not_raise()),
+        ({"type": "json", "default": "1"}, does_not_raise()),
+        ({"type": "json", "default": 1}, does_not_raise()),
+        ({"type": "json", "default": "1.1"}, does_not_raise()),
+        ({"type": "json", "default": 1.1}, does_not_raise()),
+        ({"type": "json", "default": "true"}, does_not_raise()),
+        ({"type": "json", "default": True}, does_not_raise()),
+        ({"type": "json", "default": "false"}, does_not_raise()),
+        ({"type": "json", "default": False}, does_not_raise()),
+        ({"type": "json", "default": "{}"}, does_not_raise()),
+        ({"type": "json", "default": {}}, does_not_raise()),
+        ({"type": "json", "default": "[]"}, does_not_raise()),
+        ({"type": "json", "default": []}, does_not_raise()),
+        ({"type": "json", "default": "null"}, does_not_raise()),
+        ({"type": "json", "default": None}, does_not_raise()),
+        ({"type": "yaml", "default": '"string"'}, does_not_raise()),
+        ({"type": "yaml", "default": "string"}, does_not_raise()),
+        ({"type": "yaml", "default": "1"}, does_not_raise()),
+        ({"type": "yaml", "default": 1}, does_not_raise()),
+        ({"type": "yaml", "default": "1.1"}, does_not_raise()),
+        ({"type": "yaml", "default": 1.1}, does_not_raise()),
+        ({"type": "yaml", "default": "true"}, does_not_raise()),
+        ({"type": "yaml", "default": True}, does_not_raise()),
+        ({"type": "yaml", "default": "false"}, does_not_raise()),
+        ({"type": "yaml", "default": False}, does_not_raise()),
+        ({"type": "yaml", "default": "{}"}, does_not_raise()),
+        ({"type": "yaml", "default": {}}, does_not_raise()),
+        ({"type": "yaml", "default": "[]"}, does_not_raise()),
+        ({"type": "yaml", "default": []}, does_not_raise()),
+        ({"type": "yaml", "default": "null"}, does_not_raise()),
+        ({"type": "yaml", "default": None}, does_not_raise()),
     ],
 )
 def test_validate_default_value(
     tmp_path_factory: pytest.TempPathFactory,
-    type_name: str,
-    default: Any,
+    spec: AnyByStrDict,
     expected: AbstractContextManager[None],
 ) -> None:
     src, dst = map(tmp_path_factory.mktemp, ("src", "dst"))
-    build_file_tree(
-        {
-            (src / "copier.yml"): yaml.dump(
-                {
-                    "q": {
-                        "type": type_name,
-                        "default": default,
-                    }
-                }
-            )
-        }
-    )
+    build_file_tree({(src / "copier.yml"): yaml.dump({"q": spec})})
     with expected:
         copier.run_copy(str(src), dst, defaults=True)