From b4cd90a3cc47bc2f94e4449fa99b37445b878a5f Mon Sep 17 00:00:00 2001
From: Issy Long <me@issyl0.co.uk>
Date: Sun, 26 Mar 2023 18:25:33 +0100
Subject: [PATCH] Allow `resource` blocks to include `on_*` blocks or
 conditionals

- https://github.com/Homebrew/homebrew-core/pull/126705/commits/d2a58a7853e0581ceb80ab38e3244d1969fd2907 was deemed "unwiedly", but it passes the RuboCop.
- https://github.com/Homebrew/homebrew-core/pull/126705#discussion_r1148558613 is more wieldy, but needed RuboCop tweaks.
---
 Library/Homebrew/rubocops/lines.rb            |  7 +++--
 .../shared/on_system_conditionals_helper.rb   | 30 ++++++++++++++-----
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb
index 10aa5a4ab8..2f86c8bcf5 100644
--- a/Library/Homebrew/rubocops/lines.rb
+++ b/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,
diff --git a/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb b/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb
index 432b42ed5e..97f2f68557 100644
--- a/Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb
+++ b/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