admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Revert "Allow resource blocks to include on_* blocks or conditionals"

Browse Source 
This reverts commit b4cd90a3cc47bc2f94e4449fa99b37445b878a5f.

This should never have been merged, given its extraction into PR 15062
had a reasonably long discussion and was decided against in
https://github.com/Homebrew/homebrew-core/pull/126705#discussion_r1149545828,
but I didn't realise I hadn't backed it out of PR 15060 before it was
approved and I merged it.
This commit is contained in:
Issy Long 2023-03-29 00:40:46 +01:00
parent 222ef50063
commit 9302090404
No known key found for this signature in database
GPG Key ID: 8247C390DADC67D4
2 changed files with 9 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Library/Homebrew/rubocops/lines.rb
View File

@ -401,7 +401,6 @@ 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|
@ -421,13 +420,11 @@ module RuboCop
audit_arch_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES,
optional_blocks: CAN_HAVE_ON_SYSTEM_BLOCK_NAMES)
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES)
audit_base_os_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES,
optional_blocks: CAN_HAVE_ON_SYSTEM_BLOCK_NAMES)
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES)
audit_macos_version_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,

30
Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb
View File

@ -78,16 +78,11 @@ module RuboCop
end
end
def audit_arch_conditionals(body_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
def audit_arch_conditionals(body_node, allowed_methods: [], allowed_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,
optional_blocks: optional_blocks,
)
next if if_node_is_allowed?(if_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
if_statement_problem(if_node, "if Hardware::CPU.#{arch_option}?", "on_#{arch_option}",
else_method: else_method, else_node: else_node)
@ -98,12 +93,7 @@ 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,
optional_blocks: optional_blocks,
)
next if if_node_is_allowed?(method_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
offending_node(method_node)
problem "Don't use `#{method_node.source}`, use `on_arm` and `on_intel` blocks instead."
@ -111,7 +101,7 @@ module RuboCop
end
end
def audit_base_os_conditionals(body_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
def audit_base_os_conditionals(body_node, allowed_methods: [], allowed_blocks: [])
BASE_OS_OPTIONS.each do |base_os_option|
os_method, else_method = if base_os_option == :macos
[:mac?, :on_linux]
@ -119,12 +109,7 @@ 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,
optional_blocks: optional_blocks,
)
next if if_node_is_allowed?(if_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
if_statement_problem(if_node, "if OS.#{os_method}", "on_#{base_os_option}",
else_method: else_method, else_node: else_node)
@ -185,16 +170,15 @@ module RuboCop
end
end
def if_node_is_allowed?(if_node, allowed_methods: [], allowed_blocks: [], optional_blocks: [])
def if_node_is_allowed?(if_node, allowed_methods: [], allowed_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_and_optional_blocks
allowed_blocks
else
next
end