Merge pull request #2982 from GauthamGoli/audit_class_rubocop_port

audit: Port audit_class to rubocop, add tests and autocorrect

8 files changed, 138 insertions, 82 deletions

diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index d089f308d..884861c87 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -54,6 +54,7 @@ module Homebrew
 
   def audit
     Homebrew.inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D"
+    Homebrew.auditing = true
 
     formula_count = 0
     problem_count = 0
@@ -381,21 +382,6 @@ class FormulaAuditor
     end
   end
 
-  def audit_class
-    if @strict
-      unless formula.test_defined?
-        problem "A `test do` test block should be added"
-      end
-    end
-
-    classes = %w[GithubGistFormula ScriptFileFormula AmazonWebServicesFormula]
-    klass = classes.find do |c|
-      Object.const_defined?(c) && formula.class < Object.const_get(c)
-    end
-
-    problem "#{klass} is deprecated, use Formula instead" if klass
-  end
-
   # core aliases + tap alias names + tap alias full name
   @@aliases ||= Formula.aliases + Formula.tap_aliases
 
diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb
index 9f79781b4..32b5377a0 100644
--- a/Library/Homebrew/global.rb
+++ b/Library/Homebrew/global.rb
@@ -45,11 +45,15 @@ module Homebrew
       @failed == true
     end
 
-    attr_writer :raise_deprecation_exceptions
+    attr_writer :raise_deprecation_exceptions, :auditing
 
     def raise_deprecation_exceptions?
       @raise_deprecation_exceptions == true
     end
+
+    def auditing?
+      @auditing == true
+    end
   end
 end
 
diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb
index b1144e075..8dc49bb45 100644
--- a/Library/Homebrew/rubocops.rb
+++ b/Library/Homebrew/rubocops.rb
@@ -11,3 +11,4 @@ require_relative "./rubocops/conflicts_cop"
 require_relative "./rubocops/options_cop"
 require_relative "./rubocops/urls_cop"
 require_relative "./rubocops/lines_cop"
+require_relative "./rubocops/class_cop"
diff --git a/Library/Homebrew/rubocops/class_cop.rb b/Library/Homebrew/rubocops/class_cop.rb
new file mode 100644
index 000000000..dad81abfc
--- /dev/null
+++ b/Library/Homebrew/rubocops/class_cop.rb
@@ -0,0 +1,41 @@
+require_relative "./extend/formula_cop"
+
+module RuboCop
+  module Cop
+    module FormulaAudit
+      class ClassName < FormulaCop
+        DEPRECATED_CLASSES = %w[
+          GithubGistFormula
+          ScriptFileFormula
+          AmazonWebServicesFormula
+        ].freeze
+
+        def audit_formula(_node, _class_node, parent_class_node, _body_node)
+          parent_class = class_name(parent_class_node)
+          return unless DEPRECATED_CLASSES.include?(parent_class)
+          problem "#{parent_class} is deprecated, use Formula instead"
+        end
+
+        private
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrector.replace(node.source_range, "Formula")
+          end
+        end
+      end
+    end
+
+    module FormulaAuditStrict
+      # - `test do ..end` should be defined in the formula
+      class Test < FormulaCop
+        MSG = "A `test do` test block should be added".freeze
+
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          return if find_block(body_node, :test)
+          problem MSG
+        end
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
index 7844f7bf2..59ad1aafb 100644
--- a/Library/Homebrew/rubocops/extend/formula_cop.rb
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -422,7 +422,14 @@ module RuboCop
 
       def formula_class?(node)
         _, class_node, = *node
-        class_node && string_content(class_node) == "Formula"
+        class_names = %w[
+          Formula
+          GithubGistFormula
+          ScriptFileFormula
+          AmazonWebServicesFormula
+        ]
+
+        class_node && class_names.include?(string_content(class_node))
       end
 
       def file_path_allowed?(file_path)
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb
index 037865fdf..3e99bd06b 100644
--- a/Library/Homebrew/test/dev-cmd/audit_spec.rb
+++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb
@@ -150,70 +150,6 @@ describe FormulaAuditor do
     end
   end
 
