Skip to content

Commit

Permalink
Prep for having build file symbol info. (pantsbuild#18260)
Browse files Browse the repository at this point in the history
Work towards pantsbuild#18117 
Most notably
pantsbuild#18117 (comment)

Also improved error feedback in case of misuse of symbols in prelude
files.
  • Loading branch information
kaos authored Feb 23, 2023
1 parent 5f3f251 commit ce401b7
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/engine/internals/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ async def evaluate_preludes(
for file_content in prelude_digest_contents:
try:
file_content_str = file_content.content.decode()
content = compile(file_content_str, file_content.path, "exec")
content = compile(file_content_str, file_content.path, "exec", dont_inherit=True)
exec(content, globals, locals)
except Exception as e:
raise Exception(f"Error parsing prelude file {file_content.path}: {e}")
Expand All @@ -107,7 +107,7 @@ async def evaluate_preludes(
# Ensure preludes can reference each other by populating the shared globals object with references
# to the other symbols
globals.update(locals)
return BuildFilePreludeSymbols(FrozenDict(locals))
return BuildFilePreludeSymbols.from_namespace(locals)


@rule
Expand Down
70 changes: 68 additions & 2 deletions src/python/pants/engine/internals/build_files_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from pants.engine.internals.dep_rules import MaybeBuildFileDependencyRulesImplementation
from pants.engine.internals.mapper import AddressFamily
from pants.engine.internals.parametrize import Parametrize
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser
from pants.engine.internals.parser import BuildFilePreludeSymbols, BuildFileSymbolInfo, Parser
from pants.engine.internals.scheduler import ExecutionError
from pants.engine.internals.session import SessionValues
from pants.engine.internals.synthetic_targets import (
Expand Down Expand Up @@ -135,7 +135,17 @@ def run_prelude_parsing_rule(prelude_content: str) -> BuildFilePreludeSymbols:


def test_prelude_parsing_good() -> None:
result = run_prelude_parsing_rule("def foo(): return 1")
prelude_content = dedent(
"""
def bar():
__defaults__(all=dict(ok=123))
return build_file_dir()
def foo():
return 1
"""
)
result = run_prelude_parsing_rule(prelude_content)
assert result.symbols["foo"]() == 1


Expand All @@ -161,6 +171,45 @@ def make_target():
run_prelude_parsing_rule(prelude_content)


def test_prelude_check_filepath() -> None:
prelude_content = dedent(
"""
build_file_dir()
"""
)
with pytest.raises(
Exception,
match="The BUILD file `build_file_dir` may only be used in BUILD files\\. If used",
):
run_prelude_parsing_rule(prelude_content)


def test_prelude_check_defaults() -> None:
prelude_content = dedent(
"""
__defaults__(all=dict(bad=123))
"""
)
with pytest.raises(
Exception,
match="The BUILD file `__defaults__` may only be used in BUILD files\\. If used",
):
run_prelude_parsing_rule(prelude_content)


def test_prelude_check_env() -> None:
prelude_content = dedent(
"""
env("nope")
"""
)
with pytest.raises(
Exception,
match="The BUILD file `env` may only be used in BUILD files\\.$",
):
run_prelude_parsing_rule(prelude_content)


def test_prelude_exceptions() -> None:
prelude_content = dedent(
"""\
Expand Down Expand Up @@ -188,6 +237,23 @@ def make_a_target():
result.symbols["make_a_target"]()


def test_prelude_docstrings() -> None:
macro_docstring = "This is the doc-string for `macro_func`."
prelude_content = dedent(
f"""
def macro_func(arg: int) -> str:
'''{macro_docstring}'''
pass
"""
)
result = run_prelude_parsing_rule(prelude_content)
info = result.info["macro_func"]
assert BuildFileSymbolInfo("macro_func", result.symbols["macro_func"]) == info
assert macro_docstring == info.help
assert "(arg: int) -> str" == info.signature
assert {"macro_func"} == set(result.info)


class ResolveField(StringField):
alias = "resolve"

Expand Down
89 changes: 70 additions & 19 deletions src/python/pants/engine/internals/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

from __future__ import annotations

import inspect
import re
import threading
import tokenize
from dataclasses import dataclass
from difflib import get_close_matches
from io import StringIO
from pathlib import PurePath
from typing import Any, Callable
from typing import Any, Callable, Mapping, TypeVar

from pants.base.deprecated import warn_or_error
from pants.base.exceptions import MappingError
Expand All @@ -24,12 +25,43 @@
from pants.engine.unions import UnionMembership
from pants.util.docutil import doc_url
from pants.util.frozendict import FrozenDict
from pants.util.memo import memoized_property
from pants.util.strutil import softwrap

T = TypeVar("T")


@dataclass(frozen=True)
class BuildFilePreludeSymbols:
symbols: FrozenDict[str, Any]
info: FrozenDict[str, BuildFileSymbolInfo]

@memoized_property
def symbols(self) -> FrozenDict[str, Any]:
return FrozenDict({name: symbol.value for name, symbol in self.info.items()})

@classmethod
def from_namespace(cls, ns: Mapping[str, Any]) -> BuildFilePreludeSymbols:
info = {}
for name, symb in ns.items():
info[name] = BuildFileSymbolInfo(name, symb)
return cls(info=FrozenDict(info))


@dataclass(frozen=True)
class BuildFileSymbolInfo:
name: str
value: Any

@property
def help(self) -> str | None:
return inspect.getdoc(self.value) if hasattr(self.value, "__name__") else None

@property
def signature(self) -> str | None:
if not callable(self.value):
return None
else:
return str(inspect.signature(self.value))


class ParseError(Exception):
Expand All @@ -48,6 +80,7 @@ def __init__(self):
self._filepath: str | None = None
self._target_adaptors: list[TargetAdaptor] = []
self._is_bootstrap: bool | None = None
self._env_vars: EnvironmentVars | None = None

def reset(
self,
Expand All @@ -56,38 +89,51 @@ def reset(
defaults: BuildFileDefaultsParserState,
dependents_rules: BuildFileDependencyRulesParserState | None,
dependencies_rules: BuildFileDependencyRulesParserState | None,
env_vars: EnvironmentVars,
) -> None:
self._is_bootstrap = is_bootstrap
self._defaults = defaults
self._dependents_rules = dependents_rules
self._dependencies_rules = dependencies_rules
self._env_vars = env_vars
self._filepath = filepath
self._target_adaptors.clear()

def add(self, target_adaptor: TargetAdaptor) -> None:
self._target_adaptors.append(target_adaptor)

def filepath(self) -> str:
if self._filepath is None:
raise AssertionError(
"The BUILD file filepath was accessed before being set. This indicates a "
"programming error in Pants. Please file a bug report at "
"https://github.com/pantsbuild/pants/issues/new."
)
return self._filepath

def parsed_targets(self) -> list[TargetAdaptor]:
return list(self._target_adaptors)

def _prelude_check(self, name: str, value: T | None, closure_supported: bool = True) -> T:
if value is not None:
return value
note = (
(
" If used in a prelude file, it must be within a function that is called from a BUILD"
" file."
)
if closure_supported
else ""
)
raise NameError(
softwrap(
f"""
The BUILD file {name} may only be used in BUILD files.{note}
"""
)
)

def filepath(self) -> str:
return self._prelude_check("`build_file_dir`", self._filepath)

@property
def defaults(self) -> BuildFileDefaultsParserState:
if self._defaults is None:
raise AssertionError(
"The BUILD file __defaults__ was accessed before being set. This indicates a "
"programming error in Pants. Please file a bug report at "
"https://github.com/pantsbuild/pants/issues/new."
)
return self._defaults
return self._prelude_check("`__defaults__`", self._defaults)

@property
def env_vars(self) -> EnvironmentVars:
return self._prelude_check("`env`", self._env_vars, closure_supported=False)

@property
def is_bootstrap(self) -> bool:
Expand All @@ -98,6 +144,9 @@ def is_bootstrap(self) -> bool:
)
return self._is_bootstrap

def get_env(self, name: str, *args, **kwargs) -> Any:
return self.env_vars.get(name, *args, **kwargs)

def set_defaults(
self, *args: SetDefaultsT, ignore_unknown_fields: bool = False, **kwargs
) -> None:
Expand Down Expand Up @@ -221,6 +270,7 @@ def resolve_field_default() -> Any:
symbols: dict[str, Any] = {
**object_aliases.objects,
"build_file_dir": lambda: PurePath(parse_state.filepath()).parent,
"env": parse_state.get_env,
"__defaults__": parse_state.set_defaults,
"__dependents_rules__": parse_state.set_dependents_rules,
"__dependencies_rules__": parse_state.set_dependencies_rules,
Expand Down Expand Up @@ -256,10 +306,10 @@ def parse(
defaults=defaults,
dependents_rules=dependents_rules,
dependencies_rules=dependencies_rules,
env_vars=env_vars,
)

global_symbols: dict[str, Any] = {
"env": env_vars.get,
**self._symbols,
**extra_symbols.symbols,
}
Expand All @@ -285,6 +335,7 @@ def parse(
defaults=defaults,
dependents_rules=dependents_rules,
dependencies_rules=dependencies_rules,
env_vars=env_vars,
)
continue
break
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/internals/parser_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def perform_test(extra_targets: list[str], dym: str) -> None:
object_aliases=build_file_aliases,
ignore_unrecognized_symbols=False,
)
prelude_symbols = BuildFilePreludeSymbols(FrozenDict({"prelude": 0}))
prelude_symbols = BuildFilePreludeSymbols.from_namespace(FrozenDict({"prelude": 0}))
fmt_extra_sym = str(extra_targets)[1:-1] + (", ") if len(extra_targets) != 0 else ""
with pytest.raises(ParseError) as exc:
parser.parse(
Expand Down

0 comments on commit ce401b7

Please sign in to comment.