Merge pull request #15060 from issyl0/formula-block-component-order

Library/Homebrew/rubocops/components_order.rb

@@ -27,6 +27,11 @@ module RuboCop
             [{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }],
           ]
 
+          head_blocks = find_blocks(body_node, :head)
+          head_blocks.each do |head_block|
+            check_block_component_order(FORMULA_COMPONENT_PRECEDENCE_LIST, head_block)
+          end
+
           on_system_methods.each do |on_method|
             on_method_blocks = find_blocks(body_node, on_method)
             next if on_method_blocks.empty?
@@ -41,6 +46,8 @@ module RuboCop
 
           resource_blocks = find_blocks(body_node, :resource)
           resource_blocks.each do |resource_block|
+            check_block_component_order(FORMULA_COMPONENT_PRECEDENCE_LIST, resource_block)
+
             on_system_blocks = {}
 
             on_system_methods.each do |on_method|
@@ -111,6 +118,11 @@ module RuboCop
           end
         end
 
+        def check_block_component_order(component_precedence_list, block)
+          @present_components, offensive_node = check_order(component_precedence_list, block.body)
+          component_problem(*offensive_node) if offensive_node
+        end
+
         def check_on_system_block_content(component_precedence_list, on_system_block)
           if on_system_block.body.block_type? && !on_system_methods.include?(on_system_block.body.method_name) # rubocop:disable Style/InverseMethods (false positive)
             offending_node(on_system_block)

Library/Homebrew/rubocops/lines.rb

@@ -401,6 +401,7 @@ module RuboCop
 
         NO_ON_SYSTEM_METHOD_NAMES = [:install, :post_install].freeze
         NO_ON_SYSTEM_BLOCK_NAMES = [:service, :test].freeze
+        CAN_HAVE_ON_SYSTEM_BLOCK_NAMES = [:resource].freeze
 
         def audit_formula(_node, _class_node, _parent_class_node, body_node)
           NO_ON_SYSTEM_METHOD_NAMES.each do |formula_method_name|
