Skip to content

Commit

Permalink
Fix confusing error for an @console_rule not returning a Goal
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric-Arellano committed Jan 9, 2020
1 parent 56a8892 commit ae46190
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
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

0 comments on commit ae46190

Please sign in to comment.