Modernizing objects and context_aware_object_factories (including so it shows up in docsite)

[Eric-Arellano]

We've leaned into objects by introducing parametrize. Compare this to #12410, for example, trying to get rid of the setup_py object. They're here to stay.

Issues

The major issue is that objects and CAOFs aren't integrated into ./pants help and our generated docs.

(I also think it's a downside that the symbol pollutes the entire BUILD namespace, whereas a field is scoped to its owning target and allows reuse. But sometimes this is justified.)

--

Note that generally both objects and context_aware_object_factories don't stop you from doing unsafe things like open(), just like the Rules API can't stop you.

Proposal: BuildSymbol and BuildSymbolFactory

It's very convenient that objects does not impose structure on you. This lets you do things like:

• Set up a constant, e.g. BuildFileAliases(objects={"CONSTANT": "hello"}}.
• Use a custom __init__ to store data for lazy consumption by rules.

It would be a regression if we imposed structure like requiring you to implement a class with alias: ClassVar[str].

Meanwhile, it's boiler-platey that every context_aware_object_factory has the same setup of an __init__ that takes ParseContext, then having to define __call__. CAOFs do not share the same flexibility as objects, nor can they.

So I propose wrapping objects via BuildSymbol, but then defining a proper superclass for CAOFs with BuildSymbolFactory:

PARAMETRIZE_BUILD_SYMBOL = BuildSymbol(
    "parametrize",
    "help message",
    Parametrize,
)


class CwdBuildSymbol(BuildSymbolFactory):
    alias = "cwd"
    help = ""
    
    def __call__(self) -> str:
        # Note that this is a direct property; we get rid of `ParseContext` indirection.
        return self.rel_path

We'd have a new register.py plugin hook, replacing build_file_aliases:

def build_symbols():
     return [PARAMETRIZE_BUILD_SYMBOL, CwdBuildSymbol]

Then our help code can conveniently iterate over every BuildSymbol & BuildSymbolFactory, like how it does with Target, Field, and Subsystem.

[kaos]

Do we still need the CAOF (BuildSymbolFactory)? What do they offer that a target generator can't?

I love the BuildSymbol part of this sketch. :) 👍🏽

[Eric-Arellano]

Do we still need the CAOF (BuildSymbolFactory)?

I think so because of cwd: objects isn't powerful enough. Then I ponder in #14833 that I think it might actually be helpful to augment our current macro support with making CAOFs first-class & documented as a more powerful version of macros.

--

I'm realizing that I think we really want to integrate the light-weight macros from https://www.pantsbuild.org/docs/macros into ./pants help build-symbols, too. We document it as a first-class mechanism, so it should indeed be first-class. Note that those macros are like objects in that we impose no structure on you, and we need to respect that:

CONSTANT = 1

def python2_sources(**kwargs):
     python_sources(**kwargs)

Also, imports are banned in those macros. So we can't define some wrapping class like BuildSymbol. Unless we lighten our import ban to start allow-listing some?

I was wondering about a magic constant you declare like:

_help_messages = {
   CONSTANT: "blah blah",
   python2_sources: "blah blah",
}

We would need to special case this so that it does not cause the symbol to be registered as accessible in all BUILD files, and particularly so that when you have multiple macros we don't complain that you're defining the same symbol twice. Tricky. And maybe too boiler-platey / obscure?

We could use docstring, but that won't work for constants :/ Maybe that's acceptable? Constants can't be documented and we render <no help>? Keep it simple? I think I vote this.

[kaos]

I haven't looked how these macros are loaded, but can't we populate the env when loading these macros, with say a macro decorator that could be used to hook up to the help system?

[Eric-Arellano]

We could do that I think! I'm not sure how they work with constants? Maybe they set a value like __pants_help?

@macro("Hello world!")
CONSTANT = 1

@macro("Help")
def python2_sources():
     ...

I like that a lot because it's consistent and we don't coerce docstring, which people might want to use for other reasons. (It was a big improvement when we stopped using docstring for targets & subsystems).

[kaos]

Yeah, for constants though, I don't think we should use the same decorator though, as that is not technically a macro at all (as I see it, at least).

Perhaps macro wasn't the best suggestion for the decorator name, I think doc might be better.

Perhaps

@doc("help text describing the constant")
CONSTANT = "value"

# but then again, that could work for methods as well...
@doc("Help text")
def python2_sources():
    ...

The decorator could be smart enough here to see what kind of value is being decorated and treat it accordingly.

[Eric-Arellano]

I like that a lot, thank you! One weird thing is that technically doc would be squatting on any other symbol in BUILD files using the same name, like if you want a doc target type. Probably fine, but we may want to call it something like macro_doc, not sure. Related: it might be confusing that our Unrecognized Symbol error message would include doc, given that we only expect macro authors to use it and not normal BUILD files.

[kaos]

Good point. Trying out how register feels..

@register(doc="help text describing the constant")
CONSTANT = "value"

# but then again, that could work for methods as well...
@register(doc="Help text")
def python2_sources():
    ...

It has the added benefit of being a generic placeholder, where any future features may be added to it.

Also, I don't think we'll want to have such a generic target name for anything else, ever.. ? (thinking that it would be scoped to what it is registering, in such cases.. stuff_register() or register_stuff() etc)

For BUILD file macros, the generic register name fits well, as it is a globally available thing that's being registered for any and all parts of the project.

[kaos]