@@ -420,11 +421,13 @@ module RuboCop
 
           audit_arch_conditionals(body_node,
                                   allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
-                                  allowed_blocks:  NO_ON_SYSTEM_BLOCK_NAMES)
+                                  allowed_blocks:  NO_ON_SYSTEM_BLOCK_NAMES,
+                                  optional_blocks: CAN_HAVE_ON_SYSTEM_BLOCK_NAMES)
 
           audit_base_os_conditionals(body_node,
                                      allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
-                                     allowed_blocks:  NO_ON_SYSTEM_BLOCK_NAMES)
+                                     allowed_blocks:  NO_ON_SYSTEM_BLOCK_NAMES,
+                                     optional_blocks: CAN_HAVE_ON_SYSTEM_BLOCK_NAMES)
 
           audit_macos_version_conditionals(body_node,
                                            allowed_methods:     NO_ON_SYSTEM_METHOD_NAMES,

Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb

@@ -78,11 +78,16 @@ module RuboCop
         end
       end
 
-      def audit_arch_conditionals(body_node, allowed_methods: [], allowed_blocks: [])
+      def audit_arch_conditionals(body_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
         ARCH_OPTIONS.each do |arch_option|
           else_method = (arch_option == :arm) ? :on_intel : :on_arm
           if_arch_node_search(body_node, arch: :"#{arch_option}?") do |if_node, else_node|
-            next if if_node_is_allowed?(if_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
+            next if if_node_is_allowed?(
+              if_node,
+              allowed_methods: allowed_methods,
+              allowed_blocks:  allowed_blocks,
+              optional_blocks: optional_blocks,
+            )
 
             if_statement_problem(if_node, "if Hardware::CPU.#{arch_option}?", "on_#{arch_option}",
                                  else_method: else_method, else_node: else_node)
@@ -93,7 +98,12 @@ module RuboCop
           hardware_cpu_search(body_node, method: method) do |method_node|
             # These should already be caught by `if_arch_node_search`
             next if method_node.parent.source.start_with? "if #{method_node.source}"
-            next if if_node_is_allowed?(method_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
+            next if if_node_is_allowed?(
+              method_node,
+              allowed_methods: allowed_methods,
+              allowed_blocks:  allowed_blocks,
+              optional_blocks: optional_blocks,
+            )
 
             offending_node(method_node)
             problem "Don't use `#{method_node.source}`, use `on_arm` and `on_intel` blocks instead."
@@ -101,7 +111,7 @@ module RuboCop
         end
       end
 
-      def audit_base_os_conditionals(body_node, allowed_methods: [], allowed_blocks: [])
+      def audit_base_os_conditionals(body_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
         BASE_OS_OPTIONS.each do |base_os_option|
           os_method, else_method = if base_os_option == :macos
             [:mac?, :on_linux]
@@ -109,7 +119,12 @@ module RuboCop
             [:linux?, :on_macos]
           end
           if_base_os_node_search(body_node, base_os: os_method) do |if_node, else_node|
-            next if if_node_is_allowed?(if_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
+            next if if_node_is_allowed?(
+              if_node,
+              allowed_methods: allowed_methods,
+              allowed_blocks:  allowed_blocks,
+              optional_blocks: optional_blocks,
+            )
 
             if_statement_problem(if_node, "if OS.#{os_method}", "on_#{base_os_option}",
                                  else_method: else_method, else_node: else_node)
@@ -170,15 +185,16 @@ module RuboCop
         end
       end
 
-      def if_node_is_allowed?(if_node, allowed_methods: [], allowed_blocks: [])
+      def if_node_is_allowed?(if_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
         # TODO: check to see if it's legal
         valid = T.let(false, T::Boolean)
+        allowed_and_optional_blocks = allowed_blocks + optional_blocks
         if_node.each_ancestor do |ancestor|
           valid_method_names = case ancestor.type
           when :def
             allowed_methods
           when :block
-            allowed_blocks
+            allowed_and_optional_blocks
           else
             next
           end

Library/Homebrew/test/rubocops/components_order_spec.rb

@@ -803,7 +803,37 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
       RUBY
     end
 
+    context "when in a head block" do
+      it "reports an offense if stanzas inside `head` blocks are out of order" do
+        expect_offense(<<~RUBY)
+          class Foo < Formula
+            homepage "https://brew.sh"
+
+            head do
+              depends_on "bar"
+              url "https://github.com/foo/foo.git", branch: "main"
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `url` (line 6) should be put before `depends_on` (line 5)
+            end
+          end
+        RUBY
+      end
+    end
+
     context "when in a resource block" do
+      it "reports an offense if stanzas inside `resource` blocks are out of order" do
+        expect_offense(<<~RUBY)
+          class Foo < Formula
+            homepage "https://brew.sh"
+
+            resource do
+              sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
+              url "https://brew.sh/resource1.tar.gz"
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `url` (line 6) should be put before `sha256` (line 5)
+            end
+          end
+        RUBY
+      end
+
       it "reports no offenses for a valid `on_macos` and `on_linux` block" do
         expect_no_offenses(<<~RUBY)
           class Foo < Formula
@@ -1106,16 +1136,15 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
             url "https://brew.sh/foo-1.0.tgz"
 
             resource do
-              on_intel do
-                url "https://brew.sh/resource2.tar.gz"
-                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
-              end
-
               on_arm do
               ^^^^^^^^^ `on_arm` blocks within `resource` blocks must contain at least `url` and `sha256` and at most `url`, `mirror`, `version` and `sha256` (in order).
                 sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
                 url "https://brew.sh/resource2.tar.gz"
               end
+              on_intel do
+                url "https://brew.sh/resource2.tar.gz"
+                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
+              end
             end
           end
         RUBY
@@ -1127,11 +1156,6 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
             url "https://brew.sh/foo-1.0.tgz"
 
             resource do
-              on_intel do
-                url "https://brew.sh/resource2.tar.gz"
-                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
-              end
-
               on_arm do
                 if foo == :bar
                   url "https://brew.sh/resource2.tar.gz"
@@ -1141,6 +1165,10 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
                   sha256 "686372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
                 end
               end
+              on_intel do
+                url "https://brew.sh/resource2.tar.gz"
+                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
+              end
             end
           end
         RUBY
@@ -1152,11 +1180,6 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
             url "https://brew.sh/foo-1.0.tgz"
 
             resource do
-              on_intel do
-                url "https://brew.sh/resource2.tar.gz"
-                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
-              end
-
               on_arm do
               ^^^^^^^^^ `on_arm` blocks within `resource` blocks must contain at least `url` and `sha256` and at most `url`, `mirror`, `version` and `sha256` (in order).
                 if foo == :bar
@@ -1167,6 +1190,10 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
                   url "https://brew.sh/resource1.tar.gz"
                 end
               end
+              on_intel do
+                url "https://brew.sh/resource2.tar.gz"
+                sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
+              end
             end
           end
         RUBY