bottle: add bottle stanza by traversing AST

[SeekingMeaning]

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

---

This should improve brew bottle and make it 💥 less often by traversing the abstract syntax tree of a new formula and not using regex

Related: #9081 (comment)

[reitermarkus]

Very nice.

Would be good to also have some tests with examples where the non-AST version broke before. Also, maybe move this into bottles.rb.

[MikeMcQuaid]

I ❤️ this a LOT, thanks so much @SeekingMeaning! bump-revision would be another good candidate after this is merged. Agreed with @reitermarkus that some very basic tests and extracting it into another file would be great.

[SeekingMeaning]

I'm not sure if this is too long for testing one example:

  context "when `license` is simple" do
    let(:formula_contents) do
      <<~RUBY.chomp
        class Foobar < Formula
          desc "Foobar"
          license "GPL-3.0"
        end
      RUBY
    end

    let(:new_contents) do
      <<~RUBY.chomp
        class Foobar < Formula
          desc "Foobar"
          license "GPL-3.0"

          bottle do
            sha256 "123" => :cat
          end
        end
      RUBY
    end

    it "adds `bottle` after `license`" do
      described_class.add_bottle_stanza! formula_contents, bottle_output
      expect(formula_contents).to eq(new_contents)
    end
  end

Context:

describe "#add_bottle_stanza!" do
  let(:bottle_output) do
    require "active_support/core_ext/string/indent"

    <<~RUBY.chomp.indent(2)
      bottle do
        sha256 "123" => :cat
      end
    RUBY
  end

  # above code block example goes here

  # more examples
end

[MikeMcQuaid]

@SeekingMeaning Doesn't seem too long to me!

[MikeMcQuaid]

This is looking great @SeekingMeaning! A few nits but I'm 👍🏻 to merge this whenever.

[SeekingMeaning]

As COMPONENTS_BEFORE_BOTTLE comes directly from components_order.rb, is it okay to have this "duplicate" list in bottles.rb, or should I extract out component_precedence_list into a static constant? And if so, where would be a good place for this constant?

[MikeMcQuaid]

As COMPONENTS_BEFORE_BOTTLE comes directly from components_order.rb, is it okay to have this "duplicate" list in bottles.rb, or should I extract out component_precedence_list into a static constant? And if so, where would be a good place for this constant?

Maybe something like Library/Homebrew/tap_constants.rb: Library/Homebrew/bottle_constants.rb?

[SeekingMeaning]

No, I meant this: https://github.com/Homebrew/brew/blob/master/Library/Homebrew/rubocops/components_order.rb#L20-L58

[MikeMcQuaid]

@SeekingMeaning Yeh, I'm suggesting it might be good to extract a constant from both places. Alternatively, just require the rubocop module in the bottle code (can't do the other way around as RuboCop needs to be independent of the Homebrew code).

[BrewTestBot]

Review period ended.

[SeekingMeaning]

Linux gives this error:

Error: cannot load such file -- rubocop-ast

[iMichka]

Looks similar to: Homebrew/discussions#265

[MikeMcQuaid]

👍🏻 to merge once CI is 💚.

[MikeMcQuaid]

Yup, that suggested change fixed CI 🎉. Not merging yet because this has the potential to cause homebrew-core issues so I want to wait until @SeekingMeaning is around to respond quickly to any if needed. Nice work here 👏🏻

[iMichka]

Same here, really cool work. I'll let @SeekingMeaning merge and monitor stuff in case there is any issue.

[SeekingMeaning]

Thanks guys! This should only affect new formulae added to homebrew-core but I'll try to keep an eye on everything just in case.