diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 2204e52351..d62fc83069 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/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 = /( - (\ {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 + Utils::Bottles.add_bottle_stanza!(s.inreplace_string, output) end end diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index e23a2dfa2b..2b442f84fd 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/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 = [ - [{ name: :include, type: :method_call }], - [{ name: :desc, type: :method_call }], - [{ name: :homepage, type: :method_call }], - [{ name: :url, type: :method_call }], - [{ name: :mirror, type: :method_call }], - [{ name: :version, type: :method_call }], - [{ name: :sha256, type: :method_call }], - [{ name: :license, type: :method_call }], - [{ name: :revision, type: :method_call }], - [{ name: :version_scheme, type: :method_call }], - [{ name: :head, type: :method_call }], - [{ name: :stable, type: :block_call }], - [{ name: :livecheck, type: :block_call }], - [{ name: :bottle, type: :block_call }], - [{ name: :pour_bottle?, type: :block_call }], - [{ name: :head, type: :block_call }], - [{ name: :bottle, type: :method_call }], - [{ name: :keg_only, type: :method_call }], - [{ name: :option, type: :method_call }], - [{ name: :deprecated_option, type: :method_call }], - [{ name: :disable!, type: :method_call }], - [{ name: :deprecate!, type: :method_call }], - [{ name: :depends_on, type: :method_call }], - [{ name: :uses_from_macos, type: :method_call }], - [{ name: :on_macos, type: :block_call }], - [{ name: :on_linux, type: :block_call }], - [{ name: :conflicts_with, type: :method_call }], - [{ name: :skip_clean, type: :method_call }], - [{ name: :cxxstdlib_check, type: :method_call }], - [{ name: :link_overwrite, type: :method_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: :patch, type: :method_call }, { name: :patch, type: :block_call }], - [{ name: :needs, type: :method_call }], - [{ name: :install, type: :method_definition }], - [{ name: :post_install, type: :method_definition }], - [{ name: :caveats, type: :method_definition }], - [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }], - [{ name: :test, type: :block_call }], - ] + COMPONENT_PRECEDENCE_LIST = [ + [{ name: :include, type: :method_call }], + [{ name: :desc, type: :method_call }], + [{ name: :homepage, type: :method_call }], + [{ name: :url, type: :method_call }], + [{ name: :mirror, type: :method_call }], + [{ name: :version, type: :method_call }], + [{ name: :sha256, type: :method_call }], + [{ name: :license, type: :method_call }], + [{ name: :revision, type: :method_call }], + [{ name: :version_scheme, type: :method_call }], + [{ name: :head, type: :method_call }], + [{ name: :stable, type: :block_call }], + [{ name: :livecheck, type: :block_call }], + [{ name: :bottle, type: :block_call }], + [{ name: :pour_bottle?, type: :block_call }], + [{ name: :head, type: :block_call }], + [{ name: :bottle, type: :method_call }], + [{ name: :keg_only, type: :method_call }], + [{ name: :option, type: :method_call }], + [{ name: :deprecated_option, type: :method_call }], + [{ name: :disable!, type: :method_call }], + [{ name: :deprecate!, type: :method_call }], + [{ name: :depends_on, type: :method_call }], + [{ name: :uses_from_macos, type: :method_call }], + [{ name: :on_macos, type: :block_call }], + [{ name: :on_linux, type: :block_call }], + [{ name: :conflicts_with, type: :method_call }], + [{ name: :skip_clean, type: :method_call }], + [{ name: :cxxstdlib_check, type: :method_call }], + [{ name: :link_overwrite, type: :method_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: :patch, type: :method_call }, { name: :patch, type: :block_call }], + [{ name: :needs, type: :method_call }], + [{ name: :install, type: :method_definition }], + [{ name: :post_install, type: :method_definition }], + [{ name: :caveats, type: :method_definition }], + [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }], + [{ 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 diff --git a/Library/Homebrew/rubocops/shared/helper_functions.rb b/Library/Homebrew/rubocops/shared/helper_functions.rb index 326f477547..09ba102a7a 100644 --- a/Library/Homebrew/rubocops/shared/helper_functions.rb +++ b/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. diff --git a/Library/Homebrew/test/utils/bottles/bottles_spec.rb b/Library/Homebrew/test/utils/bottles/bottles_spec.rb index c7a0cf9af3..0731b5a4b7 100644 --- a/Library/Homebrew/test/utils/bottles/bottles_spec.rb +++ b/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 diff --git a/Library/Homebrew/utils/bottles.rb b/Library/Homebrew/utils/bottles.rb index 99c727cbd1..dec1fab50e 100644 --- a/Library/Homebrew/utils/bottles.rb +++ b/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.