From ad912487e5ad3e19606fae11ceed0951a6248b43 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 22 Feb 2026 10:59:02 -0800 Subject: [PATCH 1/9] fix issue with optional fields in dependency validator --- .../pants/backend/python/target_types_test.py | 108 +++++++++++++++++- src/python/pants/engine/target.py | 25 +++- src/python/pants/engine/target_test.py | 9 ++ 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 9e2ed3e527b..3bd03b0a651 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -10,6 +10,7 @@ import pytest from pants.backend.python import target_types_rules +from pants.backend.python.dependency_inference import rules as python_dependency_inference_rules from pants.backend.python.dependency_inference.rules import import_rules from pants.backend.python.macros.python_artifact import PythonArtifact from pants.backend.python.target_types import ( @@ -21,16 +22,20 @@ PexEntryPointField, PexExecutableField, PexScriptField, + PythonDependenciesField, PythonDistribution, PythonRequirementsField, PythonRequirementTarget, + PythonResolveField, PythonSourcesGeneratorTarget, + PythonSourceTarget, ResolvedPexEntryPoint, ResolvePexEntryPointRequest, ResolvePythonDistributionEntryPointsRequest, normalize_module_mapping, ) from pants.backend.python.target_types_rules import ( + DependencyValidationFieldSet, InferPexBinaryEntryPointDependency, InferPythonDistributionDependencies, PexBinaryEntryPointDependencyInferenceFieldSet, @@ -40,10 +45,12 @@ from pants.backend.python.util_rules import python_sources from pants.core.goals.generate_lockfiles import UnrecognizedResolveNamesError from pants.core.util_rules.unowned_dependency_behavior import UnownedDependencyError -from pants.engine.addresses import Address +from pants.engine.addresses import Address, Addresses from pants.engine.internals.graph import _TargetParametrizations, _TargetParametrizationsRequest +from pants.engine.internals.parametrize import Parametrize from pants.engine.internals.scheduler import ExecutionError from pants.engine.target import ( + DependenciesRequest, InferredDependencies, InvalidFieldException, InvalidFieldTypeException, @@ -610,3 +617,102 @@ def gen_pex_binary_tgt(entry_point: str, tags: list[str] | None = None) -> PexBi gen_pex_binary_tgt("subdir.f.py", tags=["overridden"]), gen_pex_binary_tgt("subdir.f:main"), } + + +def test_python_dependency_validation_with_parametrized_resolve_repro() -> None: + rule_runner = RuleRunner( + rules=[ + *target_types_rules.rules(), + *python_sources.rules(), + *python_dependency_inference_rules.rules(), + QueryRule(_TargetParametrizations, [_TargetParametrizationsRequest]), + QueryRule(Addresses, [DependenciesRequest]), + ], + target_types=[PythonSourceTarget, PythonSourcesGeneratorTarget], + objects={"parametrize": Parametrize}, + ) + rule_runner.write_files( + { + "pants.toml": dedent( + """\ + [python] + enable_resolves = true + default_resolve = "a" + interpreter_constraints = ["==3.11.*", "==3.14.*"] + interpreter_versions_universe = ["3.11", "3.14"] + default_to_resolve_interpreter_constraints = true + + [python.resolves] + "a" = "a.lock" + "b" = "b.lock" + + [python.resolves_to_interpreter_constraints] + "a" = ["==3.11.*"] + "b" = ["==3.14.*"] + + [python-infer] + imports = true + + [source] + root_patterns = ["/src/python"] + """ + ), + "src/python/pkg/base.py": "class Base:\n pass\n", + "src/python/pkg/derived.py": ( + "from pkg.base import Base\n\nclass Derived(Base):\n pass\n" + ), + "src/python/pkg/BUILD": dedent( + """\ + python_sources( + sources=["base.py", "derived.py"], + **parametrize("b", resolve="b"), + **parametrize("a", resolve="a"), + overrides={ + "derived.py": { + "dependencies": ["./base.py"], + }, + }, + ) + """ + ), + } + ) + rule_runner.set_options(["--pants-config-files=['pants.toml']"]) + generated_targets = [ + tgt + for tgt in rule_runner.request( + _TargetParametrizations, + [ + _TargetParametrizationsRequest( + Address("src/python/pkg"), description_of_origin="tests" + ) + ], + ).parametrizations.values() + if isinstance(tgt, PythonSourceTarget) + ] + assert len(generated_targets) == 4 + base_a = next( + tgt + for tgt in generated_targets + if tgt.address.relative_file_path == "base.py" + and tgt.address.parameters.get("parametrize") == "a" + ) + derived_a = next( + tgt + for tgt in generated_targets + if tgt.address.relative_file_path == "derived.py" + and tgt.address.parameters.get("parametrize") == "a" + ) + + # The generated targets have the correct resolve. + assert base_a[PythonResolveField].value == "a" + assert derived_a[PythonResolveField].value == "a" + validation_field_set = DependencyValidationFieldSet.create(derived_a) + assert validation_field_set.resolve is not None + assert validation_field_set.resolve.value == "a" + + resolved = rule_runner.request( + Addresses, + [DependenciesRequest(derived_a[PythonDependenciesField])], + ) + assert tuple(resolved) == (base_a.address,) diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 452dbf701ee..6527de4e3c3 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -11,6 +11,7 @@ import logging import os.path import textwrap +from types import UnionType import zlib from abc import ABC, ABCMeta, abstractmethod from collections import deque @@ -26,9 +27,12 @@ Generic, Protocol, Self, + Union, TypeVar, cast, final, + get_args, + get_origin, get_type_hints, ) @@ -1475,11 +1479,28 @@ def create(cls: type[_FS], tgt: Target) -> _FS: @final @memoized_classproperty def fields(cls) -> FrozenDict[str, type[Field]]: + def field_type_from_annotation(annotation: Any) -> type[Field] | None: + if isinstance(annotation, type) and issubclass(annotation, Field): + return annotation + + origin = get_origin(annotation) + if origin not in (Union, UnionType): + return None + + field_types = [ + arg + for arg in get_args(annotation) + if isinstance(arg, type) and issubclass(arg, Field) + ] + if len(field_types) != 1: + return None + return field_types[0] + return FrozenDict( ( (name, field_type) - for name, field_type in get_type_hints(cls).items() - if isinstance(field_type, type) and issubclass(field_type, Field) + for name, annotation in get_type_hints(cls).items() + if (field_type := field_type_from_annotation(annotation)) is not None ) ) diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index 21796845c62..e470df18e50 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -643,6 +643,12 @@ class OptionalFieldSet(FieldSet): def opt_out(cls, tgt: Target) -> bool: return tgt.get(OptOutField).value is True + @dataclass(frozen=True) + class OptionalUnionFieldSet(FieldSet): + required_fields = () + + optional: OptionalField | None = None + required_addr = Address("", target_name="required") required_tgt = TargetWithRequired({RequiredField.alias: "configured"}, required_addr) optional_addr = Address("", target_name="unrelated") @@ -682,6 +688,9 @@ def opt_out(cls, tgt: Target) -> bool: assert OptionalFieldSet.create(optional_tgt).optional.value == "configured" assert OptionalFieldSet.create(no_fields_tgt).optional.value == OptionalField.default + assert OptionalUnionFieldSet.fields == FrozenDict({"optional": OptionalField}) + assert OptionalUnionFieldSet.create(optional_tgt).optional.value == "configured" + assert OptionalUnionFieldSet.create(no_fields_tgt).optional.value == OptionalField.default # ----------------------------------------------------------------------------------------------- From 7ba7d64a999b601abf07d5bb55a023c0c6c2e620 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 22 Feb 2026 12:49:41 -0800 Subject: [PATCH 2/9] pants fix --- src/python/pants/engine/target.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 6527de4e3c3..553c86bbb05 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -11,7 +11,6 @@ import logging import os.path import textwrap -from types import UnionType import zlib from abc import ABC, ABCMeta, abstractmethod from collections import deque @@ -20,6 +19,7 @@ from enum import Enum from operator import attrgetter from pathlib import PurePath +from types import UnionType from typing import ( AbstractSet, Any, @@ -27,8 +27,8 @@ Generic, Protocol, Self, - Union, TypeVar, + Union, cast, final, get_args, From c7299df9c6c5253795b1f693f55ad4f1c3745f99 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 22 Feb 2026 12:51:47 -0800 Subject: [PATCH 3/9] typing fix --- src/python/pants/engine/target_test.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index e470df18e50..08b8bf6a2a0 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -689,8 +689,12 @@ class OptionalUnionFieldSet(FieldSet): assert OptionalFieldSet.create(optional_tgt).optional.value == "configured" assert OptionalFieldSet.create(no_fields_tgt).optional.value == OptionalField.default assert OptionalUnionFieldSet.fields == FrozenDict({"optional": OptionalField}) - assert OptionalUnionFieldSet.create(optional_tgt).optional.value == "configured" - assert OptionalUnionFieldSet.create(no_fields_tgt).optional.value == OptionalField.default + optional_union_field = OptionalUnionFieldSet.create(optional_tgt).optional + assert optional_union_field is not None + assert optional_union_field.value == "configured" + default_union_field = OptionalUnionFieldSet.create(no_fields_tgt).optional + assert default_union_field is not None + assert default_union_field.value == OptionalField.default # ----------------------------------------------------------------------------------------------- From a8b8903b07c753b57d77d5a691b45eae1ef96164 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 22 Feb 2026 13:04:25 -0800 Subject: [PATCH 4/9] update release notes --- docs/notes/2.32.x.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/notes/2.32.x.md b/docs/notes/2.32.x.md index 02a049faa84..bb2148ad805 100644 --- a/docs/notes/2.32.x.md +++ b/docs/notes/2.32.x.md @@ -67,6 +67,8 @@ The `runtime` field of [`aws_python_lambda_layer`](https://www.pantsbuild.org/2. The `grpc-python-plugin` tool now uses an updated `v1.73.1` plugin built from Date: Sun, 22 Feb 2026 13:38:33 -0800 Subject: [PATCH 5/9] fix incorrect python-default resolve --- src/python/pants/backend/python/goals/export_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index 5c10b47f209..fdf695c354b 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -110,6 +110,7 @@ def test_export_venv_new_codepath( [ *pants_args_for_python_lockfiles, f"--python-interpreter-constraints=['=={current_interpreter}']", + "--python-default-resolve=a", "--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}", "--export-resolve=a", "--export-resolve=b", From 0e365709f4e50d4a3fc36fa82fd7cd2c1534339d Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 22 Feb 2026 14:42:10 -0800 Subject: [PATCH 6/9] more export python_distribution fixes --- src/python/pants/backend/python/goals/export_integration_test.py | 1 + .../pants/backend/python/util_rules/local_dists_pep660_test.py | 1 + 2 files changed, 2 insertions(+) diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index 8288e70e378..e55c869af96 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -48,6 +48,7 @@ def build_config( "python": { "enable_resolves": True, "interpreter_constraints": [f"=={platform.python_version()}"], + "default_resolve": "a", "resolves": { "a": "3rdparty/a.lock", "b": "3rdparty/b.lock", diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index e4d0a6125d4..3c1d1ece32d 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -95,6 +95,7 @@ def test_sort_all_python_distributions_by_resolve(rule_runner: PythonRuleRunner) rule_runner.set_options( [ "--python-enable-resolves=True", + "--python-default-resolve=a", "--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}", # Turn off lockfile validation to make the test simpler. "--python-invalid-lockfile-behavior=ignore", From d8e55baca4725eee14d8f2d442cf094a0ecf04d3 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 1 Mar 2026 14:38:52 -0800 Subject: [PATCH 7/9] none_on_absence_fields --- .../pants/backend/python/target_types_test.py | 38 ++++++++++- src/python/pants/engine/target.py | 65 +++++++++++++++---- src/python/pants/engine/target_test.py | 9 ++- 3 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 3bd03b0a651..ef682376a31 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging +from dataclasses import dataclass from collections.abc import Iterable from textwrap import dedent @@ -51,11 +52,14 @@ from pants.engine.internals.scheduler import ExecutionError from pants.engine.target import ( DependenciesRequest, + FieldSet, InferredDependencies, InvalidFieldException, InvalidFieldTypeException, InvalidTargetException, + StringField, Tags, + Target, ) from pants.testutil.rule_runner import QueryRule, RuleRunner from pants.util.frozendict import FrozenDict @@ -637,7 +641,6 @@ def test_python_dependency_validation_with_parametrized_resolve_repro() -> None: """\ [python] enable_resolves = true - default_resolve = "a" interpreter_constraints = ["==3.11.*", "==3.14.*"] interpreter_versions_universe = ["3.11", "3.14"] default_to_resolve_interpreter_constraints = true @@ -716,3 +719,36 @@ def test_python_dependency_validation_with_parametrized_resolve_repro() -> None: [DependenciesRequest(derived_a[PythonDependenciesField])], ) assert tuple(resolved) == (base_a.address,) + + +class _ReproRequiredField(StringField): + alias = "required" + required = True + + +class _ReproOptionalField(StringField): + alias = "optional" + + +class _ReproTarget(Target): + alias = "repro_target" + core_fields = (_ReproRequiredField,) + + +@dataclass(frozen=True) +class _ReproFieldSet(FieldSet): + required_fields = (_ReproRequiredField,) + + required: _ReproRequiredField + optional: _ReproOptionalField | None = None + + +def test_field_set_optional_union_field_is_none_when_target_lacks_field_repro() -> None: + target = _ReproTarget( + {_ReproRequiredField.alias: "value"}, + Address("src/python/pkg", target_name="repro"), + ) + field_set = _ReproFieldSet.create(target) + + assert field_set.required.value == "value" + assert field_set.optional is None diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 553c86bbb05..b6bfef65d66 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -1385,13 +1385,24 @@ def gen_tgt(address: Address, full_fp: str, generated_target_fields: dict[str, A # ----------------------------------------------------------------------------------------------- def _get_field_set_fields_from_target( field_set: type[FieldSet], target: Target -) -> dict[str, Field]: - return { - dataclass_field_name: ( - target[field_cls] if field_cls in field_set.required_fields else target.get(field_cls) - ) - for dataclass_field_name, field_cls in field_set.fields.items() - } +) -> dict[str, Field | None]: + result: dict[str, Field | None] = {} + for dataclass_field_name, field_cls in field_set.fields.items(): + if field_cls in field_set.required_fields: + result[dataclass_field_name] = target[field_cls] + continue + + if dataclass_field_name in field_set.none_on_absence_fields and not target.has_field( + field_cls + ): + # Preserve true optionality for `Field | None` annotations when the target type + # doesn't define that field. + result[dataclass_field_name] = None + continue + + result[dataclass_field_name] = target.get(field_cls) + + return result _FS = TypeVar("_FS", bound="FieldSet") @@ -1403,8 +1414,9 @@ class FieldSet(EngineAwareParameter, metaclass=ABCMeta): Subclasses should declare all the fields they consume as dataclass attributes. They should also indicate which of these are required, rather than optional, through the class property - `required_fields`. When a field is optional, the default constructor for the field will be used - for any targets that do not have that field registered. + `required_fields`. For fields annotated as `Field | None`, Pants will preserve `None` when the + target type does not have that field registered. For non-union `Field` annotations, Pants will + construct the default field value when the target type does not have that field registered. Subclasses must set `@dataclass(frozen=True)` for their declared fields to be recognized. @@ -1479,9 +1491,9 @@ def create(cls: type[_FS], tgt: Target) -> _FS: @final @memoized_classproperty def fields(cls) -> FrozenDict[str, type[Field]]: - def field_type_from_annotation(annotation: Any) -> type[Field] | None: + def field_type_from_annotation(annotation: Any) -> tuple[type[Field], bool] | None: if isinstance(annotation, type) and issubclass(annotation, Field): - return annotation + return annotation, False origin = get_origin(annotation) if origin not in (Union, UnionType): @@ -1494,14 +1506,41 @@ def field_type_from_annotation(annotation: Any) -> type[Field] | None: ] if len(field_types) != 1: return None - return field_types[0] + return field_types[0], (type(None) in get_args(annotation)) return FrozenDict( ( (name, field_type) for name, annotation in get_type_hints(cls).items() - if (field_type := field_type_from_annotation(annotation)) is not None + if (field_type_info := field_type_from_annotation(annotation)) is not None + for field_type, _ in (field_type_info,) + ) + ) + + @final + @memoized_classproperty + def none_on_absence_fields(cls) -> FrozenOrderedSet[str]: + def is_optional_field_annotation(annotation: Any) -> bool: + origin = get_origin(annotation) + if origin not in (Union, UnionType): + return False + + return ( + type(None) in get_args(annotation) + and len( + [ + arg + for arg in get_args(annotation) + if isinstance(arg, type) and issubclass(arg, Field) + ] + ) + == 1 ) + + return FrozenOrderedSet( + name + for name, annotation in get_type_hints(cls).items() + if is_optional_field_annotation(annotation) ) def debug_hint(self) -> str: diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index 08b8bf6a2a0..b20a12978e9 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -653,6 +653,8 @@ class OptionalUnionFieldSet(FieldSet): required_tgt = TargetWithRequired({RequiredField.alias: "configured"}, required_addr) optional_addr = Address("", target_name="unrelated") optional_tgt = TargetWithoutRequired({OptionalField.alias: "configured"}, optional_addr) + optional_default_addr = Address("", target_name="optional_default") + optional_default_tgt = TargetWithoutRequired({}, optional_default_addr) no_fields_addr = Address("", target_name="no_fields") no_fields_tgt = NoFieldsTarget({}, no_fields_addr) opt_out_addr = Address("", target_name="conditional") @@ -689,12 +691,15 @@ class OptionalUnionFieldSet(FieldSet): assert OptionalFieldSet.create(optional_tgt).optional.value == "configured" assert OptionalFieldSet.create(no_fields_tgt).optional.value == OptionalField.default assert OptionalUnionFieldSet.fields == FrozenDict({"optional": OptionalField}) + assert OptionalUnionFieldSet.none_on_absence_fields == FrozenOrderedSet(["optional"]) optional_union_field = OptionalUnionFieldSet.create(optional_tgt).optional assert optional_union_field is not None assert optional_union_field.value == "configured" + default_registered_union_field = OptionalUnionFieldSet.create(optional_default_tgt).optional + assert default_registered_union_field is not None + assert default_registered_union_field.value == OptionalField.default default_union_field = OptionalUnionFieldSet.create(no_fields_tgt).optional - assert default_union_field is not None - assert default_union_field.value == OptionalField.default + assert default_union_field is None # ----------------------------------------------------------------------------------------------- From c44e094397a47bea9bceeb26f872f9417a6eab96 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 1 Mar 2026 14:43:38 -0800 Subject: [PATCH 8/9] fmt and remove export test work-arounds --- .../pants/backend/python/goals/export_integration_test.py | 1 - src/python/pants/backend/python/goals/export_test.py | 1 - src/python/pants/backend/python/target_types_test.py | 2 +- .../pants/backend/python/util_rules/local_dists_pep660_test.py | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/goals/export_integration_test.py b/src/python/pants/backend/python/goals/export_integration_test.py index e55c869af96..8288e70e378 100644 --- a/src/python/pants/backend/python/goals/export_integration_test.py +++ b/src/python/pants/backend/python/goals/export_integration_test.py @@ -48,7 +48,6 @@ def build_config( "python": { "enable_resolves": True, "interpreter_constraints": [f"=={platform.python_version()}"], - "default_resolve": "a", "resolves": { "a": "3rdparty/a.lock", "b": "3rdparty/b.lock", diff --git a/src/python/pants/backend/python/goals/export_test.py b/src/python/pants/backend/python/goals/export_test.py index fdf695c354b..5c10b47f209 100644 --- a/src/python/pants/backend/python/goals/export_test.py +++ b/src/python/pants/backend/python/goals/export_test.py @@ -110,7 +110,6 @@ def test_export_venv_new_codepath( [ *pants_args_for_python_lockfiles, f"--python-interpreter-constraints=['=={current_interpreter}']", - "--python-default-resolve=a", "--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}", "--export-resolve=a", "--export-resolve=b", diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index ef682376a31..43c81e04dcf 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -4,8 +4,8 @@ from __future__ import annotations import logging -from dataclasses import dataclass from collections.abc import Iterable +from dataclasses import dataclass from textwrap import dedent import pytest diff --git a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py index 3c1d1ece32d..e4d0a6125d4 100644 --- a/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py +++ b/src/python/pants/backend/python/util_rules/local_dists_pep660_test.py @@ -95,7 +95,6 @@ def test_sort_all_python_distributions_by_resolve(rule_runner: PythonRuleRunner) rule_runner.set_options( [ "--python-enable-resolves=True", - "--python-default-resolve=a", "--python-resolves={'a': 'lock.txt', 'b': 'lock.txt'}", # Turn off lockfile validation to make the test simpler. "--python-invalid-lockfile-behavior=ignore", From 24d3ccbe5505f91ab83a5a0a954785fdcca13873 Mon Sep 17 00:00:00 2001 From: Tom Dyas Date: Sun, 1 Mar 2026 14:59:12 -0800 Subject: [PATCH 9/9] runtime check for "correct" field set types --- src/python/pants/engine/target.py | 80 ++++++++++++++++---------- src/python/pants/engine/target_test.py | 24 ++++++++ 2 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index b6bfef65d66..8c486ca973c 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -1490,7 +1490,7 @@ def create(cls: type[_FS], tgt: Target) -> _FS: @final @memoized_classproperty - def fields(cls) -> FrozenDict[str, type[Field]]: + def _field_info(cls) -> FrozenDict[str, tuple[type[Field], bool]]: def field_type_from_annotation(annotation: Any) -> tuple[type[Field], bool] | None: if isinstance(annotation, type) and issubclass(annotation, Field): return annotation, False @@ -1499,48 +1499,66 @@ def field_type_from_annotation(annotation: Any) -> tuple[type[Field], bool] | No if origin not in (Union, UnionType): return None + union_args = get_args(annotation) field_types = [ - arg - for arg in get_args(annotation) - if isinstance(arg, type) and issubclass(arg, Field) + arg for arg in union_args if isinstance(arg, type) and issubclass(arg, Field) ] if len(field_types) != 1: return None - return field_types[0], (type(None) in get_args(annotation)) - return FrozenDict( - ( - (name, field_type) - for name, annotation in get_type_hints(cls).items() - if (field_type_info := field_type_from_annotation(annotation)) is not None - for field_type, _ in (field_type_info,) + preserve_none_on_absence = type(None) in union_args + # Only allow optional Field annotations (`Field | None`). + if not preserve_none_on_absence or len(union_args) != 2: + return None + + return field_types[0], True + + type_hints = get_type_hints(cls) + base_dataclass_field_names = {f.name for f in dataclasses.fields(FieldSet)} + parsed: dict[str, tuple[type[Field], bool]] = {} + invalid_dataclass_fields: dict[str, Any] = {} + + for dataclass_field in dataclasses.fields(cls): + if dataclass_field.name in base_dataclass_field_names: + continue + + annotation = type_hints[dataclass_field.name] + parsed_annotation = field_type_from_annotation(annotation) + if parsed_annotation is None: + invalid_dataclass_fields[dataclass_field.name] = annotation + continue + + parsed[dataclass_field.name] = parsed_annotation + + if invalid_dataclass_fields: + field_set_name = getattr(cls, "__name__", type(cls).__name__) + invalid_field_descriptions = ", ".join( + f"{name}: {annotation!r}" + for name, annotation in sorted(invalid_dataclass_fields.items()) + ) + raise TypeError( + f"The FieldSet `{field_set_name}` has invalid dataclass field annotations. " + "Every declared dataclass field on a FieldSet must be annotated with a " + "`Field` subclass or `Field | None`. Invalid fields: " + f"{invalid_field_descriptions}" ) + + return FrozenDict(parsed) + + @final + @memoized_classproperty + def fields(cls) -> FrozenDict[str, type[Field]]: + return FrozenDict( + (field_name, field_type) for field_name, (field_type, _) in cls._field_info.items() ) @final @memoized_classproperty def none_on_absence_fields(cls) -> FrozenOrderedSet[str]: - def is_optional_field_annotation(annotation: Any) -> bool: - origin = get_origin(annotation) - if origin not in (Union, UnionType): - return False - - return ( - type(None) in get_args(annotation) - and len( - [ - arg - for arg in get_args(annotation) - if isinstance(arg, type) and issubclass(arg, Field) - ] - ) - == 1 - ) - return FrozenOrderedSet( - name - for name, annotation in get_type_hints(cls).items() - if is_optional_field_annotation(annotation) + field_name + for field_name, (_, preserve_none_on_absence) in cls._field_info.items() + if preserve_none_on_absence ) def debug_hint(self) -> str: diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index b20a12978e9..ea96a10ec36 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -702,6 +702,30 @@ class OptionalUnionFieldSet(FieldSet): assert default_union_field is None +def test_field_set_invalid_annotations() -> None: + class ValidField(StringField): + alias = "valid" + default = "default" + + @dataclass(frozen=True) + class InvalidStringFieldSet(FieldSet): + required_fields = () + + not_a_field: str + + with pytest.raises(TypeError, match="`Field` subclass or `Field \\| None`"): + _ = InvalidStringFieldSet.fields + + @dataclass(frozen=True) + class InvalidUnionFieldSet(FieldSet): + required_fields = () + + invalid: ValidField | str + + with pytest.raises(TypeError, match="`Field` subclass or `Field \\| None`"): + _ = InvalidUnionFieldSet.none_on_absence_fields + + # ----------------------------------------------------------------------------------------------- # Test Field templates # -----------------------------------------------------------------------------------------------