Skip to content

Commit

Permalink
Merge pull request #16718 from issyl0/pyyaml-resource-needs-specific-…
Browse files Browse the repository at this point in the history
…deps-too

rubocop: The `pyyaml` resource requires `depends_on "libyaml"`
  • Loading branch information
issyl0 authored Feb 21, 2024
2 parents 8b977ce + ff21ef0 commit 6a9c9c0
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 8 deletions.
1 change: 1 addition & 0 deletions Library/Homebrew/rubocops/all.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
require_relative "livecheck"
require_relative "options"
require_relative "patches"
require_relative "resource_requires_dependencies"
require_relative "service"
require_relative "text"
require_relative "urls"
Expand Down
34 changes: 26 additions & 8 deletions Library/Homebrew/rubocops/resource_requires_dependencies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
module RuboCop
module Cop
module FormulaAudit
# This cop audits Python formulae that include the "lxml" resource
# This cop audits Python formulae that include certain resources
# to ensure that they also have the correct `uses_from_macos`
# dependencies.
#
Expand All @@ -16,15 +16,33 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
return if body_node.nil?

resource_nodes = find_every_method_call_by_name(body_node, :resource)
lxml = resource_nodes.find { |node| node.arguments.first.str_content == "lxml" }
return unless lxml
return if resource_nodes.empty?

uses_from_macos_nodes = find_every_method_call_by_name(body_node, :uses_from_macos)
dependencies = uses_from_macos_nodes.map { |node| node.arguments.first.str_content }
return if dependencies.include?("libxml2") && dependencies.include?("libxslt")
%w[lxml pyyaml].each do |resource_name|
found = resource_nodes.find { |node| node.arguments&.first&.str_content == resource_name }
next unless found

offending_node(lxml)
problem "Add `uses_from_macos` lines above for `\"libxml2\"` and `\"libxslt\"`."
uses_from_macos_nodes = find_every_method_call_by_name(body_node, :uses_from_macos)
uses_from_macos = uses_from_macos_nodes.map { |node| node.arguments.first.str_content }

depends_on_nodes = find_every_method_call_by_name(body_node, :depends_on)
depends_on = depends_on_nodes.map { |node| node.arguments.first.str_content }

required_deps = case resource_name
when "lxml"
kind = "uses_from_macos"
["libxml2", "libxslt"]
when "pyyaml"
kind = "depends_on"
["libyaml"]
else
[]
end
next if required_deps.all? { |dep| uses_from_macos.include?(dep) || depends_on.include?(dep) }

offending_node(found)
problem "Add `#{kind}` lines above for #{required_deps.map { |req| "`\"#{req}\"`" }.join(" and ")}."
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@
RSpec.describe RuboCop::Cop::FormulaAudit::ResourceRequiresDependencies do
subject(:cop) { described_class.new }

context "when a formula does not have any resources" do
it "does not report offenses" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
homepage "https://brew.sh"
uses_from_macos "libxml2"
end
RUBY
end
end

context "when a formula does not have the lxml resource" do
it "does not report offenses" do
expect_no_offenses(<<~RUBY)
Expand Down Expand Up @@ -59,4 +72,84 @@ class Foo < Formula
RUBY
end
end

context "when a formula does not have the pyyaml resource" do
it "does not report offenses" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
homepage "https://brew.sh"
uses_from_macos "libxml2"
resource "not-pyyaml" do
url "blah"
sha256 "blah"
end
end
RUBY
end
end

context "when a formula has the pyyaml resource" do
it "does not report offenses if the dependencies are present" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
homepage "https://brew.sh"
depends_on "libyaml"
resource "pyyaml" do
url "blah"
sha256 "blah"
end
end
RUBY
end

it "reports offenses if missing a dependency" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
homepage "https://brew.sh"
depends_on "not_libyaml"
resource "pyyaml" do
^^^^^^^^^^^^^^^^^ FormulaAudit/ResourceRequiresDependencies: Add `depends_on` lines above for `"libyaml"`.
url "blah"
sha256 "blah"
end
end
RUBY
end
end

context "when a formula has multiple resources" do
it "reports offenses for each resource that is missing a dependency" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
homepage "https://brew.sh"
uses_from_macos "one"
uses_from_macos "two"
depends_on "three"
resource "lxml" do
^^^^^^^^^^^^^^^ FormulaAudit/ResourceRequiresDependencies: Add `uses_from_macos` lines above for `"libxml2"` and `"libxslt"`.
url "blah"
sha256 "blah"
end
resource "pyyaml" do
^^^^^^^^^^^^^^^^^ FormulaAudit/ResourceRequiresDependencies: Add `depends_on` lines above for `"libyaml"`.
url "blah"
sha256 "blah"
end
end
RUBY
end
end
end

0 comments on commit 6a9c9c0

Please sign in to comment.