From 670fb96b38e612d382466a435862242ae686bc05 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Sat, 8 Jul 2023 16:34:21 -0400 Subject: [PATCH] Provide proper error message for unrecognized symbols during bootstrap. (#19225) Fixes #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. --- src/python/pants/backend/adhoc/BUILD | 7 ++- .../pants/engine/internals/mapper_test.py | 4 +- src/python/pants/engine/internals/parser.py | 40 +++++++++++++++-- .../pants/engine/internals/parser_test.py | 45 ++++++++++++++++++- src/python/pants/engine/target.py | 3 +- 5 files changed, 90 insertions(+), 9 deletions(-) diff --git a/src/python/pants/backend/adhoc/BUILD b/src/python/pants/backend/adhoc/BUILD index 7808db2f4d8..9fa1858e906 100644 --- a/src/python/pants/backend/adhoc/BUILD +++ b/src/python/pants/backend/adhoc/BUILD @@ -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), + }, +) diff --git a/src/python/pants/engine/internals/mapper_test.py b/src/python/pants/engine/internals/mapper_test.py index 2b6a60d2d42..320928b09eb 100644 --- a/src/python/pants/engine/internals/mapper_test.py +++ b/src/python/pants/engine/internals/mapper_test.py @@ -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 @@ -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", diff --git a/src/python/pants/engine/internals/parser.py b/src/python/pants/engine/internals/parser.py index a12d8d25860..552636dde3c 100644 --- a/src/python/pants/engine/internals/parser.py +++ b/src/python/pants/engine/internals/parser.py @@ -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, @@ -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__ = "" diff --git a/src/python/pants/engine/internals/parser_test.py b/src/python/pants/engine/internals/parser_test.py index 9ad3cbdd5db..77a42358dae 100644 --- a/src/python/pants/engine/internals/parser_test.py +++ b/src/python/pants/engine/internals/parser_test.py @@ -3,6 +3,7 @@ from __future__ import annotations +import re from textwrap import dedent from typing import Any @@ -10,6 +11,7 @@ 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 ( @@ -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 @@ -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 ``." + ) + 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 diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index e00d1816561..ff95ff5c680 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -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, )