stardoc
stardoc copied to clipboard
bazelbuild
Documentation Inheritance
As explained in https://github.com/bazelbuild/bazel/issues/7977, there's a common use case for macros which serve as thin wrappers on rule invocations. In such cases, it's inconvenient and maintenance-heavy to manually mirror rule documentation as macro documentation:
def _rule_impl(ctx):
bin_output = ctx.outputs.bin_output
...
return [OutputGroupInfo(bin = depset([bin_output])]
_my_rule = rule(
implementation = _my_rule_impl,
doc = "My rule is the best rule",
attrs = {
"bin_output" : attr.output(doc = "Binary output"),
"foo" : attr.string(doc = "Lorem ipsum dolor sit amet")
}
)
def my_rule(name, **kwargs):
"""My rule is the best rule.
Args:
name: The name of the test rule.
foo: Lorem ipsum dolor sit amet
bin_output: Binary output. `name` + '.exe' by default.
**kwargs: Other attributes to include
"""
_my_rule(name = name,
bin_output = name + ".exe",
**kwargs)
There are several issues here:
- Duplication between the root and argument documentation in the
my_rulemacro and the_my_rulerule - The Args section of the macro does not appropriately match the rule. There is no "foo" parameter of the macro, but it might be part of kwargs.
- We need not document kwargs, as all kwargs should also be documented in
_my_rule
I propose a macro metatag (for Stardoc to recognize) which effectively notes inheritance of documentation. So instead:
def my_rule(name, **kwargs):
"""@inherit(_my_rule)
Args:
bin_output: `name` + '.exe' by default.
"""
_my_rule(name = name,
bin_output = name + ".exe",
**kwargs)
Note this new format will also need to be recognized by buildifier, so that buildifier does not emit warnings even if the Args section of the macro's docstring does not match the arguments of the macro. (Related to https://github.com/bazelbuild/buildtools/issues/649)
This is an extension of bazelbuild/skydoc#182 and a duplicate of bazelbuild/skydoc#115 (though contains more detail than the original FR issue)
cc @ittaiz
In your example, what happens with bin_output? It's an implementation detail, so we don't want to show up in the documentation, right?
My proposal is that it appends the additional docstring to that of the inherited rule. In other words, bin_output for the macro would have docstring:
"Binary output. name + '.exe' by default."
That said, I can see that some users might want to override the documentation of the inherited rule, so perhaps the macro's account of bin_output should also have a @inherit metatag
Your example doesn't show it, but you could have extra args too.
Args:
bin_output: ...
other_arg: ...
Also, @inherit should ideally work with other macros too.
Sure. Lets address these by using a metatag per arg as well.
def my_rule(name, otherarg, **kwargs):
"""@inherit(_my_rule)
Args:
bin_output: @argdoc(_my_rule.bin_output). `name` + '.exe' by default.
otherarg: Some new argument.
"""
_my_rule(name = name,
bin_output = name + ".exe",
**kwargs)
When one specifies @inherit at the top-level docstring, it will inherit everything, including args. When you explicitly specify an arg doc, it overrides by default -- so you need to specify @argdoc to create an appended docstring. You can thus also add new arguments.
+1, sounds like a good suggestion. It’s a bit verbose but maybe that’s what needed for the richness of the features.
Slightly aside, but how are defaults handled for rules vs. macros?
The example above uses the docstring on the macro to record the default value, but since defaults are also possible in macro definitions and rule definitions, wouldn't stardoc already pick those up? i.e. - would it end up generating something else to document the default which would conflict the the docstring the macro is adding?
So taking this:
def _rule_impl(ctx):
...
_my_rule = rule(
implementation = _my_rule_impl,
doc = "My rule is the best rule",
attrs = {
"foo" : attr.string(
default = "default value",
doc = "Lorem ipsum dolor sit amet",
)
}
)
def my_rule(name, foo="different_default", **kwargs):
"""@inherit(_my_rule)
Args:
foo: @argdoc(_my_rule.foo). more text here.
"""
_my_rule(name = name,
**kwargs)
How exactly would foo get documented on my_rule?
I assume you mean, for definition of my_rule:
def my_rule(name, foo="different_default", **kwargs):
"""@inherit(_my_rule)
Args:
foo: @argdoc(_my_rule.foo). more text here.
"""
_my_rule(name = name,
foo = foo,
**kwargs)
(note the foo = foo)
I would hope the implementation for this feature would inherit the documentation via @argdoc, but would override the default value based on the default value of the macro. This would be a good test case :)
So that works for when the macro has a default, but going back to your example:
def my_rule(name, **kwargs):
"""@inherit(_my_rule)
Args:
foo: @argdoc(_my_rule.foo). `name` + ' something' by default.
"""
foo = kwargs.pop('foo', name + ' something')
_my_rule(name = name,
foo = foo,
**kwargs)
Now my docstring has to document the default and the rule will also provide a default.
For what it's worth, in your above example, it wouldn't be appropriate to have foo under the Args section of the function docstring, as it is not an actual requires argument. It would be better to have foo as an explicit function parameter with default. In other words, I believe your example has an equivalent which more easily would relay information for documentation purposes.
However, we'd need to address the following example:
def my_rule(name, **kwargs):
"""@inherit(_my_rule)"""
_my_rule(name = name,
foo = "something",
**kwargs)
Indeed, rule documentation already includes a documented "default value" if available, so we would need to take that into account when allowing for inheritance. We wouldn't want the above example to have foo as a documented attribute of my_rule, because it is overridden and unusable by callers.
In other words, one should be able to @inherit, but specify details such as this where inherited information can be overridden or removed.
Full disclosure, this is not something I likely have cycles for in Q4. Happy to accept contributions, though we'll likely need to evaluate a design before we delve right into implementation (we'd need to make sure these concerns are addressed!)
Hi @UebelAndre, see #89 and in particular Prioritization. We won't be working on new features in the immediate future.
Another upvote for this, with another motivating use case.
Let's say I have a .bzl file defining 2 rules (a_impl, b_impl) and their 2 wrappers (a, b), and I try to generate docs in a single target. No matter how I list the symbols in the rule, the order I get is rules first, then macros, So it comes out as
a_impl
b_impl
a
b
This is an ugly ordering, and only gets worse if I have more rules defined. What I would like is
a
a_impl
b
b_impl
I can work around this by generating each rule and macro separately, and then weaving them together, but yech.
FWIW I hacked up something like this in rules_pkg earlier this year.
I do a stardoc for each .bzl file that needs it, then post-process that to merge into a single file. I can say @wraps(_real_foo) in the macro foo's docstring and then reduce that to a single entry.
So this string ends up like this in the final form.
Instead of macros inheriting rule docs, a better solution might be a new way of declaring macros which uses rule-like attribute lists; the attribute list can then be shared (with whatever modifications one might want to apply) between the underlying rule and the new-style macro wrapper - which would propagate docs for free and would not have the risk of docs going stale (unlike inheritance annotations).
CC @brandjon and @comius who are working on at least 2 different propasals in this area.
