From cdc81d0ce3092eb7e78f1ca7c41e8a113f56ab10 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 15 Oct 2021 12:01:41 -0700 Subject: [PATCH] Review feedback [ci skip-rust] [ci skip-build-wheels] --- src/python/pants/backend/project_info/peek.py | 4 +--- .../pants/backend/project_info/peek_test.py | 2 +- src/python/pants/engine/target.py | 15 +++++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/python/pants/backend/project_info/peek.py b/src/python/pants/backend/project_info/peek.py index 377fb7f1d75..5ad2c6b721a 100644 --- a/src/python/pants/backend/project_info/peek.py +++ b/src/python/pants/backend/project_info/peek.py @@ -113,9 +113,7 @@ def render_json(tds: Iterable[TargetData], exclude_defaults: bool = False) -> st nothing = object() def normalize_value(val: Any) -> Any: - if isinstance(val, collections.abc.Mapping) and not all( - isinstance(k, str) for k in val.keys() - ): + if isinstance(val, collections.abc.Mapping): return {str(k): normalize_value(v) for k, v in val.items()} return val diff --git a/src/python/pants/backend/project_info/peek_test.py b/src/python/pants/backend/project_info/peek_test.py index e8874efaea8..91217324e1f 100644 --- a/src/python/pants/backend/project_info/peek_test.py +++ b/src/python/pants/backend/project_info/peek_test.py @@ -17,7 +17,7 @@ # TODO: Remove this if/when we add an overrides field to `files`. class FilesGeneratorTargetWithOverrides(FilesGeneratorTarget): - core_fields = (*FilesGeneratorTarget.core_fields, OverridesField) + core_fields = (*FilesGeneratorTarget.core_fields, OverridesField) # type: ignore[assignment] @pytest.mark.parametrize( diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 0cef681bc2e..21741da6b37 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -607,7 +607,7 @@ class Targets(Collection[Target]): """A heterogeneous collection of instances of Target subclasses. While every element will be a subclass of `Target`, there may be many different `Target` types - in this collection, e.g. some `Files` targets and some `PythonLibrary` targets. + in this collection, e.g. some `FileTarget` and some `PythonTestTarget`. Often, you will want to filter out the relevant targets by looking at what fields they have registered, e.g.: @@ -615,7 +615,8 @@ class Targets(Collection[Target]): valid_tgts = [tgt for tgt in tgts if tgt.has_fields([Compatibility, PythonSources])] You should not check the Target's actual type because this breaks custom target types; - for example, prefer `tgt.has_field(PythonTestsSources)` to `isinstance(tgt, PythonTests)`. + for example, prefer `tgt.has_field(PythonTestsSourcesField)` to + `isinstance(tgt, PythonTestsTarget)`. """ def expect_single(self) -> Target: @@ -901,6 +902,7 @@ def generate_file_level_targets( ) used_overrides = set() + normalized_overrides = overrides or {} def gen_tgt(full_fp: str, address: Address) -> Target: generated_target_fields: dict[str, ImmutableValue] = {} @@ -926,9 +928,9 @@ def gen_tgt(full_fp: str, address: Address) -> Target: elif field.value != field.default: generated_target_fields[field.alias] = field.value - if full_fp in (overrides or {}): + if full_fp in normalized_overrides: used_overrides.add(full_fp) - generated_target_fields.update(overrides[full_fp]) + generated_target_fields.update(normalized_overrides[full_fp]) return generated_target_cls( generated_target_fields, @@ -939,13 +941,14 @@ def gen_tgt(full_fp: str, address: Address) -> Target: result = tuple(gen_tgt(fp, address) for fp, address in zip(paths, all_generated_addresses)) - unused_overrides = set(overrides or {}) - used_overrides + unused_overrides = set(normalized_overrides.keys()) - used_overrides if unused_overrides: unused_relative_paths = sorted( fast_relpath(fp, generator.address.spec_path) for fp in unused_overrides ) all_valid_relative_paths = sorted( - tgt.address._relative_file_path or tgt.address.generated_name for tgt in result + cast(str, tgt.address._relative_file_path or tgt.address.generated_name) + for tgt in result ) raise InvalidFieldException( f"Unused file paths in the `overrides` field for {generator.address}: "