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

Merge pull request #13503 from Rylan12/components-order-on-system

Browse Source 
Update `components_order` cop to check other `on_{system}` methods
This commit is contained in:
Rylan Polster 2022-07-01 11:55:16 -04:00 committed by GitHub
commit d541ab089e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 365 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Library/Homebrew/ast_constants.rb
View File

@ -1,6 +1,8 @@
# typed: true
# frozen_string_literal: true
require "macos_versions"
FORMULA_COMPONENT_PRECEDENCE_LIST = [
[{ name: :include, type: :method_call }],
[{ name: :desc, type: :method_call }],
@ -28,6 +30,11 @@ FORMULA_COMPONENT_PRECEDENCE_LIST = [
[{ name: :uses_from_macos, type: :method_call }],
[{ name: :on_macos, type: :block_call }],
[{ name: :on_linux, type: :block_call }],
[{ name: :on_arm, type: :block_call }],
[{ name: :on_intel, type: :block_call }],
*MacOSVersions::SYMBOLS.keys.map do |os_name|
[{ name: :"on_#{os_name}", type: :block_call }]
end,
[{ name: :conflicts_with, type: :method_call }],
[{ name: :skip_clean, type: :method_call }],
[{ name: :cxxstdlib_check, type: :method_call }],

114
Library/Homebrew/rubocops/components_order.rb
View File

@ -14,6 +14,12 @@ module RuboCop
class ComponentsOrder < FormulaCop
extend AutoCorrector
def on_system_methods
@on_system_methods ||= [:intel, :arm, :macos, :linux, *MacOSVersions::SYMBOLS.keys].map do |m|
:"on_#{m}"
end
end
def audit_formula(_node, _class_node, _parent_class_node, body_node)
@present_components, @offensive_nodes = check_order(FORMULA_COMPONENT_PRECEDENCE_LIST, body_node)
@ -25,48 +31,45 @@ module RuboCop
[{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }],
]
on_macos_blocks = find_blocks(body_node, :on_macos)
on_system_methods.each do |on_method|
on_method_blocks = find_blocks(body_node, on_method)
next if on_method_blocks.empty?
if on_macos_blocks.length > 1
@offensive_node = on_macos_blocks.second
problem "there can only be one `on_macos` block in a formula."
if on_method_blocks.length > 1
@offensive_node = on_method_blocks.second
problem "there can only be one `#{on_method}` block in a formula."
end
check_on_system_block_content(component_precedence_list, on_method_blocks.first)
end
check_on_os_block_content(component_precedence_list, on_macos_blocks.first) if on_macos_blocks.any?
on_linux_blocks = find_blocks(body_node, :on_linux)
if on_linux_blocks.length > 1
@offensive_node = on_linux_blocks.second
problem "there can only be one `on_linux` block in a formula."
end
check_on_os_block_content(component_precedence_list, on_linux_blocks.first) if on_linux_blocks.any?
resource_blocks = find_blocks(body_node, :resource)
resource_blocks.each do |resource_block|
on_macos_blocks = find_blocks(resource_block.body, :on_macos)
on_linux_blocks = find_blocks(resource_block.body, :on_linux)
on_system_blocks = {}
if on_macos_blocks.length.zero? && on_linux_blocks.length.zero?
# Found nothing. Try without .body as depending on the code,
# on_macos or on_linux might be in .body or not ...
on_macos_blocks = find_blocks(resource_block, :on_macos)
on_linux_blocks = find_blocks(resource_block, :on_linux)
next if on_macos_blocks.length.zero? && on_linux_blocks.length.zero?
on_system_methods.each do |on_method|
on_system_blocks[on_method] = find_blocks(resource_block.body, on_method)
end
if on_system_blocks.empty?
# Found nothing. Try without .body as depending on the code,
# on_{system} might be in .body or not ...
on_system_methods.each do |on_method|
on_system_blocks[on_method] = find_blocks(resource_block, on_method)
end
end
next if on_system_blocks.empty?
@offensive_node = resource_block
next if on_macos_blocks.length.zero? && on_linux_blocks.length.zero?
on_system_bodies = []
on_os_bodies = []
(on_macos_blocks + on_linux_blocks).each do |on_os_block|
on_os_body = on_os_block.body
branches = on_os_body.if_type? ? on_os_body.branches : [on_os_body]
on_os_bodies += branches.map { |branch| [on_os_block, branch] }
on_system_blocks.each_value do |blocks|
blocks.each do |on_system_block|
on_system_body = on_system_block.body
branches = on_system_body.if_type? ? on_system_body.branches : [on_system_body]
on_system_bodies += branches.map { |branch| [on_system_block, branch] }
end
end
message = nil
@ -79,14 +82,20 @@ module RuboCop
minimum_methods = allowed_methods.first.map { |m| "`#{m}`" }.to_sentence
maximum_methods = allowed_methods.last.map { |m| "`#{m}`" }.to_sentence
on_os_bodies.each do |on_os_block, on_os_body|
method_name = on_os_block.method_name
child_nodes = on_os_body.begin_type? ? on_os_body.child_nodes : [on_os_body]
if child_nodes.all? { |n| n.send_type? || n.block_type? }
method_names = child_nodes.map(&:method_name)
next if allowed_methods.include? method_names
on_system_bodies.each do |on_system_block, on_system_body|
method_name = on_system_block.method_name
child_nodes = on_system_body.begin_type? ? on_system_body.child_nodes : [on_system_body]
if child_nodes.all? { |n| n.send_type? || n.block_type? || n.lvasgn_type? }
method_names = child_nodes.map do |node|
next if node.lvasgn_type?
next if node.method_name == :patch
next if on_system_methods.include? node.method_name
node.method_name
end.compact
next if method_names.empty? || allowed_methods.include?(method_names)
end
offending_node(on_os_block)
offending_node(on_system_block)
message = "`#{method_name}` blocks within `resource` blocks must contain at least " \
"#{minimum_methods} and at most #{maximum_methods} (in order)."
break
@ -97,32 +106,31 @@ module RuboCop
next
end
if on_macos_blocks.length > 1
problem "there can only be one `on_macos` block in a resource block."
next
end
if on_linux_blocks.length > 1
problem "there can only be one `on_linux` block in a resource block."
next
on_system_blocks.each do |on_method, blocks|
if blocks.length > 1
problem "there can only be one `#{on_method}` block in a resource block."
next
end
end
end
end
def check_on_os_block_content(component_precedence_list, on_os_block)
on_os_allowed_methods = %w[
def check_on_system_block_content(component_precedence_list, on_system_block)
on_system_allowed_methods = %w[
depends_on
patch
resource
deprecate!
disable!
conflicts_with
fails_with
keg_only
ignore_missing_libraries
]
_, offensive_node = check_order(component_precedence_list, on_os_block.body)
on_system_allowed_methods += on_system_methods.map(&:to_s)
_, offensive_node = check_order(component_precedence_list, on_system_block.body)
component_problem(*offensive_node) if offensive_node
child_nodes = on_os_block.body.begin_type? ? on_os_block.body.child_nodes : [on_os_block.body]
child_nodes = on_system_block.body.begin_type? ? on_system_block.body.child_nodes : [on_system_block.body]
child_nodes.each do |child|
valid_node = depends_on_node?(child)
# Check for RuboCop::AST::SendNode and RuboCop::AST::BlockNode instances
@ -130,13 +138,13 @@ module RuboCop
method_type = child.send_type? || child.block_type?
next unless method_type
valid_node ||= on_os_allowed_methods.include? child.method_name.to_s
valid_node ||= on_system_allowed_methods.include? child.method_name.to_s
@offensive_node = child
next if valid_node
problem "`#{on_os_block.method_name}` cannot include `#{child.method_name}`. " \
"Only #{on_os_allowed_methods.map { |m| "`#{m}`" }.to_sentence} are allowed."
problem "`#{on_system_block.method_name}` cannot include `#{child.method_name}`. " \
"Only #{on_system_allowed_methods.map { |m| "`#{m}`" }.to_sentence} are allowed."
end
end

313
Library/Homebrew/test/rubocops/components_order_spec.rb
View File

@ -337,7 +337,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
context "when formula has no OS-specific blocks" do
context "when formula has no system-specific blocks" do
it "reports no offenses" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
@ -352,7 +352,7 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
end
end
context "when formula has OS-specific block(s)" do
context "when formula has system-specific block(s)" do
it "reports no offenses when `on_macos` and `on_linux` are used correctly" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
@ -363,7 +363,13 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
uses_from_macos "libxml2"
on_macos do
depends_on "perl"
on_arm do
depends_on "perl"
end
on_intel do
depends_on "python"
end
resource "resource1" do
url "https://brew.sh/resource1.tar.gz"
@ -433,6 +439,70 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports no offenses when `on_intel` is used correctly" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_intel do
disable! because: :does_not_build
depends_on "readline"
end
def install
end
end
RUBY
end
it "reports no offenses when `on_arm` is used correctly" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_arm do
deprecate! because: "it's deprecated"
depends_on "readline"
end
def install
end
end
RUBY
end
it "reports no offenses when `on_monterey` is used correctly" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_monterey do
disable! because: :does_not_build
depends_on "readline"
end
def install
end
end
RUBY
end
it "reports no offenses when `on_monterey :or_older` is used correctly" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_monterey :or_older do
deprecate! because: "it's deprecated"
depends_on "readline"
end
def install
end
end
RUBY
end
it "reports an offense when there are multiple `on_macos` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
@ -465,6 +535,70 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports an offense when there are multiple `on_intel` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_intel do
depends_on "readline"
end
on_intel do
^^^^^^^^^^^ there can only be one `on_intel` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "reports an offense when there are multiple `on_arm` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_arm do
depends_on "readline"
end
on_arm do
^^^^^^^^^ there can only be one `on_arm` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "reports an offense when there are multiple `on_monterey` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_monterey do
depends_on "readline"
end
on_monterey do
^^^^^^^^^^^^^^ there can only be one `on_monterey` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "reports an offense when there are multiple `on_monterey` blocks with parameters" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_monterey do
depends_on "readline"
end
on_monterey :or_older do
^^^^^^^^^^^^^^^^^^^^^^^^ there can only be one `on_monterey` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "reports an offense when the `on_macos` block contains nodes other than `depends_on`, `patch` or `resource`" do
expect_offense(<<~RUBY)
class Foo < Formula
@ -491,6 +625,60 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports an offense when the `on_intel` block contains nodes other than `depends_on`, `patch` or `resource`" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_intel do
depends_on "readline"
uses_from_macos "ncurses"
^^^^^^^^^^^^^^^^^^^^^^^^^ `on_intel` cannot include `uses_from_macos`. [...]
end
end
RUBY
end
it "reports an offense when the `on_arm` block contains nodes other than `depends_on`, `patch` or `resource`" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_arm do
depends_on "readline"
uses_from_macos "ncurses"
^^^^^^^^^^^^^^^^^^^^^^^^^ `on_arm` cannot include `uses_from_macos`. [...]
end
end
RUBY
end
it "reports an offense when the `on_monterey` block contains nodes other than " \
"`depends_on`, `patch` or `resource`" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_monterey do
depends_on "readline"
uses_from_macos "ncurses"
^^^^^^^^^^^^^^^^^^^^^^^^^ `on_monterey` cannot include `uses_from_macos`. [...]
end
end
RUBY
end
it "reports an offense when the `on_monterey :or_older` block contains nodes other than " \
"`depends_on`, `patch` or `resource`" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_monterey :or_older do
depends_on "readline"
uses_from_macos "ncurses"
^^^^^^^^^^^^^^^^^^^^^^^^^ `on_monterey` cannot include `uses_from_macos`. [...]
end
end
RUBY
end
context "when in a resource block" do
it "reports no offenses for a valid `on_macos` and `on_linux` block" do
expect_no_offenses(<<~RUBY)
@ -512,19 +700,19 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports no offenses for a valid `on_macos` and `on_linux` block (with `version`)" do
it "reports no offenses for a valid `on_arm` and `on_intel` block (with `version`)" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
resource do
on_macos do
on_arm do
url "https://brew.sh/resource1.tar.gz"
version "1.2.3"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
on_intel do
url "https://brew.sh/resource2.tar.gz"
version "1.2.3"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
@ -576,6 +764,69 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports an offense if there are two `on_intel` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ there can only be one `on_intel` block in a resource block.
on_intel do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_intel do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "reports an offense if there are two `on_arm` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ there can only be one `on_arm` block in a resource block.
on_arm do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_arm do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "reports an offense if there are two `on_monterey` blocks" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ there can only be one `on_monterey` block in a resource block.
on_monterey do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_monterey :or_older do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "reports no offenses if there is an `on_macos` block but no `on_linux` block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
@ -604,6 +855,34 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports no offenses if there is an `on_intel` block but no `on_arm` block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
on_intel do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "reports no offenses if there is an `on_arm` block but no `on_intel` block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
on_arm do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "reports an offense if the content of an `on_macos` block is improperly formatted" do
expect_offense(<<~RUBY)
class Foo < Formula
@ -697,19 +976,19 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports an offense if the content of an `on_linux` block is improperly formatted" do
it "reports an offense if the content of an `on_arm` block is improperly formatted" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
on_macos do
on_intel do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
^^^^^^^^^^^ `on_linux` blocks within `resource` blocks must contain at least `url` and `sha256` and at most `url`, `mirror`, `version` and `sha256` (in order).
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
@ -718,18 +997,18 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports no offenses if an `on_linux` block has if-else branches that are properly formatted" do
it "reports no offenses if an `on_arm` block has if-else branches that are properly formatted" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
on_macos do
on_intel do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
on_arm do
if foo == :bar
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
@ -743,19 +1022,19 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
RUBY
end
it "reports an offense if an `on_linux` block has if-else branches that aren't properly formatted" do
it "reports an offense if an `on_arm` block has if-else branches that aren't properly formatted" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
on_macos do
on_intel do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
^^^^^^^^^^^ `on_linux` blocks within `resource` blocks must contain at least `url` and `sha256` and at most `url`, `mirror`, `version` and `sha256` (in order).
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
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"

2
manpages/brew.1
View File

@ -1,7 +1,7 @@
.\" generated with Ronn/v0.7.3
.\" http://github.com/rtomayko/ronn/tree/0.7.3
.
.TH "BREW" "1" "June 2022" "Homebrew" "brew"
.TH "BREW" "1" "July 2022" "Homebrew" "brew"
.
.SH "NAME"
\fBbrew\fR \- The Missing Package Manager for macOS (or Linux)