rubocop: generate_completions DSL

[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?

---

related to #13536

needs https://github.com/Homebrew/homebrew-core/pull/105707

cc @Rylan12

I have achieved to implement a preliminary RuboCop, but I will appreciate any feedback, since I'm sure there's room for improvement.

The Cop is able to correct

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

but I have been unable to come up with a solution to get and replace with all shells whilst only walking the AST once.

The Cop also warns for the

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

pattern, but has no corrector, as discussed in #13536.

Please feel free to push any commits improving my implementation 😄

[Rylan12]

but I have been unable to come up with a solution to get and replace with all shells whilst only walking the AST once.

Not sure, I guess I haven't tried. I think you could probably write a pattern that checks for multiple instances right after each other. That would probably be pretty easy to implement. You also could probably do something like this (not tested at all):

def audit_formula(_node, _class_node, _parent_class_node, body_node)
  install = find_method_def(body_node, :install)

  completion_nodes_to_fix = []

  correctable_shell_completion_node(install) do |node, shell, base_name, binary, cmd, shell_parameter|
    ...

    # Instead of calling `offending_node` and `problem`:
    completion_nodes_to_fix << [node, replacement]
  end

  completion_nodes_to_fix[0...-1].each do |node, _|
    offending_node(node)
    # The wording here is terrible but is just an example
    problem "Use a single `generate_completions` call to generate completions" do |corrector|
      corrector.replace(@offensive_node.source_range, "")
    end
  end

  # Create the single `generate_completions` call
  offending_node(completion_nodes_to_fix.last.first)
  problem "Use `#{replacement}` instead of `#{@offensive_node.source}`." do |corrector|
    corrector.replace(@offensive_node.source_range, replacement)
  end
end

That's just an example, I haven't thought through the logic very thoroughly so there might be a better way to do it.

If those don't work, you could probably write a separate rubocop that combines all of the individual generate_completions calls into one combined call.

[Rylan12]

Looks good so far! Unit tests for this should make it much easier to test.

Ah yes, I forgot to mention tests in my initial comment. RuboCop tests are, luckily, pretty easy to make. I'd recommend looking at the ShellCommands test since it will be almost identical to what you need here. Happy to give more guidance if needed!

[max-ae]

Thank you for the feedback and helpful example code! I'll update the PR with the proposed changes as soon as I refactored the DSL PR, and will also include Unit Tests when doing so.

[max-ae]

Hello there! I have updated the PR to reflect the changes made in the DSL PR, and also included some other fixes.

I have also added another Cop to combine multiple calls with different shells into one.

The only thing I haven't been able to figure out is how to instruct the corrector to also remove the empty lines left behind, as can be seen in the homebrew/core example PR. I have looked into the source_range argument without success, but maybe @Rylan12 can give me a hint 😄

As soon as that is figured out, I will also add Unit tests.

[max-ae]

Can the tests be moved to their own file in test/rubocops/lines/generate_completions_spec.rb?

Sure, done!

[max-ae]

I have now extended the tests. 😄

Now that https://github.com/Homebrew/homebrew-core/pull/105707 is also merged, this should be good to go if CI passes 🎉

[max-ae]

Yeah, I'm gonna need some help with that CI error... I feel like I am missing a return unless somewhere?

2022-09-05T17:58:39.3127476Z [31mAn error occurred while FormulaAudit/GenerateCompletionsDSL cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:39.3130785Z To see the complete backtrace run rubocop -d.
2022-09-05T17:58:39.3134252Z [31mAn error occurred while FormulaAudit/SingleGenerateCompletionsDSLCall cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:39.3137255Z To see the complete backtrace run rubocop -d.
2022-09-05T17:58:39.6834742Z [31mAn error occurred while FormulaAudit/GenerateCompletionsDSL cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:39.6835363Z To see the complete backtrace run rubocop -d.
2022-09-05T17:58:39.6836495Z [31mAn error occurred while FormulaAudit/SingleGenerateCompletionsDSLCall cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:39.6838375Z To see the complete backtrace run rubocop -d.
2022-09-05T17:58:40.0134391Z [31m
2022-09-05T17:58:40.0135188Z 2 errors occurred:[0m
2022-09-05T17:58:40.0136425Z [31mAn error occurred while FormulaAudit/GenerateCompletionsDSL cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:40.0137372Z [31mAn error occurred while FormulaAudit/SingleGenerateCompletionsDSLCall cop was inspecting /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-portable-ruby/Abstract/portable-formula.rb:72:0.[0m
2022-09-05T17:58:40.0154164Z undefined method `metadata' for nil:NilClass
2022-09-05T17:58:40.0156644Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:79:in `display_error_summary'
2022-09-05T17:58:40.0157372Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:58:in `display_summary'
2022-09-05T17:58:40.0158062Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:27:in `block in execute_runner'
2022-09-05T17:58:40.0158740Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:52:in `with_redirect'
2022-09-05T17:58:40.0159393Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:25:in `execute_runner'
2022-09-05T17:58:40.0160202Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command/execute_runner.rb:17:in `run'
2022-09-05T17:58:40.0160832Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/command.rb:11:in `run'
2022-09-05T17:58:40.0161427Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli/environment.rb:18:in `run'
2022-09-05T17:58:40.0162020Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli.rb:72:in `run_command'
2022-09-05T17:58:40.0162602Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli.rb:79:in `execute_runners'
2022-09-05T17:58:40.0163177Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.6.0/gems/rubocop-1.35.1/lib/rubocop/cli.rb:48:in `run'
2022-09-05T17:58:40.0163588Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/utils/rubocop.rb:12:in `<main>'

I can reproduce the failure if I have a formula containing only an install def, but I don't know why the cop should fail then... We are not calling #metadata anywhere...

E.g.

class Foo < Formula
  def install
    (bash_completion/"foo").write Utils.safe_popen_read(bin/"foo", "completions", "bash")
  end
end

[max-ae]

Also, the cask audit test failures should be unrelated.

[max-ae]

Or: should we simply scope this cop to the core tap? The run for all core formulae was fine.

[max-ae]

Yay, all green now! (except for the unrelated tests) Thanks again!

That should be everything now for the new DSL 🎉

[Rylan12]

@SMillerDev the current CI issues (which are occurring for me locally even on the master branch) seem to caused by or related to https://github.com/Homebrew/brew/pull/13746. Any ideas?

Weirdly, I can see the errors if I run brew tests --only=cask/audit but if I add a line number e.g. brew tests --only=cask/audit:6 (which should just run all the tests) I don't see an error.

[SMillerDev]

I have no idea, it passed for me and in CI.

[carlocab]

Failures likely fixed by #13813.

[max-ae]

Failures likely fixed by #13813.

Curiously, they are still present in the latest CI run 🧐 Do I need to rebase?

[carlocab]

Do I need to rebase?

Yes, probably.

[Rylan12]

Thanks so much for all of your work on this, @max-ae! 🎉