Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[internal] Add infrastructure to support deprecating field names #13666

Merged
merged 4 commits into from
Nov 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 139 additions & 1 deletion src/python/pants/core/goals/update_build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from collections import defaultdict
from dataclasses import dataclass
from io import BytesIO
from typing import DefaultDict, cast
from typing import DefaultDict, Mapping, cast

from colors import green, red

Expand Down Expand Up @@ -380,11 +380,149 @@ def should_be_renamed(token: tokenize.TokenInfo) -> bool:
)


# ------------------------------------------------------------------------------------------
# Rename deprecated field types fixer
# ------------------------------------------------------------------------------------------


class RenameDeprecatedFieldsRequest(DeprecationFixerRequest):
pass


@dataclass(frozen=True)
class RenamedFieldTypes:
"""Map deprecated field names to their new name, per target."""

target_field_renames: FrozenDict[str, FrozenDict[str, str]]

@classmethod
def from_dict(cls, data: Mapping[str, Mapping[str, str]]) -> RenamedFieldTypes:
return cls(
FrozenDict(
{
target_name: FrozenDict(
{
deprecated_field_name: new_field_name
for deprecated_field_name, new_field_name in field_renames.items()
}
)
for target_name, field_renames in data.items()
}
)
)


@rule
def determine_renamed_field_types(
target_types: RegisteredTargetTypes, union_membership: UnionMembership
) -> RenamedFieldTypes:
target_field_renames: DefaultDict[str, dict[str, str]] = defaultdict(dict)
for tgt in target_types.types:
field_renames = target_field_renames[tgt.alias]
if tgt.deprecated_alias is not None:
target_field_renames[tgt.deprecated_alias] = field_renames

for field_type in tgt.class_field_types(union_membership):
if field_type.deprecated_alias is not None:
field_renames[field_type.deprecated_alias] = field_type.alias

return RenamedFieldTypes.from_dict(target_field_renames)


@rule(desc="Check for deprecated field type names", level=LogLevel.DEBUG)
def maybe_rename_deprecated_fields(
request: RenameDeprecatedFieldsRequest,
renamed_field_types: RenamedFieldTypes,
) -> RewrittenBuildFile:
pants_target: str = ""
level: int = 0
applied_renames: set[tuple[str, str, str]] = set()
tokens = iter(request.tokenize())

def parse_level(token: tokenize.TokenInfo) -> bool:
"""Returns true if token was consumed."""
nonlocal level

if level == 0 or token.type is not tokenize.OP or token.string not in ["(", ")"]:
return False

if token.string == "(":
level += 1
elif token.string == ")":
level -= 1

return True

def parse_target(token: tokenize.TokenInfo) -> bool:
"""Returns true if we're parsing a field name for a top level target."""
nonlocal pants_target
nonlocal level

if parse_level(token):
# Consumed parenthesis operator.
return False

if token.type is not tokenize.NAME:
return False

if level == 0 and next_token_is("("):
level = 1
pants_target = token.string
# Current token consumed.
return False

return level == 1

def next_token_is(string: str, token_type=tokenize.OP) -> bool:
for next_token in tokens:
if next_token.type is tokenize.NL:
continue
parse_level(next_token)
return next_token.type is token_type and next_token.string == string
return False

def should_be_renamed(token: tokenize.TokenInfo) -> bool:
nonlocal pants_target

if not parse_target(token):
return False

if pants_target not in renamed_field_types.target_field_renames:
return False

return (
next_token_is("=")
and token.string in renamed_field_types.target_field_renames[pants_target]
)

updated_text_lines = list(request.lines)
for token in tokens:
if not should_be_renamed(token):
continue
line_index = token.start[0] - 1
line = request.lines[line_index]
prefix = line[: token.start[1]]
suffix = line[token.end[1] :]
new_symbol = renamed_field_types.target_field_renames[pants_target][token.string]
applied_renames.add((pants_target, token.string, new_symbol))
updated_text_lines[line_index] = f"{prefix}{new_symbol}{suffix}"

return RewrittenBuildFile(
request.path,
tuple(updated_text_lines),
change_descriptions=tuple(
f"Rename the field `{request.red(deprecated)}` to `{request.green(new)}` for target type `{target}`"
for target, deprecated, new in sorted(applied_renames)
),
)


