Merge pull request #2901 from GauthamGoli/audit_option_rubocop_2

audit: Port audit_options strict rules to rubocop and add tests

4 files changed, 104 insertions, 17 deletions

diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml
index db7a05d87..bb437f15c 100644
--- a/Library/.rubocop.yml
+++ b/Library/.rubocop.yml
@@ -24,6 +24,9 @@ FormulaAudit/Conflicts:
 FormulaAudit/Options:
   Enabled: true
 
+FormulaAuditStrict/Options:
+  Enabled: true
+
 FormulaAuditStrict/BottleBlock:
   Enabled: true
 
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index 6a7292ea7..e144fc35c 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -554,23 +554,6 @@ class FormulaAuditor
   end
 
   def audit_options
-    formula.options.each do |o|
-      next unless @strict
-
-      if o.name == "universal"
-        problem "macOS has been 64-bit only since 10.6 so universal options are deprecated."
-      end
-
-      if o.name !~ /with(out)?-/ && o.name != "c++11" && o.name != "universal"
-        problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`."
-      end
-
-      next unless o.name =~ /^with(out)?-(?:checks?|tests)$/
-      unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
-        problem "Use '--with#{Regexp.last_match(1)}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
-      end
-    end
-
     return unless @new_formula
     return if formula.deprecated_options.empty?
     return if formula.versioned_formula?
diff --git a/Library/Homebrew/rubocops/options_cop.rb b/Library/Homebrew/rubocops/options_cop.rb
index 83d853f10..eb2a837be 100644
--- a/Library/Homebrew/rubocops/options_cop.rb
+++ b/Library/Homebrew/rubocops/options_cop.rb
@@ -16,5 +16,32 @@ module RuboCop
         end
       end
     end
+
+    module FormulaAuditStrict
+      class Options < FormulaCop
+        DEPRECATION_MSG = "macOS has been 64-bit only since 10.6 so universal options are deprecated.".freeze
+
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          option_call_nodes = find_every_method_call_by_name(body_node, :option)
+          option_call_nodes.each do |option_call|
+            offending_node(option_call)
+            option = string_content(parameters(option_call).first)
+            problem DEPRECATION_MSG if option == "universal"
+
+            if option !~ /with(out)?-/ &&
+               option != "cxx11" &&
+               option != "universal"
+              problem "Options should begin with with/without."\
+                      " Migrate '--#{option}' with `deprecated_option`."
+            end
+
+            next unless option =~ /^with(out)?-(?:checks?|tests)$/
+            next if depends_on?("check", :optional, :recommended)
+            problem "Use '--with#{Regexp.last_match(1)}-test' instead of '--#{option}'."\
+                    " Migrate '--#{option}' with `deprecated_option`."
+          end
+        end
+      end
+    end
   end
 end
diff --git a/Library/Homebrew/test/rubocops/options_cop_spec.rb b/Library/Homebrew/test/rubocops/options_cop_spec.rb
index 0ed3a9741..161f2a87a 100644
--- a/Library/Homebrew/test/rubocops/options_cop_spec.rb
+++ b/Library/Homebrew/test/rubocops/options_cop_spec.rb
@@ -29,3 +29,77 @@ describe RuboCop::Cop::FormulaAudit::Options do
     end
   end
 end
+
+describe RuboCop::Cop::FormulaAuditStrict::Options do
+  subject(:cop) { described_class.new }
+
+  context "When auditing options strictly" do
+    it "with universal" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          option :universal
+        end
+      EOS
+
+      expected_offenses = [{  message: described_class::DEPRECATION_MSG,
+                              severity: :convention,
+                              line: 3,
+                              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 deprecated options" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          option :cxx11
+          option "examples", "with-examples"
+        end
+      EOS
+
+      MSG_1 = "Options should begin with with/without."\
+              " Migrate '--examples' with `deprecated_option`.".freeze
+      expected_offenses = [{  message: MSG_1,
+                              severity: :convention,
+                              line: 4,
+                              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 misc deprecated options" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          option "without-check"
+        end
+      EOS
+
+      MSG_2 = "Use '--without-test' instead of '--without-check'."\
+              " Migrate '--without-check' with `deprecated_option`.".freeze
+      expected_offenses = [{  message: MSG_2,
+                              severity: :convention,
+                              line: 3,
+                              column: 2,
+                              source: source }]
+
+      inspect_source(cop, source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+  end
+end