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

on_os_blocks: add audit

Browse Source
This commit is contained in:
Michka Popoff 2020-04-23 11:32:23 +02:00
parent 5fb158f52c
commit c494528f15
2 changed files with 486 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -10,6 +10,11 @@ module RuboCop
# - `component_precedence_list` has component hierarchy in a nested list
# where each sub array contains components' details which are at same precedence level
class ComponentsOrder < FormulaCop
# `aspell`: options and resources should be grouped by language
COMPONENT_WHITELIST = %w[
aspell
].freeze
def audit_formula(_node, _class_node, _parent_class_node, body_node)
component_precedence_list = [
[{ name: :include, type: :method_call }],
@ -34,6 +39,8 @@ module RuboCop
[{ name: :deprecated_option, type: :method_call }],
[{ name: :depends_on, type: :method_call }],
[{ name: :uses_from_macos, type: :method_call }],
[{ name: :on_macos, type: :block_call }],
[{ name: :on_linux, type: :block_call }],
[{ name: :conflicts_with, type: :method_call }],
[{ name: :skip_clean, type: :method_call }],
[{ name: :cxxstdlib_check, type: :method_call }],
@ -49,50 +56,115 @@ module RuboCop
[{ name: :test, type: :block_call }],
]
@present_components = component_precedence_list.map do |components|
relevant_components = []
components.each do |component|
case component[:type]
when :method_call
relevant_components += find_method_calls_by_name(body_node, component[:name]).to_a
when :block_call
relevant_components += find_blocks(body_node, component[:name]).to_a
when :method_definition
relevant_components << find_method_def(body_node, component[:name])
end
end
relevant_components.delete_if(&:nil?)
@present_components, @offensive_nodes = check_order(component_precedence_list, body_node)
component_problem @offensive_nodes[0], @offensive_nodes[1] if @offensive_nodes
component_precedence_list = [
[{ name: :depends_on, type: :method_call }],
[{ name: :resource, type: :block_call }],
[{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }],
]
on_macos_blocks = find_blocks(body_node, :on_macos)
if on_macos_blocks.length > 1
@offensive_node = on_macos_blocks.second
@offense_source_range = on_macos_blocks.second.source_range
problem "there can only be one `on_macos` block in a formula."
end
# Check if each present_components is above rest of the present_components
@present_components.take(@present_components.size - 1).each_with_index do |preceding_component, p_idx|
next if preceding_component.empty?
check_on_os_block_content(component_precedence_list, on_macos_blocks.first) if on_macos_blocks.any?
@present_components.drop(p_idx + 1).each do |succeeding_component|
next if succeeding_component.empty?
on_linux_blocks = find_blocks(body_node, :on_linux)
@offensive_nodes = check_precedence(preceding_component, succeeding_component)
component_problem @offensive_nodes[0], @offensive_nodes[1] if @offensive_nodes
if on_linux_blocks.length > 1
@offensive_node = on_linux_blocks.second
@offense_source_range = on_linux_blocks.second.source_range
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)
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?
end
@offensive_node = resource_block
@offense_source_range = resource_block.source_range
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
end
if on_macos_blocks.length == 1 && on_linux_blocks.length.zero?
problem "you need to define an `on_linux` block within your resource block."
next
end
if on_macos_blocks.length.zero? && on_linux_blocks.length == 1
problem "you need to define an `on_macos` block within your resource block."
next
end
on_macos_block = on_macos_blocks.first
on_linux_block = on_linux_blocks.first
child_nodes = on_macos_block.body.child_nodes
if child_nodes[0].method_name.to_s != "url" && child_nodes[1].method_name.to_s != "sha256"
problem "only an url and a sha256 (in the right order) are allowed in a `on_macos` " \
"block within a resource block."
next
end
child_nodes = on_linux_block.body.child_nodes
if child_nodes[0].method_name.to_s != "url" && child_nodes[1].method_name.to_s != "sha256"
problem "only an url and a sha256 (in the right order) are allowed in a `on_linux` " \
"block within a resource block."
end
end
end
# `aspell`: options and resources should be grouped by language
WHITELIST = %w[
aspell
].freeze
def check_on_os_block_content(component_precedence_list, on_os_block)
_, offensive_node = check_order(component_precedence_list, on_os_block.body)
component_problem(*offensive_node) if offensive_node
on_os_block.body.child_nodes.each do |child|
valid_node = depends_on_node?(child)
# Check for RuboCop::AST::SendNode instances only, as we are checking the
# method_name for patches and resources.
next unless child.instance_of? RuboCop::AST::SendNode
# Method to format message for reporting component precedence violations
def component_problem(c1, c2)
return if WHITELIST.include?(@formula_name)
valid_node ||= child.method_name.to_s == "patch"
valid_node ||= child.method_name.to_s == "resource"
problem "`#{format_component(c1)}` (line #{line_number(c1)}) " \
"should be put before `#{format_component(c2)}` " \
"(line #{line_number(c2)})"
@offensive_node = on_os_block
@offense_source_range = on_os_block.source_range
unless valid_node
problem "`#{on_os_block.method_name}` can only include `depends_on`, `patch` and `resource` nodes."
end
end
end
# autocorrect method gets called just after component_problem method call
def autocorrect(_node)
return if @offensive_nodes.nil?
succeeding_node = @offensive_nodes[0]
preceding_node = @offensive_nodes[1]
lambda do |corrector|
@ -130,6 +202,52 @@ module RuboCop
return [idx, comp.index(node1), comp] if comp.member?(node1)
end
end
def check_order(component_precedence_list, body_node)
present_components = component_precedence_list.map do |components|
components.flat_map do |component|
case component[:type]
when :method_call
find_method_calls_by_name(body_node, component[:name]).to_a
when :block_call
find_blocks(body_node, component[:name]).to_a
when :method_definition
find_method_def(body_node, component[:name])
end
end.compact
end
# Check if each present_components is above rest of the present_components
offensive_nodes = nil
present_components.take(present_components.size - 1).each_with_index do |preceding_component, p_idx|
next if preceding_component.empty?
present_components.drop(p_idx + 1).each do |succeeding_component|
next if succeeding_component.empty?
offensive_nodes = check_precedence(preceding_component, succeeding_component)
break if offensive_nodes
end
end
[present_components, offensive_nodes]
end
# Method to format message for reporting component precedence violations
def component_problem(c1, c2)
return if COMPONENT_WHITELIST.include?(@formula_name)
problem "`#{format_component(c1)}` (line #{line_number(c1)}) " \
"should be put before `#{format_component(c2)}` " \
"(line #{line_number(c2)})"
end
# Node pattern method to match
# `depends_on` variants
def_node_matcher :depends_on_node?, <<~EOS
{(if _ (send nil? :depends_on ...) nil?)
(send nil? :depends_on ...)}
EOS
end
end
end

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

