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

Refactor on_system rubocops for use in casks

Browse Source
This commit is contained in:
Rylan Polster 2022-08-14 15:39:20 -04:00
parent 00fd810529
commit af99dfba00
No known key found for this signature in database
GPG Key ID: 46A744940CFF4D64
5 changed files with 403 additions and 193 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
Library/Homebrew/rubocops/cask/on_system_conditionals.rb Normal file
View File

@ -0,0 +1,52 @@
# typed: true
# frozen_string_literal: true
require "forwardable"
require "rubocops/shared/on_system_conditionals_helper"
module RuboCop
module Cop
module Cask
# This cop makes sure that OS conditionals are consistent.
#
# @example
# # bad
# cask 'foo' do
# if MacOS.version == :high_sierra
# sha256 "..."
# end
# end
#
# # good
# cask 'foo' do
# on_high_sierra do
# sha256 "..."
# end
# end
class OnSystemConditionals < Base
extend Forwardable
extend AutoCorrector
include OnSystemConditionalsHelper
include CaskHelp
FLIGHT_STANZA_NAMES = [:preflight, :postflight, :uninstall_preflight, :uninstall_postflight].freeze
def on_cask(cask_block)
@cask_block = cask_block
toplevel_stanzas.each do |stanza|
next unless FLIGHT_STANZA_NAMES.include? stanza.stanza_name
audit_on_system_blocks(stanza.stanza_node, stanza.stanza_name)
end
end
private
attr_reader :cask_block
def_delegators :cask_block, :toplevel_stanzas, :cask_body
end
end
end
end

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

