diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 000000000..7199182e4 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,6 @@ +FROM python:3.11-slim +WORKDIR /app +COPY . . +RUN pip install -e . +RUN pip install pytest +CMD ["/bin/bash"] diff --git a/Github-setup.md b/Github-setup.md new file mode 100644 index 000000000..5bd7b38b7 --- /dev/null +++ b/Github-setup.md @@ -0,0 +1,12 @@ +Repository URL: +https://github.com/Imaad00/attrs + +Commit Hash Used: +d7f3c345d22671d71b129036783ce0ce33105956 + +Directory Touched: +src/attr/_make.py + +Files Introduced: +tests/test_factory_validation_order.py +test.sh diff --git a/PROBLEM.md b/PROBLEM.md new file mode 100644 index 000000000..d9a4f73f6 --- /dev/null +++ b/PROBLEM.md @@ -0,0 +1,54 @@ +Incorrect Default Factory Execution Order in attrs Causing Validation Errors +---------------------------------------------------------------------------- + +Brief +----- +Classes defined using `@define` in the attrs library support dynamic default +values via a `factory=` argument and value validation via validators. In +certain cases where a required field, a factory-provided field, and one or more +validators exist together, attrs initializes default values too early. This +causes validators to run on incomplete or incorrect state. + +This produces mismatches between user intent and runtime behavior — validators +may see pre-factory values, incorrectly succeed, or incorrectly fail. + +Expected Behavior +----------------- +Default factories must run only after all explicitly provided required values +have been: +1. Assigned +2. Validated individually + +Validators must always receive: +• The resolved value returned by the default factory +• A fully initialized set of attributes +• Complete cross-field state + +Required Functional Behavior +---------------------------- +1. Required fields missing → raise a validation error (unchanged behavior). +2. The default factory must execute *only if* the user does not pass a value. +3. The default factory must run *after* successful validation of required fields. +4. All validators — including cross-field validators — must observe the final + resolved default value. +5. If any validator raises, object initialization must halt without returning + a partially initialized instance. +6. Classes without factories or cross-field validators must behave identically + to current releases (no regressions). + +Edge Cases to Support +--------------------- +• Field B default depends on validated A +• Validators referencing multiple attributes +• Factories returning mutable objects (must not reuse previous instance) +• Missing required fields +• Multiple factories in one class + +Success Outcome +--------------- +After applying a fix, all validators must execute against final initialized +state, default factories must run in the correct order, and all existing behavior +must remain unchanged unless explicitly addressed in this brief. + +Everything described must be testable deterministically, with no randomness, +timers, or network interaction. diff --git a/Test.patch b/Test.patch new file mode 100644 index 000000000..0e28fbaff Binary files /dev/null and b/Test.patch differ diff --git a/classification.md b/classification.md new file mode 100644 index 000000000..5f7661702 --- /dev/null +++ b/classification.md @@ -0,0 +1,9 @@ +Language: Python +Difficulty: Hard +Category: Bug Fix + Feature Clarification +Repository: attrs (https://github.com/python-attrs/attrs) +License: MIT +Reasoning: +- Touches core initialization logic +- Requires test-driven development +- Needs correct validation ordering diff --git a/commit.txt b/commit.txt new file mode 100644 index 000000000..f0894620d Binary files /dev/null and b/commit.txt differ diff --git a/description.md b/description.md new file mode 100644 index 000000000..d0238db53 --- /dev/null +++ b/description.md @@ -0,0 +1,26 @@ +Title: Correct Factory Validation Execution Ordering in attrs Initialization + +Brief: +Classes defined using `@define` in the attrs library support default factories +to generate attribute values when not explicitly passed by users. Currently, +factory execution may occur before required field validation, causing +cross-field validators to observe incomplete state, leading to silent data +corruption. + +Intended Behavior: +- Required fields must be validated before any factory executes. +- Factories must only run when user does not supply a value. +- Factory-produced values must be validated before assignment. +- Cross-field and per-field validators must observe fully initialized state. +- Behavior must remain unchanged for attributes without factories. + +Edge Cases: +- Required missing → error +- Factory returning invalid value → error +- Cross-field validator inspecting both attributes +- Mutable default returned from factory should not be shared + +Success Outcome: +- New tests exercise above cases +- Base tests pass unchanged +- New tests fail before fix and pass after fix diff --git a/solution.patch b/solution.patch new file mode 100644 index 000000000..7c5ecf2d7 Binary files /dev/null and b/solution.patch differ diff --git a/src/attr/_make.py b/src/attr/_make.py index d24d9ba98..c64863153 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -2280,6 +2280,7 @@ def _attrs_to_init_script( init_factory_name = _INIT_FACTORY_PAT % (a.name,) if converter is not None: + # arg was passed explicitly → validate immediately lines.append( " " + fmt_setter_with_converter( @@ -2287,11 +2288,19 @@ def _attrs_to_init_script( ) ) lines.append("else:") + # no arg passed → run factory → validate → assign + lines.append(f" val = {init_factory_name}({maybe_self})") + if a.validator is not None: + val_name = "__attr_validator_" + a.name + attr_name_ref = "__attr_" + a.name + lines.append(f" {val_name}(self, {attr_name_ref}, val)") + names_for_globals[val_name] = a.validator + names_for_globals[attr_name_ref] = a lines.append( " " + fmt_setter_with_converter( attr_name, - init_factory_name + "(" + maybe_self + ")", + "val", has_on_setattr, converter, ) @@ -2300,37 +2309,29 @@ def _attrs_to_init_script( converter.converter ) else: + # arg passed explicitly → validate immediately lines.append( " " + fmt_setter(attr_name, arg_name, has_on_setattr) ) lines.append("else:") + # no arg passed → run factory → validate → assign + lines.append(f" val = {init_factory_name}({maybe_self})") + if a.validator is not None: + val_name = "__attr_validator_" + a.name + attr_name_ref = "__attr_" + a.name + lines.append(f" {val_name}(self, {attr_name_ref}, val)") + names_for_globals[val_name] = a.validator + names_for_globals[attr_name_ref] = a lines.append( " " + fmt_setter( attr_name, - init_factory_name + "(" + maybe_self + ")", + "val", has_on_setattr, ) ) - names_for_globals[init_factory_name] = a.default.factory - else: - if a.kw_only: - kw_only_args.append(arg_name) - else: - args.append(arg_name) - pre_init_args.append(arg_name) - if converter is not None: - lines.append( - fmt_setter_with_converter( - attr_name, arg_name, has_on_setattr, converter - ) - ) - names_for_globals[converter._get_global_name(a.name)] = ( - converter.converter - ) - else: - lines.append(fmt_setter(attr_name, arg_name, has_on_setattr)) + names_for_globals[init_factory_name] = a.default.factory if a.init is True: if a.type is not None and converter is None: diff --git a/test.sh b/test.sh new file mode 100644 index 000000000..7784467ea --- /dev/null +++ b/test.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -e +case "$1" in + base) + pytest tests/ --ignore=tests/test_factory_validation_order.py + ;; + new) + pytest tests/test_factory_validation_order.py + ;; + *) + echo "Usage: ./test.sh {base|new}" + exit 1 + ;; +esac diff --git a/tests/test_factory_validation_order.py b/tests/test_factory_validation_order.py new file mode 100644 index 000000000..1ec2f699c --- /dev/null +++ b/tests/test_factory_validation_order.py @@ -0,0 +1,58 @@ +import pytest + +from attrs import define, field, validators + + +def test_default_factory_runs_after_required_validation(): + def make_b(): + return 10 + + @define + class Item: + a: int = field(validator=validators.gt(0)) + b: int = field( + default=None, factory=make_b, validator=validators.gt(0) + ) + + obj = Item(5) + assert obj.b == 10 + + +def test_cross_field_validator_sees_final_defaults(): + def cross_validate(instance, attribute, value): + # b must always be greater than a + if instance.b <= instance.a: + raise ValueError("b must be greater than a") + + @define + class Item: + a: int = field() + b: int = field( + default=None, factory=lambda: 5, validator=cross_validate + ) + + with pytest.raises(ValueError): + Item(5) + + # When user provides b manually, it should succeed + assert Item(5, 10).b == 10 + + +def test_missing_required_still_errors(): + @define + class Thing: + a: int = field() + b: int = field(default=None, factory=lambda: 3) + + with pytest.raises(TypeError): + Thing() + + +def test_independent_fields_unchanged(): + @define + class Simple: + x: int = field(validator=validators.gt(0)) + y: str = field(default="hi") + + obj = Simple(10) + assert obj.y == "hi"