Skip to content

Commit

Permalink
Deprecate not setting help for Targets, Fields, and Subsystems (pan…
Browse files Browse the repository at this point in the history
…tsbuild#11380)

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Dec 23, 2020
1 parent 0c8458c commit 8fe3a1e
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 28 deletions.
40 changes: 38 additions & 2 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,18 @@ class Field:
than `Any`. The type hint for `raw_value` is used to generate documentation, e.g. for
`./pants help $target_type`.
Set the `help` class property with a description, which will be used in `./pants help`. For the
best rendering, use soft wrapping (e.g. implicit string concatenation) within paragraphs, but
hard wrapping (`\n`) to separate distinct paragraphs and/or lists.
Example:
# NB: Really, this should subclass IntField. We only use Field as an example.
class Timeout(Field):
alias = "timeout"
value: Optional[int]
default = None
help = "A timeout field.\n\nMore information."
@classmethod
def compute_value(cls, raw_value: Optional[int], *, address: Address) -> Optional[int:
Expand All @@ -100,11 +105,14 @@ def compute_value(cls, raw_value: Optional[int], *, address: Address) -> Optiona
return value_or_default
"""

# Subclasses must define this.
# Subclasses must define these.
alias: ClassVar[str]
help: ClassVar[str]

# Subclasses must define at least one of these two.
default: ClassVar[ImmutableValue]
required: ClassVar[bool] = False

# Subclasses may define these.
deprecated_removal_version: ClassVar[Optional[str]] = None
deprecated_removal_hint: ClassVar[Optional[str]] = None
Expand All @@ -114,6 +122,17 @@ def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
self._check_deprecated(raw_value, address)
self.value: Optional[ImmutableValue] = self.compute_value(raw_value, address=address)

if not address.is_file_target and not hasattr(self, "help"):
warn_or_error(
removal_version="2.3.0.dev0",
deprecated_entity_description="not setting `help` on a `Field`",
hint=(
"Please set the class property `help: str` for the field "
f"`{self.__class__}`. In Pants 2.3, Pants will no longer look at the docstring "
"for help messages and it will error if `help` is not defined."
),
)

@classmethod
def compute_value(cls, raw_value: Optional[Any], *, address: Address) -> ImmutableValue:
"""Convert the `raw_value` into `self.value`.
Expand Down Expand Up @@ -252,11 +271,17 @@ def __eq__(self, other: Union[Any, AsyncFieldMixin]) -> bool:

@frozen_after_init
class Target:
"""A Target represents a combination of fields that are valid _together_."""
"""A Target represents a combination of fields that are valid _together_.
Set the `help` class property with a description, which will be used in `./pants help`. For the
best rendering, use soft wrapping (e.g. implicit string concatenation) within paragraphs, but
hard wrapping (`\n`) to separate distinct paragraphs and/or lists.
"""

# Subclasses must define these
alias: ClassVar[str]
core_fields: ClassVar[Tuple[Type[Field], ...]]
help: ClassVar[str]

# Subclasses may define these.
deprecated_removal_version: ClassVar[Optional[str]] = None
Expand Down Expand Up @@ -293,6 +318,17 @@ def __init__(
),
)

if not address.is_file_target and not hasattr(self, "help"):
warn_or_error(
removal_version="2.3.0.dev0",
deprecated_entity_description="not setting `help` on a `Target`",
hint=(
"Please set the class property `help: str` for the target type "
f"`{self.__class__}`. In Pants 2.3, Pants will no longer look at the docstring "
"for help messages and it will error if `help` is not defined."
),
)

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

Expand Down
26 changes: 14 additions & 12 deletions src/python/pants/help/help_info_extracter.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,20 @@ class TargetFieldHelpInfo:

@classmethod
def create(cls, field: Type[Field]) -> TargetFieldHelpInfo:
# NB: It is very common (and encouraged) to subclass Fields to give custom behavior, e.g.
# `PythonSources` subclassing `Sources`. Here, we set `fallback_to_ancestors=True` so that
# we can still generate meaningful documentation for all these custom fields without
# requiring the Field author to rewrite the docstring.
#
# However, if the original plugin author did not define docstring, then this means we
# would typically fall back to the docstring for `Field` or a template like `StringField`.
# This is a an awkward edge of our heuristic and it's not intentional since these core
# `Field` types have documentation oriented to the plugin author and not the end user
# filling in fields in a BUILD file.
description: Optional[str]
if hasattr(field, "help"):
description = field.help # type: ignore[attr-defined]
description = field.help
else:
# NB: It is very common (and encouraged) to subclass Fields to give custom behavior, e.g.
# `PythonSources` subclassing `Sources`. Here, we set `fallback_to_ancestors=True` so that
# we can still generate meaningful documentation for all these custom fields without
# requiring the Field author to rewrite the docstring.
#
# However, if the original plugin author did not define docstring, then this means we
# would typically fall back to the docstring for `Field` or a template like `StringField`.
# This is a an awkward edge of our heuristic and it's not intentional since these core
# `Field` types have documentation oriented to the plugin author and not the end user
# filling in fields in a BUILD file.
description = get_docstring(
field,
flatten=True,
Expand Down Expand Up @@ -231,9 +232,10 @@ class TargetTypeHelpInfo:
def create(
cls, target_type: Type[Target], *, union_membership: UnionMembership
) -> TargetTypeHelpInfo:
description: Optional[str]
summary: Optional[str]
if hasattr(target_type, "help"):
description = target_type.help # type: ignore[attr-defined]
description = target_type.help
summary = first_paragraph(description)
else:
description = get_docstring(target_type)
Expand Down
15 changes: 14 additions & 1 deletion src/python/pants/option/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
from dataclasses import dataclass
from typing import Optional, Type, cast

from pants.base.deprecated import warn_or_error
from pants.option.option_value_container import OptionValueContainer
from pants.util.memo import memoized_property

GLOBAL_SCOPE = ""
GLOBAL_SCOPE_CONFIG_SECTION = "GLOBAL"
Expand Down Expand Up @@ -34,10 +36,21 @@ class ScopeInfo:
removal_version: Optional[str] = None
removal_hint: Optional[str] = None

@property
# TODO: We only memoize this to avoid repeating the deprecation warning. Revert back once the
# deprecation is finished.
@memoized_property
def description(self) -> str:
if hasattr(self.optionable_cls, "help"):
return cast(str, getattr(self.optionable_cls, "help"))
warn_or_error(
removal_version="2.3.0.dev0",
deprecated_entity_description="not setting `help` on a `Subsystem`",
hint=(
"Please set the class property `help: str` for the subsystem "
f"`{self.optionable_cls}`. In Pants 2.3, Pants will no longer look at the "
f"docstring or help messages and it will error if `help` is not defined."
),
)
return cast(str, self._optionable_cls_attr("get_description", lambda: "")())

@property
Expand Down
18 changes: 5 additions & 13 deletions src/python/pants/option/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,26 +76,18 @@ class Subsystem(Optionable):
Subsystems encapsulate the configuration and initialization of things like JVMs,
Python interpreters, SCMs and so on.
Subsystem instances can be global or per-optionable. Global instances are useful for representing
global concepts, such as the SCM used in the workspace. Per-optionable instances allow individual
Optionable objects (notably, tasks) to have their own configuration for things such as artifact
caches.
Each subsystem type has an option scope. The global instance of that subsystem initializes
itself from options in that scope. An optionable-specific instance initializes itself from options
in an appropriate subscope, which defaults back to the global scope.
For example, the global artifact cache options would be in scope `cache`, but the
compile.java task can override those options in scope `cache.compile.java`.
Subsystems may depend on other subsystems.
Set the `help` class property with a description, which will be used in `./pants help`. For the
best rendering, use soft wrapping (e.g. implicit string concatenation) within paragraphs, but
hard wrapping (`\n`) to separate distinct paragraphs and/or lists.
:API: public
"""

scope: str
options: OptionValueContainer

help: ClassVar[str]

# TODO: The full Options object for this pants run for use by `global_instance` and
# `scoped_instance`.
_options: ClassVar[Optional[Options]] = None
Expand Down

0 comments on commit 8fe3a1e

Please sign in to comment.