@ -3,6 +3,7 @@
require "macos_versions"
require "rubocops/extend/formula"
require "rubocops/shared/on_system_conditionals_helper"
module RuboCop
module Cop
@ -377,104 +378,20 @@ module RuboCop
#
# @api private
class OnSystemConditionals < FormulaCop
include OnSystemConditionalsHelper
extend AutoCorrector
NO_ON_SYSTEM_METHOD_NAMES = [:install, :post_install].freeze
NO_ON_SYSTEM_BLOCK_NAMES = [:service, :test].freeze
ON_ARCH_OPTIONS = [:intel, :arm].freeze
ON_BASE_OS_OPTIONS = [:macos, :linux].freeze
ON_MACOS_VERSION_OPTIONS = MacOSVersions::SYMBOLS.keys.freeze
ALL_SYSTEM_OPTIONS = [*ON_ARCH_OPTIONS, *ON_BASE_OS_OPTIONS, *ON_MACOS_VERSION_OPTIONS, :system].freeze
MACOS_VERSION_CONDITIONALS = {
"==" => nil,
"<=" => :or_older,
">=" => :or_newer,
}.freeze
ON_SYSTEM_CONDITIONALS = [:<, :<=].freeze
def on_system_method_info(on_system_option)
info = {}
info[:method] = :"on_#{on_system_option}"
info[:if_module], info[:if_method] = if ON_ARCH_OPTIONS.include?(on_system_option)
["Hardware::CPU", :"#{on_system_option}?"]
elsif ON_BASE_OS_OPTIONS.include?(on_system_option)
["OS", (on_system_option == :macos) ? :mac? : :linux?]
else
["MacOS", :version]
end
info[:on_system_string] = "on_#{on_system_option}"
info[:if_string] = if on_system_option == :system
"if OS.linux? || MacOS.version"
else
"if #{info[:if_module]}.#{info[:if_method]}"
end
info
end
def audit_formula(_node, _class_node, _parent_class_node, body_node)
top_level_nodes_to_check = []
NO_ON_SYSTEM_METHOD_NAMES.each do |formula_method_name|
method_node = find_method_def(body_node, formula_method_name)
top_level_nodes_to_check << [formula_method_name, method_node] if method_node
audit_on_system_blocks(method_node, formula_method_name) if method_node
end
NO_ON_SYSTEM_BLOCK_NAMES.each do |formula_block_name|
block_node = find_block(body_node, formula_block_name)
top_level_nodes_to_check << [formula_block_name, block_node] if block_node
end
ALL_SYSTEM_OPTIONS.each do |on_system_option|
method_info = on_system_method_info(on_system_option)
top_level_nodes_to_check.each do |top_level_name, top_level_node|
top_level_node_string = if top_level_node.def_type?
"def #{top_level_name}"
else
"#{top_level_name} do"
end
find_every_method_call_by_name(top_level_node, method_info[:method]).each do |method|
if ON_MACOS_VERSION_OPTIONS.include?(on_system_option)
on_macos_version_method_call(method, on_method: method_info[:method]) do |on_method_parameters|
if on_method_parameters.empty?
method_info[:if_string] = "if MacOS.version == :#{on_system_option}"
else
method_info[:on_system_string] = "#{method_info[:method]} :#{on_method_parameters.first}"
if_condition_operator = MACOS_VERSION_CONDITIONALS.key(on_method_parameters.first)
method_info[:if_string] = "if MacOS.version #{if_condition_operator} :#{on_system_option}"
end
end
elsif method_info[:method] == :on_system
on_system_method_call(method) do |macos_symbol|
base_os, condition = macos_symbol.to_s.split(/_(?=or_)/).map(&:to_sym)
method_info[:on_system_string] = if condition.present?
"on_system :linux, macos: :#{base_os}_#{condition}"
else
"on_system :linux, macos: :#{base_os}"
end
if_condition_operator = MACOS_VERSION_CONDITIONALS.key(condition)
method_info[:if_string] = "if OS.linux? || MacOS.version #{if_condition_operator} :#{base_os}"
end
end
offending_node(method)
problem "Don't use `#{method_info[:on_system_string]}` in `#{top_level_node_string}`, " \
"use `#{method_info[:if_string]}` instead." do |corrector|
block_node = offending_node.parent
next if block_node.type != :block
# TODO: could fix corrector to handle this but punting for now.
next if block_node.single_line?
source_range = offending_node.source_range.join(offending_node.parent.loc.begin)
corrector.replace(source_range, method_info[:if_string])
end
end
end
audit_on_system_blocks(block_node, formula_block_name) if block_node
end
# Don't restrict OS.mac? or OS.linux? usage in taps; they don't care
@ -483,115 +400,19 @@ module RuboCop
# that case.
return if formula_tap != "homebrew-core"
ALL_SYSTEM_OPTIONS.each do |on_system_option|
method_info = on_system_method_info(on_system_option)
audit_arch_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES)
if_nodes_to_check = []
audit_base_os_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES)
if ON_ARCH_OPTIONS.include?(on_system_option)
if_arch_node_search(body_node, arch: method_info[:if_method]) do |if_node, else_node|
else_info = if else_node.present?
{
can_autocorrect: true,
on_system_method: (on_system_option == :intel) ? "on_arm" : "on_intel",
node: else_node,
}
end
if_nodes_to_check << [if_node, else_info]
end
elsif ON_BASE_OS_OPTIONS.include?(on_system_option)
if_base_os_node_search(body_node, base_os: method_info[:if_method]) do |if_node, else_node|
else_info = if else_node.present?
{
can_autocorrect: true,
on_system_method: (on_system_option == :macos) ? "on_linux" : "on_macos",
node: else_node,
}
end
if_nodes_to_check << [if_node, else_info]
end
else
if_macos_version_node_search(body_node, os_name: on_system_option) do |if_node, operator, else_node|
if operator == :<
method_info[:on_system_string] = "on_system"
elsif operator == :<=
method_info[:on_system_string] = "on_system :linux, macos: :#{on_system_option}_or_older"
elsif operator != :== && MACOS_VERSION_CONDITIONALS.key?(operator.to_s)
method_info[:on_system_string] = "#{method_info[:method]} " \
":#{MACOS_VERSION_CONDITIONALS[operator.to_s]}"
end
method_info[:if_string] = "if #{method_info[:if_module]}.#{method_info[:if_method]} #{operator} " \
":#{on_system_option}"
if else_node.present? || !MACOS_VERSION_CONDITIONALS.key?(operator.to_s)
else_info = { can_autocorrect: false }
end
if_nodes_to_check << [if_node, else_info]
end
end
if_nodes_to_check.each do |if_node, else_info|
# TODO: check to see if it's legal
valid = T.let(false, T::Boolean)
if_node.each_ancestor do |ancestor|
valid_method_names = case ancestor.type
when :def
NO_ON_SYSTEM_METHOD_NAMES
when :block
NO_ON_SYSTEM_BLOCK_NAMES
else
next
end
next unless valid_method_names.include?(ancestor.method_name)
valid = true
break
end
next if valid
offending_node(if_node)
problem "Don't use `#{method_info[:if_string]}`, " \
"use `#{method_info[:on_system_string]} do` instead." do |corrector|
# TODO: could fix corrector to handle this but punting for now.
next if if_node.unless?
if else_info.present?
next unless else_info[:can_autocorrect]
corrector.replace(if_node.source_range,
"#{method_info[:on_system_string]} do\n#{if_node.body.source}\nend\n" \
"#{else_info[:on_system_method]} do\n#{else_info[:node].source}\nend")
else
corrector.replace(if_node.source_range,
"#{method_info[:on_system_string]} do\n#{if_node.body.source}\nend")
end
end
end
end
audit_macos_version_conditionals(body_node,
allowed_methods: NO_ON_SYSTEM_METHOD_NAMES,
allowed_blocks: NO_ON_SYSTEM_BLOCK_NAMES,
recommend_on_system: true)
end
def_node_matcher :on_macos_version_method_call, <<~PATTERN
(send nil? %on_method (sym ${:or_newer :or_older})?)
PATTERN
def_node_matcher :on_system_method_call, <<~PATTERN
(send nil? :on_system (sym :linux) (hash (pair (sym :macos) (sym $_))))
PATTERN
def_node_search :if_arch_node_search, <<~PATTERN
$(if (send (const (const nil? :Hardware) :CPU) %arch) _ $_)
PATTERN
def_node_search :if_base_os_node_search, <<~PATTERN
$(if (send (const nil? :OS) %base_os) _ $_)
PATTERN
def_node_search :if_macos_version_node_search, <<~PATTERN
$(if (send (send (const nil? :MacOS) :version) ${:== :<= :< :>= :> :!=} (sym %os_name)) _ $_)
PATTERN
end
# This cop checks for other miscellaneous style violations.