-  describe "#audit_class" do
-    specify "missing test" do
-      fa = formula_auditor "foo", <<-EOS.undent
-        class Foo < Formula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_class
-      expect(fa.problems).to eq([])
-
-      fa = formula_auditor "foo", <<-EOS.undent, strict: true
-        class Foo < Formula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_class
-      expect(fa.problems).to eq(["A `test do` test block should be added"])
-    end
-
-    specify "GithubGistFormula", :needs_compat do
-      ENV.delete("HOMEBREW_DEVELOPER")
-
-      fa = formula_auditor "foo", <<-EOS.undent
-        class Foo < GithubGistFormula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_class
-      expect(fa.problems)
-        .to eq(["GithubGistFormula is deprecated, use Formula instead"])
-    end
-
-    specify "ScriptFileFormula", :needs_compat do
-      ENV.delete("HOMEBREW_DEVELOPER")
-
-      fa = formula_auditor "foo", <<-EOS.undent
-        class Foo < ScriptFileFormula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_class
-      expect(fa.problems)
-        .to eq(["ScriptFileFormula is deprecated, use Formula instead"])
-    end
-
-    specify "AmazonWebServicesFormula", :needs_compat do
-      ENV.delete("HOMEBREW_DEVELOPER")
-
-      fa = formula_auditor "foo", <<-EOS.undent
-        class Foo < AmazonWebServicesFormula
-          url "http://example.com/foo-1.0.tgz"
-        end
-      EOS
-
-      fa.audit_class
-      expect(fa.problems)
-        .to eq(["AmazonWebServicesFormula is deprecated, use Formula instead"])
-    end
-  end
-
   describe "#line_problems" do
     specify "pkgshare" do
       fa = formula_auditor "foo", <<-EOS.undent, strict: true
diff --git a/Library/Homebrew/test/rubocops/class_cop_spec.rb b/Library/Homebrew/test/rubocops/class_cop_spec.rb
new file mode 100644
index 000000000..676dd4f6e
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/class_cop_spec.rb
@@ -0,0 +1,81 @@
+require "rubocop"
+require "rubocop/rspec/support"
+require_relative "../../extend/string"
+require_relative "../../rubocops/class_cop"
+
+describe RuboCop::Cop::FormulaAudit::ClassName do
+  subject(:cop) { described_class.new }
+
+  context "When auditing formula" do
+    it "with deprecated inheritance" do
+      formulas = [{
+        "class" => "GithubGistFormula",
+      }, {
+        "class" => "ScriptFileFormula",
+      }, {
+        "class" => "AmazonWebServicesFormula",
+      }]
+
+      formulas.each do |formula|
+        source = <<-EOS.undent
+        class Foo < #{formula["class"]}
+          url 'http://example.com/foo-1.0.tgz'
+        end
+        EOS
+
+        expected_offenses = [{  message: "#{formula["class"]} is deprecated, use Formula instead",
+                                severity: :convention,
+                                line: 1,
+                                column: 12,
+                                source: source }]
+
+        inspect_source(cop, source)
+
+        expected_offenses.zip(cop.offenses.reverse).each do |expected, actual|
+          expect_offense(expected, actual)
+        end
+      end
+    end
+
+    it "with deprecated inheritance and autocorrect" do
+      source = <<-EOS.undent
+        class Foo < AmazonWebServicesFormula
+          url 'http://example.com/foo-1.0.tgz'
+        end
+      EOS
+      corrected_source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+        end
+      EOS
+
+      new_source = autocorrect_source(cop, source)
+      expect(new_source).to eq(corrected_source)
+    end
+  end
+end
+
+describe RuboCop::Cop::FormulaAuditStrict::Test do
+  subject(:cop) { described_class.new }
+
+  context "When auditing formula" do
+    it "without a test block" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+        end
+      EOS
+      expected_offenses = [{  message: described_class::MSG,
+                              severity: :convention,
+                              line: 1,
+                              column: 0,
+                              source: source }]
+
+      inspect_source(cop, source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index e3137ac49..07e339576 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -102,7 +102,7 @@ def odeprecated(method, replacement = nil, disable: false, disable_on: nil, call
   if ARGV.homebrew_developer? || disable ||
      Homebrew.raise_deprecation_exceptions?
     raise MethodDeprecatedError, message
-  else
+  elsif !Homebrew.auditing?
     opoo "#{message}\n"
   end
 end