Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rubocops/components_redundancy: stable/head block removal #16413

Merged
merged 1 commit into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions Library/Homebrew/rubocops/components_redundancy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,19 @@ module Cop
module FormulaAudit
# This cop checks if redundant components are present and for other component errors.
#
# - `url|checksum|mirror` should be inside `stable` block
# - `url|checksum|mirror|version` should be inside `stable` block
# - `head` and `head do` should not be simultaneously present
# - `bottle :unneeded`/`:disable` and `bottle do` should not be simultaneously present
# - `stable do` should not be present without a `head` spec
# - `stable do` should not be present with only `url|checksum|mirror|version`
# - `head do` should not be present with only `url`
#
# @api private
class ComponentsRedundancy < FormulaCop
HEAD_MSG = "`head` and `head do` should not be simultaneously present"
BOTTLE_MSG = "`bottle :modifier` and `bottle do` should not be simultaneously present"
STABLE_MSG = "`stable do` should not be present without a `head` spec"
STABLE_BLOCK_METHODS = [:url, :sha256, :mirror, :version].freeze

def audit_formula(_node, _class_node, _parent_class_node, body_node)
return if body_node.nil?
Expand All @@ -37,9 +40,24 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)

stable_block = find_block(body_node, :stable)
if stable_block
[:url, :sha256, :mirror].each do |method_name|
STABLE_BLOCK_METHODS.each do |method_name|
problem "`#{method_name}` should be put inside `stable` block" if method_called?(body_node, method_name)
end

unless stable_block.body.nil?
child_nodes = stable_block.body.begin_type? ? stable_block.body.child_nodes : [stable_block.body]
if child_nodes.all? { |n| n.send_type? && STABLE_BLOCK_METHODS.include?(n.method_name) }
problem "`stable do` should not be present with only #{STABLE_BLOCK_METHODS.join("/")}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like there's a better way to word this but I don't have any suggestions. Maybe worth mentioning it's redundant rather than broken.

end
end
end

head_block = find_block(body_node, :head)
if head_block && !head_block.body.nil?
child_nodes = head_block.body.begin_type? ? head_block.body.child_nodes : [head_block.body]
if child_nodes.all? { |n| n.send_type? && n.method_name == :url }
problem "`head do` should not be present with only `url`"
end
end

problem HEAD_MSG if method_called?(body_node, :head) &&
Expand Down
42 changes: 42 additions & 0 deletions Library/Homebrew/test/rubocops/components_redundancy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,47 @@ class Foo < Formula
end
RUBY
end

it "reports an offense if `stable do` or `head do` is present with only `url`" do
expect_offense(<<~RUBY)
class Foo < Formula
stable do
^^^^^^^^^ FormulaAudit/ComponentsRedundancy: `stable do` should not be present with only url/sha256/mirror/version
url "https://brew.sh/foo-1.0.tgz"
end
head do
^^^^^^^ FormulaAudit/ComponentsRedundancy: `head do` should not be present with only `url`
url "https://brew.sh/foo.git"
end
end
RUBY
end

it "reports no offenses if `stable do` is present with `url` and `depends_on`" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
head "https://brew.sh/foo.git"
stable do
url "https://brew.sh/foo-1.0.tgz"
depends_on "bar"
end
end
RUBY
end

it "reports no offenses if `head do` is present with `url` and `depends_on`" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
head do
url "https://brew.sh/foo.git"
depends_on "bar"
end
end
RUBY
end
end
end