From 1b22e16a08810c5071f21555dde543e340f3f76f Mon Sep 17 00:00:00 2001 From: Kevin Abel Date: Wed, 28 Mar 2018 20:54:39 -0500 Subject: [PATCH] rubocop: Fix audit for dependency order with multiple tags Dependencies that have multiple tags (`[:build, :test]`) get sorted into multiple locations resulting in the cop always reporting an offense regardless of order. --- .../Homebrew/rubocops/dependency_order_cop.rb | 17 +++++++++++------ .../test/rubocops/dependency_order_cop_spec.rb | 12 ++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/rubocops/dependency_order_cop.rb b/Library/Homebrew/rubocops/dependency_order_cop.rb index ef93141a4d..14f0f85957 100644 --- a/Library/Homebrew/rubocops/dependency_order_cop.rb +++ b/Library/Homebrew/rubocops/dependency_order_cop.rb @@ -44,14 +44,19 @@ module RuboCop end # Separate dependencies according to precedence order: - # build-time > run-time > normal > recommended > optional + # build-time > test > normal > recommended > optional def sort_dependencies_by_type(dependency_nodes) + unsorted_deps = dependency_nodes.to_a ordered = [] - ordered.concat(dependency_nodes.select { |dep| buildtime_dependency? dep }.to_a) - ordered.concat(dependency_nodes.select { |dep| test_dependency? dep }.to_a) - ordered.concat(dependency_nodes.reject { |dep| negate_normal_dependency? dep }.to_a) - ordered.concat(dependency_nodes.select { |dep| recommended_dependency? dep }.to_a) - ordered.concat(dependency_nodes.select { |dep| optional_dependency? dep }.to_a) + ordered.concat(unsorted_deps.select { |dep| buildtime_dependency? dep }) + unsorted_deps -= ordered + ordered.concat(unsorted_deps.select { |dep| test_dependency? dep }) + unsorted_deps -= ordered + ordered.concat(unsorted_deps.reject { |dep| negate_normal_dependency? dep }) + unsorted_deps -= ordered + ordered.concat(unsorted_deps.select { |dep| recommended_dependency? dep }) + unsorted_deps -= ordered + ordered.concat(unsorted_deps.select { |dep| optional_dependency? dep }) end # `depends_on :apple if build.with? "foo"` should always be defined diff --git a/Library/Homebrew/test/rubocops/dependency_order_cop_spec.rb b/Library/Homebrew/test/rubocops/dependency_order_cop_spec.rb index 92ca724750..cc5b0460a5 100644 --- a/Library/Homebrew/test/rubocops/dependency_order_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/dependency_order_cop_spec.rb @@ -46,6 +46,18 @@ describe RuboCop::Cop::NewFormulaAudit::DependencyOrder do end RUBY end + + it "correct depends_on order for multiple tags" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + homepage "http://example.com" + url "http://example.com/foo-1.0.tgz" + depends_on "bar" => [:build, :test] + depends_on "foo" => :build + depends_on "apple" + end + RUBY + end end context "autocorrect" do