@ -90,6 +90,47 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
end
RUBY
end
it "the on_macos block is not after uses_from_macos" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_macos do
depends_on "readline"
end
uses_from_macos "bar"
^^^^^^^^^^^^^^^^^^^^^ `uses_from_macos` (line 6) should be put before `on_macos` (line 3)
end
RUBY
end
it "the on_linux block is not after uses_from_macos" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_linux do
depends_on "readline"
end
uses_from_macos "bar"
^^^^^^^^^^^^^^^^^^^^^ `uses_from_macos` (line 6) should be put before `on_linux` (line 3)
end
RUBY
end
it "the on_linux block is not after the on_macos block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_linux do
depends_on "vim"
end
on_macos do
^^^^^^^^^^^ `on_macos` (line 6) should be put before `on_linux` (line 3)
depends_on "readline"
end
end
RUBY
end
end
context "When auditing formula components order with autocorrect" do
@ -141,4 +182,301 @@ describe RuboCop::Cop::FormulaAudit::ComponentsOrder do
expect(corrected_source).to eq(correct_source)
end
end
context "no on_os_block" do
it "does not fail when there is no on_os block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
depends_on "pkg-config" => :build
def install
end
end
RUBY
end
end
context "on_os_block" do
it "correctly uses on_macos and on_linux blocks" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
depends_on "pkg-config" => :build
uses_from_macos "libxml2"
on_macos do
depends_on "perl"
resource "resource1" do
url "https://brew.sh/resource1.tar.gz"
sha256 "a2f5650770e1c87fb335af19a9b7eb73fc05ccf22144eb68db7d00cd2bcb0902"
patch do
url "https://raw.githubusercontent.com/Homebrew/formula-patches/0ae366e6/patch3.diff"
sha256 "89fa3c95c329ec326e2e76493471a7a974c673792725059ef121e6f9efb05bf4"
end
end
resource "resource2" do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
patch do
url "https://raw.githubusercontent.com/Homebrew/formula-patches/0ae366e6/patch1.diff"
sha256 "89fa3c95c329ec326e2e76493471a7a974c673792725059ef121e6f9efb05bf4"
end
patch do
url "https://raw.githubusercontent.com/Homebrew/formula-patches/0ae366e6/patch2.diff"
sha256 "89fa3c95c329ec326e2e76493471a7a974c673792725059ef121e6f9efb05bf4"
end
end
on_linux do
depends_on "readline"
end
def install
end
end
RUBY
end
end
context "on_macos_block" do
it "correctly uses as single on_macos block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_macos do
depends_on "readline"
end
def install
end
end
RUBY
end
end
context "on_linux_block" do
it "correctly uses as single on_linux block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
on_linux do
depends_on "readline"
end
def install
end
end
RUBY
end
end
it "there can only be one on_macos block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_macos do
depends_on "readline"
end
on_macos do
^^^^^^^^^^^ there can only be one `on_macos` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "there can only be one on_linux block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_linux do
depends_on "readline"
end
on_linux do
^^^^^^^^^^^ there can only be one `on_linux` block in a formula.
depends_on "foo"
end
end
RUBY
end
it "the on_macos block can only contain depends_on, patch and resource nodes" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_macos do
^^^^^^^^^^^ `on_macos` can only include `depends_on`, `patch` and `resource` nodes.
depends_on "readline"
uses_from_macos "ncurses"
end
end
RUBY
end
it "the on_linux block can only contain depends_on, patch and resource nodes" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
on_linux do
^^^^^^^^^^^ `on_linux` can only include `depends_on`, `patch` and `resource` nodes.
depends_on "readline"
uses_from_macos "ncurses"
end
end
RUBY
end
context "resource" do
it "correctly uses an on_macos and on_linux block" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
homepage "https://brew.sh"
resource do
on_macos do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "there are two on_macos blocks, which is not allowed" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ there can only be one `on_macos` block in a resource block.
on_macos do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_macos do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "there are two on_linux blocks, which is not allowed" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ there can only be one `on_linux` block in a resource block.
on_linux do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "there is a on_macos block but no on_linux block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ you need to define an `on_linux` block within your resource block.
on_macos do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "there is a on_linux block but no on_macos block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ you need to define an `on_macos` block within your resource block.
on_linux do
url "https://brew.sh/resource1.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "the content of the on_macos block is wrong in a resource block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ only an url and a sha256 (in the right order) are allowed in a `on_macos` block within a resource block.
on_macos do
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
url "https://brew.sh/resource2.tar.gz"
end
on_linux do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
end
end
RUBY
end
it "the content of the on_linux block is wrong in a resource block" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
resource do
^^^^^^^^^^^ only an url and a sha256 (in the right order) are allowed in a `on_linux` block within a resource block.
on_macos do
url "https://brew.sh/resource2.tar.gz"
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
end
on_linux do
sha256 "586372eb92059873e29eba4f9dec8381541b4d3834660707faf8ba59146dfc35"
url "https://brew.sh/resource2.tar.gz"
end
end
end
RUBY
end
include_examples "formulae exist", described_class::COMPONENT_WHITELIST
end
end