diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index 08c48224f1..a9d4d577b8 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -32,6 +32,7 @@ module RuboCop [{ name: :option, type: :method_call }], [{ name: :deprecated_option, type: :method_call }], [{ name: :depends_on, type: :method_call }], + [{ name: :uses_from_macos, type: :method_call }], [{ name: :conflicts_with, type: :method_call }], [{ name: :skip_clean, type: :method_call }], [{ name: :cxxstdlib_check, type: :method_call }], diff --git a/Library/Homebrew/rubocops/dependency_order.rb b/Library/Homebrew/rubocops/dependency_order.rb index 1b70f67213..c51a3b7d07 100644 --- a/Library/Homebrew/rubocops/dependency_order.rb +++ b/Library/Homebrew/rubocops/dependency_order.rb @@ -12,29 +12,37 @@ module RuboCop class DependencyOrder < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) check_dependency_nodes_order(body_node) + check_uses_from_macos_nodes_order(body_node) [:devel, :head, :stable].each do |block_name| block = find_block(body_node, block_name) next unless block check_dependency_nodes_order(block.body) + check_uses_from_macos_nodes_order(block.body) end end + def check_uses_from_macos_nodes_order(parent_node) + return if parent_node.nil? + + dependency_nodes = parent_node.each_child_node.select { |x| uses_from_macos_node?(x) } + ensure_dependency_order(dependency_nodes) + end + def check_dependency_nodes_order(parent_node) return if parent_node.nil? - dependency_nodes = fetch_depends_on_nodes(parent_node) - ordered = dependency_nodes.sort_by { |node| dependency_name(node).downcase } + dependency_nodes = parent_node.each_child_node.select { |x| depends_on_node?(x) } + ensure_dependency_order(dependency_nodes) + end + + def ensure_dependency_order(nodes) + ordered = nodes.sort_by { |node| dependency_name(node).downcase } ordered = sort_dependencies_by_type(ordered) sort_conditional_dependencies!(ordered) verify_order_in_source(ordered) end - # Match all `depends_on` nodes among childnodes of given parent node - def fetch_depends_on_nodes(parent_node) - parent_node.each_child_node.select { |x| depends_on_node?(x) } - end - # Separate dependencies according to precedence order: # build-time > test > normal > recommended > optional def sort_dependencies_by_type(dependency_nodes) @@ -101,6 +109,11 @@ module RuboCop (send nil? :depends_on ...)} EOS + def_node_matcher :uses_from_macos_node?, <<~EOS + {(if _ (send nil? :uses_from_macos ...) nil?) + (send nil? :uses_from_macos ...)} + EOS + def_node_search :buildtime_dependency?, "(sym :build)" def_node_search :recommended_dependency?, "(sym :recommended)" @@ -111,9 +124,9 @@ module RuboCop def_node_search :negate_normal_dependency?, "(sym {:build :recommended :test :optional})" - # Node pattern method to extract `name` in `depends_on :name` + # Node pattern method to extract `name` in `depends_on :name` or `uses_from_macos :name` def_node_search :dependency_name_node, <<~EOS - {(send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) + {(send nil? {:depends_on :uses_from_macos} {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) (if _ (send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) nil?)} EOS diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index 3faa594424..1edc7fd499 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/Library/Homebrew/test/rubocops/components_order_spec.rb @@ -6,6 +6,19 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do subject(:cop) { described_class.new } context "When auditing formula components order" do + it "When uses_from_macos precedes depends_on" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + + uses_from_macos "apple" + depends_on "foo" + ^^^^^^^^^^^^^^^^ `depends_on` (line 6) should be put before `uses_from_macos` (line 5) + end + RUBY + end + it "When url precedes homepage" do expect_offense(<<~RUBY) class Foo < Formula diff --git a/Library/Homebrew/test/rubocops/dependency_order_spec.rb b/Library/Homebrew/test/rubocops/dependency_order_spec.rb index c724b8956a..5863686c3f 100644 --- a/Library/Homebrew/test/rubocops/dependency_order_spec.rb +++ b/Library/Homebrew/test/rubocops/dependency_order_spec.rb @@ -5,6 +5,75 @@ require "rubocops/dependency_order" describe RuboCop::Cop::FormulaAudit::DependencyOrder do subject(:cop) { described_class.new } + context "uses_from_macos" do + it "wrong conditional uses_from_macos order" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "apple" if build.with? "foo" + uses_from_macos "foo" => :optional + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 5) should be put before dependency "apple" (line 4) + end + RUBY + end + + it "wrong alphabetical uses_from_macos order" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "foo" + uses_from_macos "bar" + ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "foo" (line 4) + end + RUBY + end + + it "supports requirement constants" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos FooRequirement + uses_from_macos "bar" + ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "FooRequirement" (line 4) + end + RUBY + end + + it "wrong conditional uses_from_macos order with block" do + expect_offense(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + head do + uses_from_macos "apple" if build.with? "foo" + uses_from_macos "bar" + ^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 6) should be put before dependency "apple" (line 5) + uses_from_macos "foo" => :optional + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 7) should be put before dependency "apple" (line 5) + end + uses_from_macos "apple" if build.with? "foo" + uses_from_macos "foo" => :optional + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 10) should be put before dependency "apple" (line 9) + end + RUBY + end + + it "correct uses_from_macos order for multiple tags" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "bar" => [:build, :test] + uses_from_macos "foo" => :build + uses_from_macos "apple" + end + RUBY + end + end + context "depends_on" do it "wrong conditional depends_on order" do expect_offense(<<~RUBY) @@ -75,6 +144,29 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do end context "autocorrect" do + it "wrong conditional uses_from_macos order" do + source = <<~RUBY + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "apple" if build.with? "foo" + uses_from_macos "foo" => :optional + end + RUBY + + correct_source = <<~RUBY + class Foo < Formula + homepage "https://brew.sh" + url "https://brew.sh/foo-1.0.tgz" + uses_from_macos "foo" => :optional + uses_from_macos "apple" if build.with? "foo" + end + RUBY + + corrected_source = autocorrect_source(source) + expect(corrected_source).to eq(correct_source) + end + it "wrong conditional depends_on order" do source = <<~RUBY class Foo < Formula