From 9457a23430d92ffdfd7c8baf18f548b70ee83fbb Mon Sep 17 00:00:00 2001 From: Issy Long Date: Sat, 15 Apr 2023 22:55:12 +0100 Subject: [PATCH] Extract methods for checking for `on_*` blocks and their contents - The same code to do the same thing was getting a bit repetitive in `Cask/StanzaOrder`, `Cask/StanzaGrouping` and `Cask/NoOverrides` cops. --- .../Homebrew/rubocops/cask/mixin/cask_help.rb | 10 ++++++++++ Library/Homebrew/rubocops/cask/no_overrides.rb | 10 +++------- .../Homebrew/rubocops/cask/stanza_grouping.rb | 17 +++++------------ Library/Homebrew/rubocops/cask/stanza_order.rb | 15 +++++---------- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/Library/Homebrew/rubocops/cask/mixin/cask_help.rb b/Library/Homebrew/rubocops/cask/mixin/cask_help.rb index c4e76607cb..f4ba80c8f1 100644 --- a/Library/Homebrew/rubocops/cask/mixin/cask_help.rb +++ b/Library/Homebrew/rubocops/cask/mixin/cask_help.rb @@ -22,6 +22,16 @@ module RuboCop cask_block = RuboCop::Cask::AST::CaskBlock.new(block_node, comments) on_cask(cask_block) end + + def on_system_methods(cask_stanzas) + cask_stanzas.select { |s| RuboCop::Cask::Constants::ON_SYSTEM_METHODS.include?(s.stanza_name) } + end + + def inner_stanzas(block_node, comments) + block_contents = block_node.child_nodes.select(&:begin_type?) + inner_nodes = block_contents.map(&:child_nodes).flatten.select(&:send_type?) + inner_nodes.map { |n| RuboCop::Cask::AST::Stanza.new(n, comments) } + end end end end diff --git a/Library/Homebrew/rubocops/cask/no_overrides.rb b/Library/Homebrew/rubocops/cask/no_overrides.rb index 484d8696d0..dc704f8575 100644 --- a/Library/Homebrew/rubocops/cask/no_overrides.rb +++ b/Library/Homebrew/rubocops/cask/no_overrides.rb @@ -7,7 +7,6 @@ module RuboCop class NoOverrides < Base include CaskHelp - ON_SYSTEM_METHODS = RuboCop::Cask::Constants::ON_SYSTEM_METHODS # These stanzas can be overridden by `on_*` blocks, so take them into account. # TODO: Update this list if new stanzas are added to `Cask::DSL` that call `set_unique_stanza`. OVERRIDEABLE_METHODS = [ @@ -22,8 +21,7 @@ module RuboCop def on_cask(cask_block) cask_stanzas = cask_block.toplevel_stanzas - # Skip if there are no `on_*` blocks. - return if (on_blocks = cask_stanzas.select { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) }).none? + return unless (on_blocks = on_system_methods(cask_stanzas)).any? stanzas_in_blocks = on_system_stanzas(on_blocks) @@ -40,9 +38,7 @@ module RuboCop def on_system_stanzas(on_system) names = Set.new method_nodes = on_system.map(&:method_node) - method_nodes.each do |node| - next unless node.block_type? - + method_nodes.select(&:block_type?).each do |node| node.child_nodes.each do |child| child.each_node(:send) do |send_node| # Skip (nested) livecheck blocks as its `url` is different to a download `url`. @@ -51,7 +47,7 @@ module RuboCop if send_node.ancestors.drop_while { |a| !a.begin_type? }.any? { |a| a.dstr_type? || a.regexp_type? } next end - next if ON_SYSTEM_METHODS.include?(send_node.method_name) + next if RuboCop::Cask::Constants::ON_SYSTEM_METHODS.include?(send_node.method_name) names.add(send_node.method_name) end diff --git a/Library/Homebrew/rubocops/cask/stanza_grouping.rb b/Library/Homebrew/rubocops/cask/stanza_grouping.rb index 9fa5a5b35c..489c7ba061 100644 --- a/Library/Homebrew/rubocops/cask/stanza_grouping.rb +++ b/Library/Homebrew/rubocops/cask/stanza_grouping.rb @@ -6,7 +6,7 @@ require "forwardable" module RuboCop module Cop module Cask - # This cop checks that a cask's stanzas are grouped correctly. + # This cop checks that a cask's stanzas are grouped correctly, including nested within `on_*` blocks. # @see https://docs.brew.sh/Cask-Cookbook#stanza-order class StanzaGrouping < Base extend Forwardable @@ -14,7 +14,6 @@ module RuboCop include CaskHelp include RangeHelp - ON_SYSTEM_METHODS = RuboCop::Cask::Constants::ON_SYSTEM_METHODS MISSING_LINE_MSG = "stanza groups should be separated by a single empty line" EXTRA_LINE_MSG = "stanzas within the same group should have no lines between them" @@ -24,17 +23,11 @@ module RuboCop cask_stanzas = cask_block.toplevel_stanzas add_offenses(cask_stanzas) - # If present, check grouping of stanzas within `on_*` blocks. - return if (on_blocks = cask_stanzas.select { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) }).none? + return unless (on_blocks = on_system_methods(cask_stanzas)).any? - on_blocks.map(&:method_node).each do |on_block| - next unless on_block.block_type? - - block_contents = on_block.child_nodes.select(&:begin_type?) - inner_nodes = block_contents.map(&:child_nodes).flatten.select(&:send_type?) - inner_stanzas = inner_nodes.map { |node| RuboCop::Cask::AST::Stanza.new(node, processed_source.comments) } - - add_offenses(inner_stanzas) + on_blocks.map(&:method_node).select(&:block_type?).each do |on_block| + stanzas = inner_stanzas(on_block, processed_source.comments) + add_offenses(stanzas) end end diff --git a/Library/Homebrew/rubocops/cask/stanza_order.rb b/Library/Homebrew/rubocops/cask/stanza_order.rb index 6423fc613b..4b5ea9cb7d 100644 --- a/Library/Homebrew/rubocops/cask/stanza_order.rb +++ b/Library/Homebrew/rubocops/cask/stanza_order.rb @@ -6,29 +6,24 @@ require "forwardable" module RuboCop module Cop module Cask - # This cop checks that a cask's stanzas are ordered correctly. + # This cop checks that a cask's stanzas are ordered correctly, including nested within `on_*` blocks. # @see https://docs.brew.sh/Cask-Cookbook#stanza-order class StanzaOrder < Base extend Forwardable extend AutoCorrector include CaskHelp - ON_SYSTEM_METHODS = RuboCop::Cask::Constants::ON_SYSTEM_METHODS MESSAGE = "`%s` stanza out of order" def on_cask(cask_block) @cask_block = cask_block add_offenses(toplevel_stanzas) - return unless (on_blocks = toplevel_stanzas.select { |s| ON_SYSTEM_METHODS.include?(s.stanza_name) }).any? + return unless (on_blocks = on_system_methods(toplevel_stanzas)).any? - on_blocks.map(&:method_node).each do |on_block| - next unless on_block.block_type? - - block_contents = on_block.child_nodes.select(&:begin_type?) - inner_nodes = block_contents.map(&:child_nodes).flatten.select(&:send_type?) - inner_stanzas = inner_nodes.map { |node| RuboCop::Cask::AST::Stanza.new(node, processed_source.comments) } - add_offenses(inner_stanzas, inner: true) + on_blocks.map(&:method_node).select(&:block_type?).each do |on_block| + stanzas = inner_stanzas(on_block, processed_source.comments) + add_offenses(stanzas, inner: true) end end