Formula: add DSL to generate completions

[max-ae]

• [x] Have you followed the guidelines in our Contributing document?
• [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
• [x] Have you added an explanation of what your changes do and why you'd like us to include them?
• [x] Have you written new tests for your changes? Here's an example.
• [x] Have you successfully run brew style with your changes locally?
• [x] Have you successfully run brew typecheck with your changes locally?
• [x] Have you successfully run brew tests with your changes locally?

---

This PR proposes an addition to the formula DSL to generate completions more easily.

The common pattern of

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completion", "bash")
(zsh_completion/"_foo").write Utils.safe_popen_read(bin/"foo", "completion", "zsh")
(fish_completion/"foo.fish").write Utils.safe_popen_read(bin/"foo", "completion", "fish")

in homebrew/core could then be replaced by e.g.

generate_completions(shells: [:bash, :zsh])

Let me know if this is something you are interested in pursuing, then I will look into this further, including writing tests, docs, typecheck, and maybe also an AutoCorrector, although I have not much experience with RuboCop.

[carlocab]

I haven't had the chance to take a close look at the implementation yet, but I think this might make for a nice addition to brew. Thanks for working on this!

[carlocab]

Do you have an idea of how many formulae would use this?

I count 30, with

rg 'completion.*\.write.*completion.*sh' -l | wc -l

[carlocab]

The common pattern of

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completion", "bash")
(zsh_completion/"_foo").write Utils.safe_popen_read(bin/"foo", "completion", "zsh")
(fish_completion/"foo.fish").write Utils.safe_popen_read(bin/"foo", "completion", "fish")

This seems to be the pattern for go-dependent formulae (presumably because they all use the same completions library). There's a slightly different pattern for at least some rust-dependent formulae (again, presumably because they all use the same library):

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completions", "bash")
(zsh_completion/"_foo").write Utils.safe_popen_read(bin/"foo", "completions", "zsh")
(fish_completion/"foo.fish").write Utils.safe_popen_read(bin/"foo", "completions", "fish")

There's actually a bit more variation beyond that too. It would be nice if there was a way to handle most of the more common types generically (without making the actual handling here in brew too complicated).

[max-ae]

Thanks for the feedback so far! I will look into this further.

Do you have an idea of how many formulae would use this?

I count 30, with

rg 'completion.*\.write.*completion.*sh' -l | wc -l

In my quick research, I counted 100+ formulae that could benefit from this. Keep in mind, some formulae also use something like this, so your rg expression does not capture all of them.

output = Utils.safe_popen_read(bin/"foo", "completion", "bash")
(bash_completion/"foo").write output

[max-ae]

There's actually a bit more variation beyond that too. It would be nice if there was a way to handle most of the more common types generically (without making the actual handling here in brew too complicated).

Do you have any suggestions on how to approach this? My reasoning behind my draft implementation was to just go with "completion" as a default, and being able to override it with e.g. "completions" with a parameter, but I am open to other suggestions.

Trying possible commands or assuming them based on the language they are built with seems kind of... not nice in my opinion. What do you think?

[max-ae]

I have added commits that include a typecheck signature as well as method documentation.

If someone could help or give a pointer on how to successfully implement a RuboCop, that would be greatly appreciated, given the complex logic (in my opinion, again: not much experience with RuboCop) needed to autocorrect the various completion generation patterns.

[Rylan12]

If someone could help or give a pointer on how to successfully implement a RuboCop, that would be greatly appreciated, given the complex logic (in my opinion, again: not much experience with RuboCop) needed to autocorrect the various completion generation patterns.

I think the best way to figure out the way RuboCop works is just to look at the existing examples in the rubocops/ directory. I think the SafePopenCommands and OptionDeclarations checks may be a good starting point.

I think in terms of complexity, it looks like there are two main categories. First:

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completions", "bash")

In this case, we should be able to use a def_node_search method (which I'll describe below in more detail). Like a Regex, we can use this to extract all the parts that could change (e.g. foo, completions, bash, etc). and then set up an autocorrector to automatically switch it to the new version.

Another case is:

output = Utils.safe_popen_read(bin/"foo", "completion", "bash")
(bash_completion/"foo").write output

This is trickier to autocorrect since it's harder to track what's going on with output. What I'd recommend in this case is just detecting the (bash_completion/"foo").write part and showing an error without autocorrecting. The user will then need to make the change themselves which I think is fine. However, if it turns out that some formulae can't use the DSL and need to use this style, we may not be able to have a check for this.

---

In terms of methods that are available, take a look at what's in rubocops/shared/helper_functions.rb. This has a lot of functions to help with finding certain methods and stuff like that. However, in this case, I think using def_node_search will be super helpful. Basically, it defines a function that will search a node for all descendent nodes that match a certain pattern. You can use the ruby-parse shell command (you might need to install rubocop first) and the Rubocop Node Pattern docs to figure out what the pattern should be.

---

Note also the rubocop will need to go in a separate PR because all of the homebrew/core instances need to be fixed before we merge the rubocop PR for CI to stay green. If you get stuck, feel free to open another PR and I'll be happy to help.

[max-ae]

Thank you very much @Rylan12 for the extensive writeup! I'll start working on a RuboCop in a separate PR, then.

Following that, I will open an accompanying PR in homebrew/core incorporating the new API into formulae and to get a look at the usage.

[SMillerDev]

This looks cool. Just wanted to note that there are a couple of formula using Utils.safe_popen_read({ "SHELL" => "bash" }, bin/"helm", "completion", "bash") too. So you might want to check if you can make those work here (maybe they don't need the env variables)

[Rylan12]

This looks cool. Just wanted to note that there are a couple of formula using Utils.safe_popen_read({ "SHELL" => "bash" }, bin/"helm", "completion", "bash") too. So you might want to check if you can make those work here (maybe they don't need the env variables)

What if we just always set the SHELL env variable in safe_popen_read to the corresponding shell whenever we're using generate_completions

[chenrui333]

It would be also good to add the options of generating the completions for individual shell (instead of generating for all shells).

[chenrui333]

The common pattern of

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completion", "bash")
(zsh_completion/"_foo").write Utils.safe_popen_read(bin/"foo", "completion", "zsh")
(fish_completion/"foo.fish").write Utils.safe_popen_read(bin/"foo", "completion", "fish")

This seems to be the pattern for go-dependent formulae (presumably because they all use the same completions library). There's a slightly different pattern for at least some rust-dependent formulae (again, presumably because they all use the same library):

(bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completions", "bash")
(zsh_completion/"_foo").write Utils.safe_popen_read(bin/"foo", "completions", "zsh")
(fish_completion/"foo.fish").write Utils.safe_popen_read(bin/"foo", "completions", "fish")

There's actually a bit more variation beyond that too. It would be nice if there was a way to handle most of the more common types generically (without making the actual handling here in brew too complicated).

Furthermore, they all use the same CLI framework (spf13/cobra) which has builtin completions support.

[SMillerDev]

It would be also good to add the options of generating the completions for individual shell (instead of generating for all shells).

Isn't that what the shells argument will allow?

[max-ae]

Thanks everyone for your thorough feedback! I will work on the suggested changes and subsequently update both the rubocop and example homebrew/core PR.

[max-ae]

Hello everyone, I have updated the PR with some additional changes and applied the naming suggestions. Let me know what you think! I have also updated the RuboCop PR and the homebrew/core example PR to reflect the changes made here.

[max-ae]

I have made the requested changes of making executable and subcmd mandatory arguments.

I have also added a test, ~although it fails as-is. Anyone have any idea what I am missing here? 😄~

[max-ae]

Dammit! Now I know again why I wrote it like this the first time :D Done.