audit: add uses_from_macos dependency ordering

This commit is contained in:
Jonathan Chang 2020-01-12 10:59:22 -08:00
parent 798e0a651d
commit abf2b83b35
4 changed files with 128 additions and 9 deletions

View File

@ -32,6 +32,7 @@ module RuboCop
[{ name: :option, type: :method_call }], [{ name: :option, type: :method_call }],
[{ name: :deprecated_option, type: :method_call }], [{ name: :deprecated_option, type: :method_call }],
[{ name: :depends_on, type: :method_call }], [{ name: :depends_on, type: :method_call }],
[{ name: :uses_from_macos, type: :method_call }],
[{ name: :conflicts_with, type: :method_call }], [{ name: :conflicts_with, type: :method_call }],
[{ name: :skip_clean, type: :method_call }], [{ name: :skip_clean, type: :method_call }],
[{ name: :cxxstdlib_check, type: :method_call }], [{ name: :cxxstdlib_check, type: :method_call }],

View File

@ -12,29 +12,37 @@ module RuboCop
class DependencyOrder < FormulaCop class DependencyOrder < FormulaCop
def audit_formula(_node, _class_node, _parent_class_node, body_node) def audit_formula(_node, _class_node, _parent_class_node, body_node)
check_dependency_nodes_order(body_node) check_dependency_nodes_order(body_node)
check_uses_from_macos_nodes_order(body_node)
[:devel, :head, :stable].each do |block_name| [:devel, :head, :stable].each do |block_name|
block = find_block(body_node, block_name) block = find_block(body_node, block_name)
next unless block next unless block
check_dependency_nodes_order(block.body) check_dependency_nodes_order(block.body)
check_uses_from_macos_nodes_order(block.body)
end end
end end
def check_uses_from_macos_nodes_order(parent_node)
return if parent_node.nil?
dependency_nodes = parent_node.each_child_node.select { |x| uses_from_macos_node?(x) }
ensure_dependency_order(dependency_nodes)
end
def check_dependency_nodes_order(parent_node) def check_dependency_nodes_order(parent_node)
return if parent_node.nil? return if parent_node.nil?
dependency_nodes = fetch_depends_on_nodes(parent_node) dependency_nodes = parent_node.each_child_node.select { |x| depends_on_node?(x) }
ordered = dependency_nodes.sort_by { |node| dependency_name(node).downcase } ensure_dependency_order(dependency_nodes)
end
def ensure_dependency_order(nodes)
ordered = nodes.sort_by { |node| dependency_name(node).downcase }
ordered = sort_dependencies_by_type(ordered) ordered = sort_dependencies_by_type(ordered)
sort_conditional_dependencies!(ordered) sort_conditional_dependencies!(ordered)
verify_order_in_source(ordered) verify_order_in_source(ordered)
end end
# Match all `depends_on` nodes among childnodes of given parent node
def fetch_depends_on_nodes(parent_node)
parent_node.each_child_node.select { |x| depends_on_node?(x) }
end
# Separate dependencies according to precedence order: # Separate dependencies according to precedence order:
# build-time > test > normal > recommended > optional # build-time > test > normal > recommended > optional
def sort_dependencies_by_type(dependency_nodes) def sort_dependencies_by_type(dependency_nodes)
@ -101,6 +109,11 @@ module RuboCop
(send nil? :depends_on ...)} (send nil? :depends_on ...)}
EOS EOS
def_node_matcher :uses_from_macos_node?, <<~EOS
{(if _ (send nil? :uses_from_macos ...) nil?)
(send nil? :uses_from_macos ...)}
EOS
def_node_search :buildtime_dependency?, "(sym :build)" def_node_search :buildtime_dependency?, "(sym :build)"
def_node_search :recommended_dependency?, "(sym :recommended)" def_node_search :recommended_dependency?, "(sym :recommended)"
@ -111,9 +124,9 @@ module RuboCop
def_node_search :negate_normal_dependency?, "(sym {:build :recommended :test :optional})" def_node_search :negate_normal_dependency?, "(sym {:build :recommended :test :optional})"
# Node pattern method to extract `name` in `depends_on :name` # Node pattern method to extract `name` in `depends_on :name` or `uses_from_macos :name`
def_node_search :dependency_name_node, <<~EOS def_node_search :dependency_name_node, <<~EOS
{(send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) {(send nil? {:depends_on :uses_from_macos} {(hash (pair $_ _)) $({str sym} _) $(const nil? _)})
(if _ (send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) nil?)} (if _ (send nil? :depends_on {(hash (pair $_ _)) $({str sym} _) $(const nil? _)}) nil?)}
EOS EOS

View File

@ -6,6 +6,19 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context "When auditing formula components order" do context "When auditing formula components order" do
it "When uses_from_macos precedes depends_on" do
expect_offense(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "apple"
depends_on "foo"
^^^^^^^^^^^^^^^^ `depends_on` (line 6) should be put before `uses_from_macos` (line 5)
end
RUBY
end
it "When url precedes homepage" do it "When url precedes homepage" do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
class Foo < Formula class Foo < Formula

View File

@ -5,6 +5,75 @@ require "rubocops/dependency_order"
describe RuboCop::Cop::FormulaAudit::DependencyOrder do describe RuboCop::Cop::FormulaAudit::DependencyOrder do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
context "uses_from_macos" do
it "wrong conditional uses_from_macos order" do
expect_offense(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "apple" if build.with? "foo"
uses_from_macos "foo" => :optional
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 5) should be put before dependency "apple" (line 4)
end
RUBY
end
it "wrong alphabetical uses_from_macos order" do
expect_offense(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "foo"
uses_from_macos "bar"
^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "foo" (line 4)
end
RUBY
end
it "supports requirement constants" do
expect_offense(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos FooRequirement
uses_from_macos "bar"
^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 5) should be put before dependency "FooRequirement" (line 4)
end
RUBY
end
it "wrong conditional uses_from_macos order with block" do
expect_offense(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
head do
uses_from_macos "apple" if build.with? "foo"
uses_from_macos "bar"
^^^^^^^^^^^^^^^^^^^^^ dependency "bar" (line 6) should be put before dependency "apple" (line 5)
uses_from_macos "foo" => :optional
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 7) should be put before dependency "apple" (line 5)
end
uses_from_macos "apple" if build.with? "foo"
uses_from_macos "foo" => :optional
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dependency "foo" (line 10) should be put before dependency "apple" (line 9)
end
RUBY
end
it "correct uses_from_macos order for multiple tags" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "bar" => [:build, :test]
uses_from_macos "foo" => :build
uses_from_macos "apple"
end
RUBY
end
end
context "depends_on" do context "depends_on" do
it "wrong conditional depends_on order" do it "wrong conditional depends_on order" do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
@ -75,6 +144,29 @@ describe RuboCop::Cop::FormulaAudit::DependencyOrder do
end end
context "autocorrect" do context "autocorrect" do
it "wrong conditional uses_from_macos order" do
source = <<~RUBY
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "apple" if build.with? "foo"
uses_from_macos "foo" => :optional
end
RUBY
correct_source = <<~RUBY
class Foo < Formula
homepage "https://brew.sh"
url "https://brew.sh/foo-1.0.tgz"
uses_from_macos "foo" => :optional
uses_from_macos "apple" if build.with? "foo"
end
RUBY
corrected_source = autocorrect_source(source)
expect(corrected_source).to eq(correct_source)
end
it "wrong conditional depends_on order" do it "wrong conditional depends_on order" do
source = <<~RUBY source = <<~RUBY
class Foo < Formula class Foo < Formula