Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Oct 19, 2021
1 parent 859a297 commit cdc81d0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 10 deletions.
4 changes: 1 addition & 3 deletions src/python/pants/backend/project_info/peek.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/project_info/peek_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 9 additions & 6 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -607,15 +607,16 @@ 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.:
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:
Expand Down Expand Up @@ -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] = {}
Expand All @@ -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,
Expand All @@ -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}: "
Expand Down

0 comments on commit cdc81d0

Please sign in to comment.