bottle: add bottle stanza by traversing AST

Library/Homebrew/dev-cmd/bottle.rb

@@ -536,27 +536,7 @@ module Homebrew
             odie "--keep-old was passed but there was no existing bottle block!" if args.keep_old?
             puts output
             update_or_add = "add"
-            pattern = /(
+            Utils::Bottles.add_bottle_stanza!(s.inreplace_string, output)
-                (\ {2}\#[^\n]*\n)*                                                # comments
-                \ {2}(                                                            # two spaces at the beginning
-                  (url|head)\ ['"][\S\ ]+['"]                                     # url or head with a string
-                  (
-                    ,[\S\ ]*$                                                     # url may have options
-                    (\n^\ {3}[\S\ ]+$)*                                           # options can be in multiple lines
-                  )?|
-                  (homepage|desc|sha256|version|mirror|license)\ ['"][\S\ ]+['"]| # specs with a string
-                  license\ (
-                    [^\[]+?\[[^\]]+?\]|                                           # license may contain a list
-                    [^{]+?{[^}]+?}|                                               # license may contain a hash
-                    :\S+                                                          # license as a symbol
-                  )|
-                  (revision|version_scheme)\ \d+|                                 # revision with a number
-                  (stable|livecheck)\ do(\n+^\ {4}[\S\ ]+$)*\n+^\ {2}end          # components with blocks
-                )\n+                                                              # multiple empty lines
-               )+
-             /mx
-            string = s.sub!(pattern, "\\0#{output}\n")
-            odie "Bottle block addition failed!" unless string
           end
         end
 

Library/Homebrew/rubocops/components_order.rb

@@ -11,50 +11,50 @@ module RuboCop
       # - `component_precedence_list` has component hierarchy in a nested list
       #   where each sub array contains components' details which are at same precedence level
       class ComponentsOrder < FormulaCop
-        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+        COMPONENT_PRECEDENCE_LIST = [
-          component_precedence_list = [
+          [{ name: :include,   type: :method_call }],
-            [{ name: :include,   type: :method_call }],
+          [{ name: :desc,      type: :method_call }],
-            [{ name: :desc,      type: :method_call }],
+          [{ name: :homepage,  type: :method_call }],
-            [{ name: :homepage,  type: :method_call }],
+          [{ name: :url,       type: :method_call }],
-            [{ name: :url,       type: :method_call }],
+          [{ name: :mirror,    type: :method_call }],
-            [{ name: :mirror,    type: :method_call }],
+          [{ name: :version,   type: :method_call }],
-            [{ name: :version,   type: :method_call }],
+          [{ name: :sha256,    type: :method_call }],
-            [{ name: :sha256,    type: :method_call }],
+          [{ name: :license, type: :method_call }],
-            [{ name: :license, type: :method_call }],
+          [{ name: :revision, type: :method_call }],
-            [{ name: :revision, type: :method_call }],
+          [{ name: :version_scheme, type: :method_call }],
-            [{ name: :version_scheme, type: :method_call }],
+          [{ name: :head,      type: :method_call }],
-            [{ name: :head,      type: :method_call }],
+          [{ name: :stable,    type: :block_call }],
-            [{ name: :stable,    type: :block_call }],
+          [{ name: :livecheck, type: :block_call }],
-            [{ name: :livecheck, type: :block_call }],
+          [{ name: :bottle,    type: :block_call }],
-            [{ name: :bottle,    type: :block_call }],
+          [{ name: :pour_bottle?, type: :block_call }],
-            [{ name: :pour_bottle?, type: :block_call }],
+          [{ name: :head,      type: :block_call }],
-            [{ name: :head,      type: :block_call }],
+          [{ name: :bottle,    type: :method_call }],
-            [{ name: :bottle,    type: :method_call }],
+          [{ name: :keg_only,  type: :method_call }],
-            [{ name: :keg_only,  type: :method_call }],
+          [{ name: :option,    type: :method_call }],
-            [{ name: :option,    type: :method_call }],
+          [{ name: :deprecated_option, type: :method_call }],
-            [{ name: :deprecated_option, type: :method_call }],
+          [{ name: :disable!, type: :method_call }],
-            [{ name: :disable!, type: :method_call }],
+          [{ name: :deprecate!, type: :method_call }],
-            [{ name: :deprecate!, type: :method_call }],
+          [{ name: :depends_on, type: :method_call }],
-            [{ name: :depends_on, type: :method_call }],
+          [{ name: :uses_from_macos, type: :method_call }],
-            [{ name: :uses_from_macos, type: :method_call }],
+          [{ name: :on_macos, type: :block_call }],
-            [{ name: :on_macos, type: :block_call }],
+          [{ name: :on_linux, type: :block_call }],
-            [{ name: :on_linux, type: :block_call }],
+          [{ name: :conflicts_with, type: :method_call }],
-            [{ name: :conflicts_with, type: :method_call }],
+          [{ name: :skip_clean, type: :method_call }],
-            [{ name: :skip_clean, type: :method_call }],
+          [{ name: :cxxstdlib_check, type: :method_call }],
-            [{ name: :cxxstdlib_check, type: :method_call }],
+          [{ name: :link_overwrite, type: :method_call }],
-            [{ name: :link_overwrite, type: :method_call }],
+          [{ name: :fails_with, type: :method_call }, { name: :fails_with, type: :block_call }],
-            [{ name: :fails_with, type: :method_call }, { name: :fails_with, type: :block_call }],
+          [{ name: :go_resource, type: :block_call }, { name: :resource, type: :block_call }],
-            [{ name: :go_resource, type: :block_call }, { name: :resource, type: :block_call }],
+          [{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }],
-            [{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }],
+          [{ name: :needs, type: :method_call }],
-            [{ name: :needs, type: :method_call }],
+          [{ name: :install, type: :method_definition }],
-            [{ name: :install, type: :method_definition }],
+          [{ name: :post_install, type: :method_definition }],
-            [{ name: :post_install, type: :method_definition }],
+          [{ name: :caveats, type: :method_definition }],
-            [{ name: :caveats, type: :method_definition }],
+          [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }],
-            [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }],
+          [{ name: :test, type: :block_call }],
-            [{ name: :test, type: :block_call }],
+        ].freeze
-          ]
 
-          @present_components, @offensive_nodes = check_order(component_precedence_list, body_node)
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          @present_components, @offensive_nodes = check_order(COMPONENT_PRECEDENCE_LIST, body_node)
 
           component_problem @offensive_nodes[0], @offensive_nodes[1] if @offensive_nodes
 

Library/Homebrew/rubocops/shared/helper_functions.rb

@@ -1,6 +1,8 @@
 # typed: false
 # frozen_string_literal: true
 
+require "rubocop"
+
 module RuboCop
   module Cop
     # Helper functions for cops.

Library/Homebrew/test/utils/bottles/bottles_spec.rb

@@ -14,4 +14,262 @@ describe Utils::Bottles do
       end
     end
   end
+
+  describe "#add_bottle_stanza!" do
+    let(:bottle_output) do
+      require "active_support/core_ext/string/indent"
+
+      <<~RUBY.chomp.indent(2)
+        bottle do
+          sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+        end
+      RUBY
+    end
+
+    context "when `license` is a string" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license "MIT"
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license "MIT"
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            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 "when `license` is a symbol" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license :cannot_represent
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license :cannot_represent
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            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 "when `license` is multiline" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license all_of: [
+              :public_domain,
+              "MIT",
+              "GPL-3.0-or-later" => { with: "Autoconf-exception-3.0" },
+            ]
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            license all_of: [
+              :public_domain,
+              "MIT",
+              "GPL-3.0-or-later" => { with: "Autoconf-exception-3.0" },
+            ]
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            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 "when `head` is a string" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            head "https://brew.sh/foo.git"
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            head "https://brew.sh/foo.git"
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            end
+          end
+        RUBY
+      end
+
+      it "adds `bottle` after `head`" do
+        described_class.add_bottle_stanza! formula_contents, bottle_output
+        expect(formula_contents).to eq(new_contents)
+      end
+    end
+
+    context "when `head` is a block" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+
+            head do
+              url "https://brew.sh/foo.git"
+            end
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            end
+
+            head do
+              url "https://brew.sh/foo.git"
+            end
+          end
+        RUBY
+      end
+
+      it "adds `bottle` before `head`" do
+        described_class.add_bottle_stanza! formula_contents, bottle_output
+        expect(formula_contents).to eq(new_contents)
+      end
+    end
+
+    context "when there is a comment on the same line" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz" # comment
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz" # comment
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            end
+          end
+        RUBY
+      end
+
+      it "adds `bottle` after the comment" do
+        described_class.add_bottle_stanza! formula_contents, bottle_output
+        expect(formula_contents).to eq(new_contents)
+      end
+    end
+
+    context "when the next line is a comment" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            # comment
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+            # comment
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            end
+          end
+        RUBY
+      end
+
+      it "adds `bottle` after the comment" do
+        described_class.add_bottle_stanza! formula_contents, bottle_output
+        expect(formula_contents).to eq(new_contents)
+      end
+    end
+
+    context "when the next line is blank and the one after it is a comment" do
+      let(:formula_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+
+            # comment
+          end
+        RUBY
+      end
+
+      let(:new_contents) do
+        <<~RUBY.chomp
+          class Foo < Formula
+            url "https://brew.sh/foo-1.0.tar.gz"
+
+            bottle do
+              sha256 "f7b1fc772c79c20fddf621ccc791090bc1085fcef4da6cca03399424c66e06ca" => :sierra
+            end
+
+            # comment
+          end
+        RUBY
+      end
+
+      it "adds `bottle` before the comment" do
+        described_class.add_bottle_stanza! formula_contents, bottle_output
+        expect(formula_contents).to eq(new_contents)
+      end
+    end
+  end
 end

Library/Homebrew/utils/bottles.rb

@@ -74,6 +74,77 @@ module Utils
 
         contents
       end
+
+      def add_bottle_stanza!(formula_contents, bottle_output)
+        require "rubocop-ast"
+
+        ruby_version = Version.new(HOMEBREW_REQUIRED_RUBY_VERSION).major_minor.to_f
+        processed_source = RuboCop::AST::ProcessedSource.new(formula_contents, ruby_version)
+        root_node = processed_source.ast
+
+        class_node = if root_node.class_type?
+          root_node
+        elsif root_node.begin_type?
+          root_node.children.find do |n|
+            n.class_type? && n.parent_class&.const_name == "Formula"
+          end
+        end
+
+        odie "Could not find formula class!" if class_node.nil?
+
+        body_node = class_node.body
+        odie "Formula class is empty!" if body_node.nil?
+
+        node_before_bottle = if body_node.begin_type?
+          body_node.children.compact.reduce do |previous_child, current_child|
+            break previous_child unless component_before_bottle_block? current_child
+
+            current_child
+          end
+        else
+          body_node
+        end
+        node_before_bottle = node_before_bottle.last_argument if node_before_bottle.send_type?
+
+        expr_before_bottle = node_before_bottle.location.expression
+        processed_source.comments.each do |comment|
+          comment_expr = comment.location.expression
+          distance = comment_expr.first_line - expr_before_bottle.first_line
+          case distance
+          when 0
+            if comment_expr.last_line > expr_before_bottle.last_line ||
+               comment_expr.end_pos > expr_before_bottle.end_pos
+              expr_before_bottle = comment_expr
+            end
+          when 1
+            expr_before_bottle = comment_expr
+          end
+        end
+
+        tree_rewriter = Parser::Source::TreeRewriter.new(processed_source.buffer)
+        tree_rewriter.insert_after(expr_before_bottle, "\n\n#{bottle_output.chomp}")
+        formula_contents.replace(tree_rewriter.process)
+      end
+
+      private
+
+      def component_before_bottle_block?(node)
+        require "rubocops/components_order"
+
+        RuboCop::Cop::FormulaAudit::ComponentsOrder::COMPONENT_PRECEDENCE_LIST.each do |components|
+          components.each do |component|
+            return false if component[:name] == :bottle && component[:type] == :block_call
+
+            case component[:type]
+            when :method_call
+              return true if node.send_type? && node.method_name == component[:name]
+            when :block_call
+              return true if node.block_type? && node.method_name == component[:name]
+            end
+          end
+        end
+        false
+      end
     end
 
     # Helper functions for bottles hosted on Bintray.