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

Fix confusing error for an @console_rule not returning a Goal #8931

Merged
merged 1 commit into from
Jan 9, 2020
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
20 changes: 14 additions & 6 deletions src/python/pants/engine/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,11 @@ def _get_starting_indent(source):


def _make_rule(
return_type: Type, parameter_types: typing.Iterable[Type], cacheable: bool = True,
name: Optional[bool] = None
return_type: Type,
parameter_types: typing.Iterable[Type],
*,
cacheable: bool = True,
name: Optional[str] = None
) -> Callable[[Callable], Callable]:
"""A @decorator that declares that a particular static function may be used as a TaskRule.

Expand All @@ -90,12 +93,17 @@ def _make_rule(
its inputs.
"""

is_goal_cls = issubclass(return_type, Goal)
if is_goal_cls == cacheable:
has_goal_return_type = issubclass(return_type, Goal)
if cacheable and has_goal_return_type:
raise TypeError(
'An `@rule` that produces a `Goal` must be declared with @console_rule in order to signal '
'An `@rule` that returns a `Goal` must be declared with `@console_rule` in order to signal '
'that it is not cacheable.'
)
if not cacheable and not has_goal_return_type:
raise TypeError(
'An `@console_rule` must return a subclass of `engine.goal.Goal`.'
)
is_goal_cls = has_goal_return_type

def wrapper(func):
if not inspect.isfunction(func):
Expand Down Expand Up @@ -213,7 +221,7 @@ def rule_decorator(*args, **kwargs) -> Callable:
func = args[0]

cacheable: bool = kwargs['cacheable']
name = kwargs.get('name')
name: Optional[str] = kwargs.get('name')

signature = inspect.signature(func)
func_id = f'@rule {func.__module__}:{func.__name__}'
Expand Down
21 changes: 21 additions & 0 deletions tests/python/pants_test/engine/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,27 @@ def dry(a: int, b: 42, c: float) -> bool:
return False


class ConsoleRuleValidation(TestBase):

def test_not_properly_marked_console_rule(self) -> None:
with self.assertRaisesWithMessage(
TypeError,
"An `@rule` that returns a `Goal` must be declared with `@console_rule` in order to signal "
"that it is not cacheable."
):
@rule
def normal_rule_trying_to_return_a_goal() -> Example:
return Example(0)

def test_console_rule_not_returning_a_goal(self) -> None:
with self.assertRaisesWithMessage(
TypeError, "An `@console_rule` must return a subclass of `engine.goal.Goal`."
):
@console_rule
def console_rule_returning_a_non_goal() -> int:
return 0


class RuleGraphTest(TestBase):
def test_ruleset_with_missing_product_type(self):
@rule
Expand Down