1
Library/Homebrew/rubocops/rubocop-cask.rb
View File

@ -16,6 +16,7 @@ require_relative "cask/mixin/on_url_stanza"
require_relative "cask/desc"
require_relative "cask/homepage_url_trailing_slash"
require_relative "cask/no_dsl_version"
require_relative "cask/on_system_conditionals"
require_relative "cask/stanza_order"
require_relative "cask/stanza_grouping"
require_relative "cask/url_legacy_comma_separators"

196
Library/Homebrew/rubocops/shared/on_system_conditionals_helper.rb Normal file
View File

@ -0,0 +1,196 @@
# typed: false
# frozen_string_literal: true
require "macos_versions"
require "rubocops/shared/helper_functions"
module RuboCop
module Cop
# This module performs common checks on `on_{system}` blocks in both formulae and casks.
#
# @api private
module OnSystemConditionalsHelper
extend NodePattern::Macros
include HelperFunctions
ARCH_OPTIONS = [:arm, :intel].freeze
BASE_OS_OPTIONS = [:macos, :linux].freeze
MACOS_VERSION_OPTIONS = MacOSVersions::SYMBOLS.keys.freeze
ON_SYSTEM_OPTIONS = [*ARCH_OPTIONS, *BASE_OS_OPTIONS, *MACOS_VERSION_OPTIONS, :system].freeze
MACOS_VERSION_CONDITIONALS = {
"==" => nil,
"<=" => :or_older,
">=" => :or_newer,
}.freeze
def audit_on_system_blocks(body_node, parent_name)
parent_string = if body_node.def_type?
"def #{parent_name}"
else
"#{parent_name} do"
end
ON_SYSTEM_OPTIONS.each do |on_system_option|
on_system_method = :"on_#{on_system_option}"
if_statement_string = if ARCH_OPTIONS.include?(on_system_option)
"if Hardware::CPU.#{on_system_option}?"
elsif BASE_OS_OPTIONS.include?(on_system_option)
"if OS.#{(on_system_option == :macos) ? "mac" : "linux"}?"
elsif on_system_option == :system
"if OS.linux? || MacOS.version"
else
"if MacOS.version"
end
find_every_method_call_by_name(body_node, on_system_method).each do |on_system_node|
if_conditional = ""
if MACOS_VERSION_OPTIONS.include? on_system_option
on_macos_version_method_call(on_system_node, on_method: on_system_method) do |on_method_parameters|
if on_method_parameters.empty?
if_conditional = " == :#{on_system_option}"
else
if_condition_operator = MACOS_VERSION_CONDITIONALS.key(on_method_parameters.first)
if_conditional = " #{if_condition_operator} :#{on_system_option}"
end
end
elsif on_system_option == :system
on_system_method_call(on_system_node) do |macos_symbol|
base_os, condition = macos_symbol.to_s.split(/_(?=or_)/).map(&:to_sym)
if_condition_operator = MACOS_VERSION_CONDITIONALS.key(condition)
if_conditional = " #{if_condition_operator} :#{base_os}"
end
end
offending_node(on_system_node)
problem "Don't use `#{on_system_node.source}` in `#{parent_string}`, " \
"use `#{if_statement_string}#{if_conditional}` instead." do |corrector|
block_node = offending_node.parent
next if block_node.type != :block
# TODO: could fix corrector to handle this but punting for now.
next if block_node.single_line?
source_range = offending_node.source_range.join(offending_node.parent.loc.begin)
corrector.replace(source_range, "#{if_statement_string}#{if_conditional}")
end
end
end
end
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)
if_statement_problem(if_node, "if Hardware::CPU.#{arch_option}?", "on_#{arch_option}",
else_method: else_method, else_node: else_node)
end
end
end
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]
else
[: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)
if_statement_problem(if_node, "if OS.#{os_method}", "on_#{base_os_option}",
else_method: else_method, else_node: else_node)
end
end
end
def audit_macos_version_conditionals(body_node, allowed_methods: [], allowed_blocks: [],
recommend_on_system: true)
MACOS_VERSION_OPTIONS.each do |macos_version_option|
if_macos_version_node_search(body_node, os_version: macos_version_option) do |if_node, operator, else_node|
next if if_node_is_allowed?(if_node, allowed_methods: allowed_methods, allowed_blocks: allowed_blocks)
autocorrect = else_node.blank? && MACOS_VERSION_CONDITIONALS.key?(operator.to_s)
on_system_method_string = if recommend_on_system && operator == :<
"on_system"
elsif recommend_on_system && operator == :<=
"on_system :linux, macos: :#{macos_version_option}_or_older"
elsif operator != :== && MACOS_VERSION_CONDITIONALS.key?(operator.to_s)
"on_#{macos_version_option} :#{MACOS_VERSION_CONDITIONALS[operator.to_s]}"
else
"on_#{macos_version_option}"
end
if_statement_problem(if_node, "if MacOS.version #{operator} :#{macos_version_option}",
on_system_method_string, autocorrect: autocorrect)
end
end
end
private
def if_statement_problem(if_node, if_statement_string, on_system_method_string,
else_method: nil, else_node: nil, autocorrect: true)
offending_node(if_node)
problem "Don't use `#{if_statement_string}`, " \
"use `#{on_system_method_string} do` instead." do |corrector|
next unless autocorrect
# TODO: could fix corrector to handle this but punting for now.
next if if_node.unless?
if else_method.present? && else_node.present?
corrector.replace(if_node.source_range,
"#{on_system_method_string} do\n#{if_node.body.source}\nend\n" \
"#{else_method} do\n#{else_node.source}\nend")
else
corrector.replace(if_node.source_range, "#{on_system_method_string} do\n#{if_node.body.source}\nend")
end
end
end
def if_node_is_allowed?(if_node, allowed_methods: [], allowed_blocks: [])
# TODO: check to see if it's legal
valid = false
if_node.each_ancestor do |ancestor|
valid_method_names = case ancestor.type
when :def
allowed_methods
when :block
allowed_blocks
else
next
end
next unless valid_method_names.include?(ancestor.method_name)
valid = true
break
end
return true if valid
false
end
def_node_matcher :on_macos_version_method_call, <<~PATTERN
(send nil? %on_method (sym ${:or_newer :or_older})?)
PATTERN
def_node_matcher :on_system_method_call, <<~PATTERN
(send nil? :on_system (sym :linux) (hash (pair (sym :macos) (sym $_))))
PATTERN
def_node_search :if_arch_node_search, <<~PATTERN
$(if (send (const (const nil? :Hardware) :CPU) %arch) _ $_)
PATTERN
def_node_search :if_base_os_node_search, <<~PATTERN
$(if (send (const nil? :OS) %base_os) _ $_)
PATTERN
def_node_search :if_macos_version_node_search, <<~PATTERN
$(if (send (send (const nil? :MacOS) :version) ${:== :<= :< :>= :> :!=} (sym %os_version)) _ $_)
PATTERN
end
end
end

