Rule parsing errors are terrifying.

[jsirois]

The constraints on Get contents in python rules due to the need to parse Gets for type information to set up the static rule graph are a known evil. Given that we've decided to accept that evil as the best one, we should improve the UX when the constraints are violated.

An example of what a user can hit follows. This happended to me today I was confused for a good while:

The change was to add a factory method to the subject type of a Get:

From:

@frozen_after_init
@dataclass(unsafe_hash=True)
class ExportingOwnerSearchRequest:
    search_roots: FrozenOrderedSet[ExportedTarget]
    target: Target

    def __init__(self, target: Target, *, search_roots: Optional[Iterable[ExportedTarget]]) -> None:
        self.search_roots = FrozenOrderedSet(search_roots if search_roots is not None else ())
        self.target = target

To:

@frozen_after_init
@dataclass(unsafe_hash=True)
class ExportingOwnerSearchRequest:
    search_roots: FrozenOrderedSet[ExportedTarget]
    target: Target

    def __init__(self, target: Target, *, search_roots: Optional[Iterable[ExportedTarget]]) -> None:
        self.search_roots = FrozenOrderedSet(search_roots if search_roots is not None else ())
        self.target = target

    @classmethod
    def create(
        cls: Type["ExportingOwnerSearchRequest"], target: Target, *search_roots: ExportedTarget
    ) -> "ExportingOwnerSearchRequest":
        return cls(search_roots=search_roots, target=target)

This allowed a typical Get callsite with just 1 search_root to be written:

owners = await MultiGet(
    Get(
        ExportedTarget,
        ExportingOwnerSearchRequest.create(tgt, dependency_owner.exported_target)
    )
    for tgt in ownable_targets
)

Except that produced:

$ ./pants test src/python/pants/backend/python/rules/run_setup_py_test.py
16:15:17 [INFO] initializing pantsd...
16:15:18 [ERROR] 'Attribute' object has no attribute 'id'
Traceback (most recent call last):
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/bin/daemon_pants_runner.py", line 142, in _run
    scheduler = self._core.prepare_scheduler(options_bootstrapper)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/pantsd/pants_daemon_core.py", line 105, in prepare_scheduler
    self._init_scheduler(options_fingerprint, options_bootstrapper)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/pantsd/pants_daemon_core.py", line 85, in _init_scheduler
    raise e
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/pantsd/pants_daemon_core.py", line 74, in _init_scheduler
    build_config = BuildConfigInitializer.get(options_bootstrapper)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/options_initializer.py", line 39, in get
    cls._cached_build_config = cls(options_bootstrapper).setup()
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/options_initializer.py", line 70, in setup
    return self._load_plugins()
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/options_initializer.py", line 59, in _load_plugins
    return load_backends_and_plugins(
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/extension_loader.py", line 41, in load_backends_and_plugins
    load_build_configuration_from_source(bc_builder, backends)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/extension_loader.py", line 114, in load_build_configuration_from_source
    load_backend(build_configuration, backend_package)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/init/extension_loader.py", line 128, in load_backend
    module = importlib.import_module(backend_module)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/build-support/virtualenvs/Linux/pants_dev_deps.py38.venv/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/backend/python/register.py", line 14, in <module>
    from pants.backend.python.rules import (
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/backend/python/rules/run_setup_py.py", line 582, in <module>
    async def get_requirements(
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 389, in wrapper
    return rule_decorator(*args, **kwargs)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 362, in rule_decorator
    return _make_rule(
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 242, in wrapper
    rule_visitor.visit(rule_func_node)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/usr/lib64/python3.8/ast.py", line 371, in generic_visit
    self.visit(item)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/usr/lib64/python3.8/ast.py", line 373, in generic_visit
    self.visit(value)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/usr/lib64/python3.8/ast.py", line 373, in generic_visit
    self.visit(value)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 149, in visit_Call
    self.generic_visit(call_node)
  File "/usr/lib64/python3.8/ast.py", line 371, in generic_visit
    self.visit(item)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/usr/lib64/python3.8/ast.py", line 373, in generic_visit
    self.visit(value)
  File "/usr/lib64/python3.8/ast.py", line 363, in visit
    return visitor(node)
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 147, in visit_Call
    self._gets.append(self._extract_constraints(get_descriptor))
  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/engine/rules.py", line 127, in _extract_constraints
    constructor_type_id = subject_constructor.func.id  # type: ignore[attr-defined]
AttributeError: 'Attribute' object has no attribute 'id'

The fix was to use the 3 argument form of Get explicity listing the subject type:

owners = await MultiGet(
    Get(
        ExportedTarget,
        ExportingOwnerSearchRequest,
        ExportingOwnerSearchRequest.create(tgt, dependency_owner.exported_target)
    )
    for tgt in ownable_targets
)

That fix was effectively impossible to guess though since the backtrace does not mention the file producing the parse error. It could go further and note the list of constraints on Get contents since that will be the confusing thing. The user will just have used standard python refactoring and have no clue why that was illegal.

[stuhood]

Mm, agreed. The offending file is mentioned, but it's hidden near the middle of the stack:

  File "/home/jsirois/dev/pantsbuild/jsirois-pants/src/python/pants/backend/python/rules/run_setup_py.py", line 582, in <module>
    async def get_requirements(

[Eric-Arellano]

Fixed by #11081.