diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index e7791563cd..4768a22605 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/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) 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 diff --git a/Library/Homebrew/test/rubocops/components_order_spec.rb b/Library/Homebrew/test/rubocops/components_order_spec.rb index afead946d0..9761112c49 100644 --- a/Library/Homebrew/test/rubocops/components_order_spec.rb +++ b/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