140
Library/Homebrew/test/rubocops/cask/on_system_conditionals_spec.rb Normal file
View File

@ -0,0 +1,140 @@
# typed: false
# frozen_string_literal: true
require "rubocops/rubocop-cask"
require "test/rubocops/cask/shared_examples/cask_cop"
describe RuboCop::Cop::Cask::OnSystemConditionals do
include CaskCop
subject(:cop) { described_class.new }
context "when auditing `postflight` stanzas" do
context "when there are no on_system blocks" do
let(:source) do
<<-CASK.undent
postflight do
foobar
end
CASK
end
include_examples "does not report any offenses"
end
context "when there is an `on_intel` block" do
let(:source) do
<<-CASK.undent
cask 'foo' do
postflight do
on_intel do
foobar
end
end
end
CASK
end
let(:correct_source) do
<<-CASK.undent
cask 'foo' do
postflight do
if Hardware::CPU.intel?
foobar
end
end
end
CASK
end
let(:expected_offenses) do
[{
message: "Don't use `on_intel` in `postflight do`, use `if Hardware::CPU.intel?` instead.",
severity: :convention,
line: 3,
column: 4,
source: "on_intel",
}]
end
include_examples "reports offenses"
include_examples "autocorrects source"
end
context "when there is an `on_monterey` block" do
let(:source) do
<<-CASK.undent
cask 'foo' do
postflight do
on_monterey do
foobar
end
end
end
CASK
end
let(:correct_source) do
<<-CASK.undent
cask 'foo' do
postflight do
if MacOS.version == :monterey
foobar
end
end
end
CASK
end
let(:expected_offenses) do
[{
message: "Don't use `on_monterey` in `postflight do`, use `if MacOS.version == :monterey` instead.",
severity: :convention,
line: 3,
column: 4,
source: "on_monterey",
}]
end
include_examples "reports offenses"
include_examples "autocorrects source"
end
context "when there is an `on_monterey :or_older` block" do
let(:source) do
<<-CASK.undent
cask 'foo' do
postflight do
on_monterey :or_older do
foobar
end
end
end
CASK
end
let(:correct_source) do
<<-CASK.undent
cask 'foo' do
postflight do
if MacOS.version <= :monterey
foobar
end
end
end
CASK
end
let(:expected_offenses) do
[{
message: "Don't use `on_monterey :or_older` in `postflight do`, " \
"use `if MacOS.version <= :monterey` instead.",
severity: :convention,
line: 3,
column: 4,
source: "on_monterey :or_older",
}]
end
include_examples "reports offenses"
include_examples "autocorrects source"
end
end
end