And, come to think of it. That decorator could be a window into the macro world for plugin authors as well!

Potential idea:

@register(doc="Example using env var support", expand_vars=["FOO", "BAR"])
CONSTANT = "Some value with $FOO env vars and ${BAR} values but leaves $THIS alone."

Where a plugin may have registered the expand_vars registration field, which will then get a chance to process the decorated value.

Possibly a can of worms here, but I think it could be really cool stuff. :)

[Eric-Arellano]

@stuhood why is this in the 2.13 milestone?

[stuhood]

@stuhood why is this in the 2.13 milestone?

Just based on the comments in #15503 ... I thought I recalled a 2.13 reference there, but I suppose not. Feel free to remove.

[cognifloyd]

@Eric-Arellano suggested I add some notes from discussion in slack: https://pantsbuild.slack.com/archives/C046T6T9U/p1655837762591389

One problem I have with macros files is that linting with (at least) flake8 is rather annoying because all of the targets and build symbols cause F821: undefined name. Flake8 has a --builtins commandline arg that pants could inject to tell it about all of the other symbols. And, if pants can do that, then we could possibly do the same thing for BUILD files and not just macros files.

(For now, I'm just ignoring that flake8 error in my macros file, but a nicer solution would be great).

In that discussion @thejcannon and @stuhood talked a bit about the possibility of using imports to make those symbols discoverable to tools like flake8, but that adds even more boilerplate to BUILD files.

Highlighting what @stuhood said:

yea. now that we’ve introduced hierarchy via __defaults__ though, i think that it’s possible that macros could be defined hierarchically in parent BUILD files as well… although whether that allows enough flexibility, i’m not sure.
(because load statements have always seemed too noisy to me, and having only a global preamble isn’t very scalable)

So, here are some ideas about how macros could look in BUILD files:

class my_macro(__macro__):
    def __call__(**kwargs):
        ...

I like a function syntax a bit better than class though.

def __macros__():
    def my_macro(**kwargs):
        ...

And, playing off the annotation ideas in here, let's use dunder names for the global non-target names introduced by pants (like __defaults__):

@__macro__(help="some help")
def my_macro(**kwargs):
    ...

[kaos]

I think it would make a lot of sense, defining macros in BUILD files, and that they are accessible for the current subtree.

I like the decorator syntax, as it doesn't incur any restriction on keeping all your macros within a single __macros__ body, and has a cleaner syntax than the class based alternative, imo.

100% agree with going with dunder names, good idea!

[cognifloyd]

Also, I don't think that you can directly apply decorators to constants. This is invalid syntax:

@register(doc="help text describing the constant")
CONSTANT = "value"

But, since decorators are just functions, you can call the decorator after you define the constant:

CONSTANT = "value"
__register__(CONSTANT, help="help text describing the constant")

[kaos]

I don't think that you can directly apply decorators to constants.

Right, my mistake :P

Perhaps more concisely alternative then (and I think we would need to capture the return value of __register__):

CONSTANT = __register__(doc="help text describing the constant")("value")

[kaos]

Another option is to "abuse" annotations in some way:

CONSTANT: "help text describing the constant" = "value"

[Eric-Arellano]

Another option is to "abuse" annotations in some way:

That would mess up MyPy users

[thejcannon]

Yeah big -1 on abuse of annotations that way. But also very clever, I like the thought-path there

[kaos]

Another option is to "abuse" annotations in some way:

That would mess up MyPy users

Well, the fact you can't use annotations at all in the macros file (like this) makes the mypy case a bit moot; but still I agree that I would prefer other options ;)

(the "in some way" was kind of leaving the door open to adjusting it into something mypy-acceptable but didn't want to invest a lot of effort and time into it right now, but also not forgetting my "thought-path", thanks Josh)

[cognifloyd]

-1 on using annotations here.

But that got me thinking about the range of syntax options available in python. We could make a __macros__ context manager. Then macros (constants or function defs), would only be inheritable if defined when that context manager is in scope. Something like this:

with __macros__:
    CONSTANT = "value"

    def my_macro(**kwargs):
        ...

That does not, however address documenting constants. Though, that is not unique to pants, it is a general shortcoming of python. So, we would still need decorator and/or something else to document macro defs and constants.

[cognifloyd]

Hmm... What if we lean into the context manager even further:

CONSTANT = "value"

with __macros__ as m:
    m.register(CONSTANT, help="bla bla")

    @m.register(help="foo bar")
    def my_macro(**kwargs):
        ...

[cognifloyd]

So, all build symbols can be found via the pants help system now. And objects seem to have a better plugin API now (including help text extraction). What else is left before we can call this issue complete?

Here is a brief summary of the remaining ideas I see above (did I miss any?):

• introduce a plugin API for plugins to provide a "better" alternative to context aware object factories (something that mitigates the
• define macros in BUILD files where a macro is some variant of a context aware object factory

[kaos]

Another option is to "abuse" annotations in some way:

CONSTANT: "help text describing the constant" = "value"

[@Eric-Arellano] That would mess up MyPy users

[@thejcannon] Yeah big -1 on abuse of annotations that way. But also very clever, I like the thought-path there

Just to close the loop here.. less abuse would be to use Annotated then ;)

CONSTANT: Annotated[str, "help text describing the constant"] = "value"

[kaos]

Went ahead and made a PR trying out the Annotated route to registering help text for constant values. Looks pretty neat to me 😁

[thejcannon]

Yeah I support Annotated