From a3219ca09cded33f80c2039c29f191b7369e2450 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Sun, 6 Aug 2017 14:48:39 +0530 Subject: [PATCH] Add node pattern methods to handle dependency audits in a better way --- .../Homebrew/rubocops/extend/formula_cop.rb | 28 ++++++++--- .../Homebrew/test/rubocops/text_cop_spec.rb | 48 +++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index 4be0c0fe30..8626640106 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -1,4 +1,5 @@ require "parser/current" +require_relative "../../extend/string" module RuboCop module Cop @@ -138,17 +139,14 @@ module RuboCop 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 @@ -161,6 +159,22 @@ module RuboCop 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) diff --git a/Library/Homebrew/test/rubocops/text_cop_spec.rb b/Library/Homebrew/test/rubocops/text_cop_spec.rb index b218e9c256..ec13c40415 100644 --- a/Library/Homebrew/test/rubocops/text_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/text_cop_spec.rb @@ -7,6 +7,54 @@ describe RuboCop::Cop::FormulaAudit::Text do 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