Skip to content

Commit

Permalink
Provide proper error message for unrecognized symbols during bootstra…
Browse files Browse the repository at this point in the history
…p. (pantsbuild#19225)

Fixes pantsbuild#19156 

Not sure how to provide better error context here though. In the case
presented in the issue, the error could be slightly confusing as it is
reported as unrecognized when in fact it is defined as a macro.
  • Loading branch information
kaos authored Jul 8, 2023
1 parent 11cb1f4 commit 670fb96
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 9 deletions.
7 changes: 6 additions & 1 deletion src/python/pants/backend/adhoc/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"run_system_binary_integration_test.py": dict(timeout=120),
},
)
4 changes: 2 additions & 2 deletions src/python/pants/engine/internals/mapper_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
DuplicateNameError,
SpecsFilter,
)
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser, _unrecognized_symbol_func
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser, _UnrecognizedSymbol
from pants.engine.internals.target_adaptor import TargetAdaptor as _TargetAdaptor
from pants.engine.target import RegisteredTargetTypes, Tags, Target
from pants.engine.unions import UnionMembership
Expand Down Expand Up @@ -95,7 +95,7 @@ def test_address_map_unrecognized_symbol() -> None:
address_map = parse_address_map(build_file, ignore_unrecognized_symbols=True)
assert {
"one": TargetAdaptor(type_alias="thing", name="one"),
"bad": TargetAdaptor(type_alias="thing", name="bad", age=_unrecognized_symbol_func),
"bad": TargetAdaptor(type_alias="thing", name="bad", age=_UnrecognizedSymbol("fake")),
"two": TargetAdaptor(type_alias="thing", name="two"),
"three": TargetAdaptor(
type_alias="thing",
Expand Down
40 changes: 36 additions & 4 deletions src/python/pants/engine/internals/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def parse(
raise
defined_symbols.add(bad_symbol)

global_symbols[bad_symbol] = _unrecognized_symbol_func
global_symbols[bad_symbol] = _UnrecognizedSymbol(bad_symbol)
self._parse_state.reset(
filepath=filepath,
is_bootstrap=is_bootstrap,
Expand Down Expand Up @@ -485,6 +485,38 @@ def _extract_symbol_from_name_error(err: NameError) -> str:
return result.group(1)


def _unrecognized_symbol_func(**kwargs):
"""Allows us to not choke on unrecognized symbols, including when they're called as
functions."""
class _UnrecognizedSymbol:
"""Allows us to not choke on unrecognized symbols, including when they're called as functions.
During bootstrap macros are not loaded and if used in field values to environment targets (which
are parsed during the bootstrap phase) those fields will get instances of this class as field
values.
"""

def __init__(self, name: str) -> None:
self.name = name
self.args: tuple[Any, ...] = ()
self.kwargs: dict[str, Any] = {}

def __call__(self, *args, **kwargs) -> _UnrecognizedSymbol:
self.args = args
self.kwargs = kwargs
return self

def __eq__(self, other) -> bool:
return (
isinstance(other, _UnrecognizedSymbol)
and other.name == self.name
and other.args == self.args
and other.kwargs == self.kwargs
)

def __repr__(self) -> str:
args = ", ".join(map(repr, self.args))
kwargs = ", ".join(f"{k}={v!r}" for k, v in self.kwargs.items())
signature = ", ".join(s for s in (args, kwargs) if s)
return f"{self.name}({signature})"


# Customize the type name presented by the InvalidFieldTypeException.
_UnrecognizedSymbol.__name__ = "<unrecognized symbol>"
45 changes: 44 additions & 1 deletion src/python/pants/engine/internals/parser_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

from __future__ import annotations

import re
from textwrap import dedent
from typing import Any

import pytest

from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.core.target_types import GenericTarget
from pants.engine.addresses import Address
from pants.engine.env_vars import EnvironmentVars
from pants.engine.internals.defaults import BuildFileDefaults, BuildFileDefaultsParserState
from pants.engine.internals.parser import (
Expand All @@ -18,7 +20,7 @@
Parser,
_extract_symbol_from_name_error,
)
from pants.engine.target import RegisteredTargetTypes
from pants.engine.target import InvalidFieldException, RegisteredTargetTypes, StringField
from pants.engine.unions import UnionMembership
from pants.testutil.pytest_util import no_exception
from pants.util.docutil import doc_url
Expand Down Expand Up @@ -128,6 +130,47 @@ def perform_test(extra_targets: list[str], dym: str) -> None:
perform_test(test_targs, dym_many)


def test_unrecognized_symbol_during_bootstrap_issue_19156(
defaults_parser_state: BuildFileDefaultsParserState,
) -> None:
build_file_aliases = BuildFileAliases(
objects={"obj": 0},
context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
)
parser = Parser(
build_root="",
registered_target_types=RegisteredTargetTypes({"tgt": GenericTarget}),
union_membership=UnionMembership({}),
object_aliases=build_file_aliases,
ignore_unrecognized_symbols=True,
)
prelude_symbols = BuildFilePreludeSymbols.create({"prelude": 0}, ())
target_adaptors = parser.parse(
"dir/BUILD",
"tgt(field=fake(42,a=(), b='c'))",
prelude_symbols,
EnvironmentVars({}),
False,
defaults_parser_state,
dependents_rules=None,
dependencies_rules=None,
)

assert len(target_adaptors) == 1
raw_field = target_adaptors[0].kwargs["field"]
assert repr(raw_field) == "fake(42, a=(), b='c')"

class TestField(StringField):
alias = "field"

err = re.escape(
f"The 'field' field in target // must be a string, but was `{raw_field!r}` "
"with type `<unrecognized symbol>`."
)
with pytest.raises(InvalidFieldException, match=err):
TestField(raw_field, Address(""))


@pytest.mark.parametrize("symbol", ["a", "bad", "BAD", "a___b_c", "a231", "áç"])
def test_extract_symbol_from_name_error(symbol: str) -> None:
assert _extract_symbol_from_name_error(NameError(f"name '{symbol}' is not defined")) == symbol
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -1592,9 +1592,10 @@ def __init__(
expected_type: str,
description_of_origin: str | None = None,
) -> None:
raw_type = f"with type `{type(raw_value).__name__}`"
super().__init__(
f"The {repr(field_alias)} field in target {address} must be {expected_type}, but was "
f"`{repr(raw_value)}` with type `{type(raw_value).__name__}`.",
f"`{repr(raw_value)}` {raw_type}.",
description_of_origin=description_of_origin,
)

Expand Down

0 comments on commit 670fb96

Please sign in to comment.