Merge pull request #13636 from FnControlHomebrew/depends

Check dependency order in on_system methods

Library/Homebrew/rubocops/components_order.rb

@@ -14,12 +14,6 @@ module FormulaAudit
       class ComponentsOrder < FormulaCop
         extend AutoCorrector
 
-        def on_system_methods
-          @on_system_methods ||= [:intel, :arm, :macos, :linux, :system, *MacOSVersions::SYMBOLS.keys].map do |m|
-            :"on_#{m}"
-          end
-        end
-
         def audit_formula(_node, _class_node, _parent_class_node, body_node)
           @present_components, @offensive_nodes = check_order(FORMULA_COMPONENT_PRECEDENCE_LIST, body_node)
 

Library/Homebrew/rubocops/dependency_order.rb

@@ -16,7 +16,7 @@ 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)
-          [:head, :stable].each do |block_name|
+          ([:head, :stable] + on_system_methods).each do |block_name|
             block = find_block(body_node, block_name)
             next unless block
 

Library/Homebrew/rubocops/extend/formula.rb

@@ -198,6 +198,12 @@ def file_path_allowed?
 
         @file_path !~ Regexp.union(paths_to_exclude)
       end
+
+      def on_system_methods
+        @on_system_methods ||= [:intel, :arm, :macos, :linux, :system, *MacOSVersions::SYMBOLS.keys].map do |m|
+          :"on_#{m}"
+        end
+      end
     end
   end
 end

Library/Homebrew/test/rubocops/dependency_order_spec.rb

@@ -114,6 +114,34 @@ class Foo < Formula
         end
       RUBY
     end
+
+    it "reports and corrects wrong conditional order within a system block" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          homepage "https://brew.sh"
+          url "https://brew.sh/foo-1.0.tgz"
+          on_arm 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
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Foo < Formula
+          homepage "https://brew.sh"
+          url "https://brew.sh/foo-1.0.tgz"
+          on_arm do
+            uses_from_macos "bar"
+            uses_from_macos "foo" => :optional
+            uses_from_macos "apple" if build.with? "foo"
+          end
+        end
+      RUBY
+    end
   end
 
   context "when auditing `depends_on`" do
@@ -224,5 +252,33 @@ class Foo < Formula
         end
       RUBY
     end
+
+    it "reports and corrects wrong conditional order within a system block" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          homepage "https://brew.sh"
+          url "https://brew.sh/foo-1.0.tgz"
+          on_linux do
+            depends_on "apple" if build.with? "foo"
+            depends_on "bar"
+            ^^^^^^^^^^^^^^^^ dependency "bar" (line 6) should be put before dependency "apple" (line 5)
+            depends_on "foo" => :optional
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 7) should be put before dependency "apple" (line 5)
+          end
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Foo < Formula
+          homepage "https://brew.sh"
+          url "https://brew.sh/foo-1.0.tgz"
+          on_linux do
+            depends_on "bar"
+            depends_on "foo" => :optional
+            depends_on "apple" if build.with? "foo"
+          end
+        end
+      RUBY
+    end
   end
 end