Merge pull request #4327 from reitermarkus/dependency-order-cop

Support constants in `DependencyOrder` cop.

Library/Homebrew/rubocops/dependency_order_cop.rb

@@ -20,7 +20,7 @@ module RuboCop
         def check_dependency_nodes_order(parent_node)
           return if parent_node.nil?
           dependency_nodes = fetch_depends_on_nodes(parent_node)
-          ordered = dependency_nodes.sort { |a, b| sort_by_dependency_name(a, b) }
+          ordered = dependency_nodes.sort_by { |node| dependency_name(node).downcase }
           ordered = sort_dependencies_by_type(ordered)
           sort_conditional_dependencies!(ordered)
           verify_order_in_source(ordered)
@@ -31,18 +31,6 @@ module RuboCop
           parent_node.each_child_node.select { |x| depends_on_node?(x) }
         end
 
-        def sort_by_dependency_name(a, b)
-          a_name = dependency_name(a)
-          b_name = dependency_name(b)
-          if a_name < b_name
-            -1
-          elsif a_name > b_name
-            1
-          else
-            0
-          end
-        end
-
         # Separate dependencies according to precedence order:
         # build-time > test > normal > recommended > optional
         def sort_dependencies_by_type(dependency_nodes)
@@ -119,8 +107,8 @@ module RuboCop
 
         # Node pattern method to extract `name` in `depends_on :name`
         def_node_search :dependency_name_node, <<~EOS
-          {(send nil? :depends_on {(hash (pair $_ _)) $({str sym} _)})
-           (if _ (send nil? :depends_on {(hash (pair $_ _)) $({str sym} _)}) nil?)}
+          {(send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)})
+           (if _ (send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) nil?)}
         EOS
 
         # Node pattern method to extract `name` in `build.with? :name`

Library/Homebrew/test/rubocops/dependency_order_cop_spec.rb

@@ -28,6 +28,18 @@ describe RuboCop::Cop::NewFormulaAudit::DependencyOrder do
       RUBY
     end
 
+    it "supports requirement constants" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          homepage "http://example.com"
+          url "http://example.com/foo-1.0.tgz"
+          depends_on FooRequirement
+          depends_on "bar"
+          ^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "FooRequirement" (line 4)
+        end
+      RUBY
+    end
+
     it "wrong conditional depends_on order" do
       expect_offense(<<~RUBY)
         class Foo < Formula