def rules():
return (
*collect_rules(),
*pex.rules(),
UnionRule(RewrittenBuildFileRequest, RenameDeprecatedTargetsRequest),
UnionRule(RewrittenBuildFileRequest, RenameDeprecatedFieldsRequest),
# NB: We want this to come at the end so that running Black happens after all our
# deprecation fixers.
UnionRule(RewrittenBuildFileRequest, FormatWithBlackRequest),
Expand Down
49 changes: 49 additions & 0 deletions src/python/pants/core/goals/update_build_files_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@
from pants.backend.python.util_rules import pex
from pants.core.goals.update_build_files import (
FormatWithBlackRequest,
RenameDeprecatedFieldsRequest,
RenameDeprecatedTargetsRequest,
RenamedFieldTypes,
RenamedTargetTypes,
RewrittenBuildFile,
RewrittenBuildFileRequest,
UpdateBuildFilesGoal,
UpdateBuildFilesSubsystem,
format_build_file_with_black,
maybe_rename_deprecated_fields,
maybe_rename_deprecated_targets,
update_build_files,
)
Expand Down Expand Up @@ -225,3 +228,49 @@ def test_rename_deprecated_target_types_rewrite(lines: list[str], expected: list
)
assert result.change_descriptions
assert result.lines == tuple(expected)


# ------------------------------------------------------------------------------------------
# Renamed field types fixer
# ------------------------------------------------------------------------------------------


@pytest.mark.parametrize(
"lines",
(
# Already valid.
["target(new_name='')"],
["target(new_name = 56 ) "],
["target(foo=1, new_name=2)"],
["target(", "new_name", "=3)"],
# Unrelated lines.
["", "123", "target()", "name='new_name'"],
["unaffected(deprecated_name='not this target')"],
["target(nested=here(deprecated_name='too deep'))"],
),
)
def test_rename_deprecated_field_types_noops(lines: list[str]) -> None:
result = maybe_rename_deprecated_fields(
RenameDeprecatedFieldsRequest("BUILD", tuple(lines), colors_enabled=False),
RenamedFieldTypes.from_dict({"target": {"deprecated_name": "new_name"}}),
)
assert not result.change_descriptions
assert result.lines == tuple(lines)


@pytest.mark.parametrize(
"lines,expected",
(
(["tgt1(deprecated_name='')"], ["tgt1(new_name='')"]),
(["tgt1 ( deprecated_name = ' ', ", ")"], ["tgt1 ( new_name = ' ', ", ")"]),
(["tgt1(deprecated_name='') # comment"], ["tgt1(new_name='') # comment"]),
(["tgt1(", "deprecated_name", "=", ")"], ["tgt1(", "new_name", "=", ")"]),
),
)
def test_rename_deprecated_field_types_rewrite(lines: list[str], expected: list[str]) -> None:
result = maybe_rename_deprecated_fields(
RenameDeprecatedTargetsRequest("BUILD", tuple(lines), colors_enabled=False),
RenamedFieldTypes.from_dict({"tgt1": {"deprecated_name": "new_name"}}),
)
assert result.change_descriptions
assert result.lines == tuple(expected)
33 changes: 33 additions & 0 deletions src/python/pants/engine/internals/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
Dependencies,
DependenciesRequest,
ExplicitlyProvidedDependencies,
Field,
FieldSet,
FieldSetsPerTarget,
FieldSetsPerTargetRequest,
Expand Down Expand Up @@ -141,6 +142,32 @@ def warn_deprecated_target_type(request: _WarnDeprecatedTargetRequest) -> _WarnD
return _WarnDeprecatedTarget()


# We use a rule for this warning so that it gets memoized, i.e. doesn't get repeated for every
# offending field.
class _WarnDeprecatedField:
pass


@dataclass(frozen=True)
class _WarnDeprecatedFieldRequest:
field_type: type[Field]


@rule
def warn_deprecated_field_type(request: _WarnDeprecatedFieldRequest) -> _WarnDeprecatedField:
field_type = request.field_type
assert field_type.deprecated_alias_removal_version is not None
warn_or_error(
removal_version=field_type.deprecated_alias_removal_version,
entity=f"the field name {field_type.deprecated_alias}",
hint=(
f"Instead, use `{field_type.alias}`, which behaves the same. Run `./pants "
"update-build-files` to automatically fix your BUILD files."
),
)
return _WarnDeprecatedField()


@rule
async def resolve_target(
address: Address,
Expand All @@ -162,6 +189,12 @@ async def resolve_target(
):
await Get(_WarnDeprecatedTarget, _WarnDeprecatedTargetRequest(target_type))
target = target_type(target_adaptor.kwargs, address, union_membership)
for field_type in target.field_types:
if (
field_type.deprecated_alias is not None
and field_type.deprecated_alias in target_adaptor.kwargs
):
await Get(_WarnDeprecatedField, _WarnDeprecatedFieldRequest(field_type))
return WrappedTarget(target)

wrapped_generator_tgt = await Get(
Expand Down
11 changes: 10 additions & 1 deletion src/python/pants/engine/internals/graph_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@

class MockDependencies(Dependencies):
supports_transitive_excludes = True
deprecated_alias = "deprecated_field"
deprecated_alias_removal_version = "9.9.9.dev0"


class SpecialCasedDeps1(SpecialCasedDependencies):
Expand Down Expand Up @@ -540,11 +542,18 @@ def test_dep_no_cycle_indirect(transitive_targets_rule_runner: RuleRunner) -> No
}


def test_deprecated_field_name(transitive_targets_rule_runner: RuleRunner, caplog) -> None:
transitive_targets_rule_runner.write_files({"BUILD": "target(name='t', deprecated_field=[])"})
transitive_targets_rule_runner.get_target(Address("", target_name="t"))
assert len(caplog.records) == 1
assert "Instead, use `dependencies`" in caplog.text


def test_resolve_deprecated_target_name(transitive_targets_rule_runner: RuleRunner, caplog) -> None:
transitive_targets_rule_runner.write_files({"BUILD": "deprecated_target(name='t')"})
transitive_targets_rule_runner.get_target(Address("", target_name="t"))
assert len(caplog.records) == 1
assert "Instead, use `target`"
assert "Instead, use `target`" in caplog.text


def test_resolve_generated_target(transitive_targets_rule_runner: RuleRunner) -> None:
Expand Down
29 changes: 20 additions & 9 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ def compute_value(cls, raw_value: Optional[int], *, address: Address) -> Optiona
removal_version: ClassVar[str | None] = None
removal_hint: ClassVar[str | None] = None

deprecated_alias: ClassVar[str | None] = None
deprecated_alias_removal_version: ClassVar[str | None] = None

@final
def __init__(self, raw_value: Optional[Any], address: Address) -> None:
self._check_deprecated(raw_value, address)
Expand Down Expand Up @@ -327,38 +330,46 @@ def __init__(
warn_or_error(
self.removal_version,
entity=f"the {repr(self.alias)} target type",
hint=(
f"Using the `{self.alias}` target type for {address}. " f"{self.removal_hint}"
),
hint=f"Using the `{self.alias}` target type for {address}. {self.removal_hint}",
)

self.address = address
self.plugin_fields = self._find_plugin_fields(union_membership or UnionMembership({}))

self.residence_dir = residence_dir if residence_dir is not None else address.spec_path
self.field_values = self._calculate_field_values(unhydrated_values, address)
self.validate()

@final
def _calculate_field_values(
self, unhydrated_values: dict[str, Any], address: Address
) -> FrozenDict[type[Field], Field]:
field_values = {}
aliases_to_field_types = {field_type.alias: field_type for field_type in self.field_types}
valid_aliases = set()
aliases_to_field_types = {}
for field_type in self.field_types:
valid_aliases.add(field_type.alias)
aliases_to_field_types[field_type.alias] = field_type
if field_type.deprecated_alias is not None:
aliases_to_field_types[field_type.deprecated_alias] = field_type
for alias, value in unhydrated_values.items():
if alias not in aliases_to_field_types:
raise InvalidFieldException(
f"Unrecognized field `{alias}={value}` in target {address}. Valid fields for "
f"the target type `{self.alias}`: {sorted(aliases_to_field_types.keys())}.",
f"the target type `{self.alias}`: {sorted(valid_aliases)}.",
)
field_type = aliases_to_field_types[alias]
field_values[field_type] = field_type(value, address)

# For undefined fields, mark the raw value as None.
for field_type in set(self.field_types) - set(field_values.keys()):
field_values[field_type] = field_type(None, address)
self.field_values = FrozenDict(
return FrozenDict(
sorted(
field_values.items(),
key=lambda field_type_to_val_pair: field_type_to_val_pair[0].alias,
)
)

self.validate()

@final
@property
def field_types(self) -> Tuple[Type[Field], ...]:
Expand Down