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

Add node pattern methods to handle dependency audits in a better way #3012

Merged
merged 1 commit into from
Aug 8, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions Library/Homebrew/rubocops/extend/formula_cop.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "parser/current"
require_relative "../../extend/string"

module RuboCop
module Cop
Expand Down Expand Up @@ -138,17 +139,14 @@ def depends_on_name_type?(node, name = nil, type = :required)

case type
when :required
type_match = !node.method_args.nil? &&
(node.method_args.first.str_type? || node.method_args.first.sym_type?)
type_match = required_dependency?(node)
if type_match && !name_match
name_match = node_equals?(node.method_args.first, name)
name_match = required_dependency_name?(node, name)
end
when :build, :optional, :recommended, :run
type_match = !node.method_args.nil? &&
node.method_args.first.hash_type? &&
node.method_args.first.values.first.children.first == type
type_match = dependency_type_hash_match?(node, type)
if type_match && !name_match
name_match = node_equals?(node.method_args.first.keys.first.children.first, name)
name_match = dependency_name_hash_match?(node, name)
end
else
type_match = false
Expand All @@ -161,6 +159,22 @@ def depends_on_name_type?(node, name = nil, type = :required)
type_match && name_match
end

def_node_search :required_dependency?, <<-EOS.undent
(send nil :depends_on ({str sym} _))
EOS

def_node_search :required_dependency_name?, <<-EOS.undent
(send nil :depends_on ({str sym} %1))
EOS

def_node_search :dependency_type_hash_match?, <<-EOS.undent
(hash (pair ({str sym} _) ({str sym} %1)))
EOS

def_node_search :dependency_name_hash_match?, <<-EOS.undent
(hash (pair ({str sym} %1) ({str sym} _)))
EOS

# To compare node with appropriate Ruby variable
def node_equals?(node, var)
node == Parser::CurrentRuby.parse(var.inspect)
Expand Down
48 changes: 48 additions & 0 deletions Library/Homebrew/test/rubocops/text_cop_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,54 @@
subject(:cop) { described_class.new }

context "When auditing formula text" do
it "with both openssl and libressl optional dependencies" do
source = <<-EOS.undent
class Foo < Formula
url "http://example.com/foo-1.0.tgz"
homepage "http://example.com"

depends_on "openssl"
depends_on "libressl" => :optional
end
EOS

expected_offenses = [{ message: "Formulae should not depend on both OpenSSL and LibreSSL (even optionally).",
severity: :convention,
line: 6,
column: 2,
source: source }]

inspect_source(cop, source)

expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end

it "with both openssl and libressl dependencies" do
source = <<-EOS.undent
class Foo < Formula
url "http://example.com/foo-1.0.tgz"
homepage "http://example.com"

depends_on "openssl"
depends_on "libressl"
end
EOS

expected_offenses = [{ message: "Formulae should not depend on both OpenSSL and LibreSSL (even optionally).",
severity: :convention,
line: 6,
column: 2,
source: source }]

inspect_source(cop, source)

expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end

it "When xcodebuild is called without SYMROOT" do
source = <<-EOS.undent
class Foo < Formula
Expand Down