WIP: Upgrade rule parsing to follow calls

[thejcannon]

This adds a file rule_visitor.py which exposes a function collect_awaitables which is responsible for collecting the awaitables for a rule function.

Here's the gist:

• Just like before we use ast to parse the function for Call nodes, and handle the Get and Effect types
• But now we also attempt to traverse calls to find the awaitables in helper methods.

The traversal is still pretty rudimentary, and includes guard rails. The idea is for the parser to be more permissive, but still not "perfect" or even "smart".

---

This PR also shows some Python tools using helpers in a helper module for exposition. There's probably a discussion to be had at where the line should be drawn between "use a normal async helper" and "use a new rule". These changes are _mostly) for demonstration.

One thing to note, too is that the helpers inputs don't need to be anything fancy, like hashable, which is different from rule inputs.

---

This came about because I want to add the # of files to the LintResults and didn't want to edit everywhere we create a LintResults. If we had a helper, it'd be easy-peasy 🎉

[thejcannon]

This PR won't pass checks because I'm not bothering to get it all type-correct 😉

[kaos]

Why this level of indirection for get_setup?

Rather than run_linter_from_venv(..., get_setup=Get(ToolSetup, SetupRequest, ..)) ?

[thejcannon]

So why is it a callable? I just didn't think of that when I wrote it 😄

.... I mean, uh, I just wanted to demonstrate the parser parses lambdas 👀

[kaos]

Oh, cool. :)

[stuhood]

Hey @thejcannon : this is a really interesting direction: thank you for sketching it.

I think that there is definitely room for more async-via-Gets code which is not itself @rule based. @rules play a few roles:

• memoization
• dependency injection
• pluggability (via @unions)

Introducing direct calls like you've done here still gets you most of the dependency injection (via Gets but not parameters), while dropping the memoization and pluggability.

I think that being able to skip memoization and avoid creating temporary dataclasses will be worthwhile in a bunch of cases (maybe not at public API boundaries?, but certainly internally) for performance and ease of use reasons, and so I'm probably on board with this.

---

I know that it is much more challenging (maybe impossible, without explicit annotations/syntax?), but a thing that would be really, really killer would be the ability to use async class/instance methods of @unions in a similar fashion. That would give us much, much more flexible extensibility in plugin APIs.

As a strawman, a syntax like:

# On a `type[UnionBase]` class
await Summon(UnionBase, union_member_cls).class_method_of_union()
# On a `UnionBase` instance
await Summon(UnionBase, union_member_instance).instance_method_of_union()

Similar to a Get for a union, this would need to be expanded after unions had all been collected (i.e., here)... so at parse time, you'd have to capture the method name, and defer capturing the Gets from method bodies until after you know which unions exist.

EDIT: And I suppose that if we had this syntax, we'd want it to work for concrete types too, not just @unions... since the syntax is mostly just a type-annotation.

[thejcannon]

Hey @stuhood what do you think it would take to bring this from PoC into the repo? More tests obviously. Anything else?

[stuhood]

Hey @stuhood what do you think it would take to bring this from PoC into the repo? More tests obviously. Anything else?

Yea, probably tests, and a pretty clear idea of the guidance that we'll put on https://www.pantsbuild.org/v2.11/docs/rules-api-concepts about when to use static functions vs @rules: i.e.,

1. you don't need memoization
2. it doesn't need to be pluggable
3. it's a private API (IMO: see below)

In particular, IMO: we probably shouldn't use static functions for public APIs (since none of our existing type-level documentation will work for them)... which actually means avoiding using them for the case you've demonstrated here. If setup_tool is supposed to be a public API (...although these APIs are obviously still experimental) for use by Linters/Formatters in plugins, then we should probably expose it via a @rule. Depending how well things go, this may just be a temporary restriction though.

But certainly there are lots of private cases that would be significantly cleaned up by calling a static function. So yea, minimal blockers here.

[thejcannon]

For 3. it might be worth a quick check that the helper function starts with _. That'd somewhat codify the requirement.

[thejcannon]

Another thought I had that might make this more stomachable. We could only collect info from functions decorated rule.helper. It makes helper-izing a function explicit, and allows us a convenient marker for collection (if we did want to make the helpers public)

[Eric-Arellano]

We could only collect info from functions decorated rule.helper.

I like that a lot. At least while we experiment with this API. We can always remove the helper if we decide it's not necessary. Easier to take away than to add later.

[stuhood]

Just encountered a case (static function which wanted to await something) where this would have been nice to have!

[thejcannon]

Yeah, I frequently have this thought nowadays. (Especially for the PEX code). Soon I'll pop this off my backlog

[thejcannon]

Successor: #15025