Merge pull request #2664 from GauthamGoli/audit_caveats_rubocop

audit: Port audit_caveats method to rubocop and add tests

7 files changed, 80 insertions, 23 deletions

diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml
index 8fd64cb1a..293e88749 100644
--- a/Library/.rubocop.yml
+++ b/Library/.rubocop.yml
@@ -11,6 +11,9 @@ require: ./Homebrew/rubocops.rb
 FormulaAudit/Text:
   Enabled: true
 
+FormulaAudit/Caveats:
+  Enabled: true
+
 FormulaAuditStrict/BottleBlock:
   Enabled: true
 
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index 4ceff0f3d..bf7d2fc79 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -1148,11 +1148,6 @@ class FormulaAuditor
     problem "Use pkgshare instead of (share#{$1}\"#{formula.name}\")"
   end
 
-  def audit_caveats
-    return unless formula.caveats.to_s.include?("setuid")
-    problem "Don't recommend setuid in the caveats, suggest sudo instead."
-  end
-
   def audit_reverse_migration
     # Only enforce for new formula being re-added to core and official taps
     return unless @strict
diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb
index 8627d2c04..c4a38cdb7 100644
--- a/Library/Homebrew/rubocops.rb
+++ b/Library/Homebrew/rubocops.rb
@@ -4,3 +4,4 @@ require_relative "./rubocops/components_order_cop"
 require_relative "./rubocops/components_redundancy_cop"
 require_relative "./rubocops/homepage_cop"
 require_relative "./rubocops/text_cop"
+require_relative "./rubocops/caveats_cop"
diff --git a/Library/Homebrew/rubocops/caveats_cop.rb b/Library/Homebrew/rubocops/caveats_cop.rb
new file mode 100644
index 000000000..3935d5638
--- /dev/null
+++ b/Library/Homebrew/rubocops/caveats_cop.rb
@@ -0,0 +1,16 @@
+require_relative "./extend/formula_cop"
+
+module RuboCop
+  module Cop
+    module FormulaAudit
+      class Caveats < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, _body_node)
+          caveats_strings.each do |n|
+            next unless regex_match_group(n, /\bsetuid\b/i)
+            problem "Don't recommend setuid in the caveats, suggest sudo instead."
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
index d9940a037..75a3e72d5 100644
--- a/Library/Homebrew/rubocops/extend/formula_cop.rb
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -24,7 +24,11 @@ module RuboCop
         return unless match_object
         node_begin_pos = start_column(node)
         line_begin_pos = line_start_column(node)
-        @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1
+        if node_begin_pos == line_begin_pos
+          @column = node_begin_pos + match_object.begin(0) - line_begin_pos
+        else
+          @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1
+        end
         @length = match_object.to_s.length
         @line_no = line_number(node)
         @source_buf = source_buffer(node)
@@ -33,6 +37,12 @@ module RuboCop
         match_object
       end
 
+      # Returns all string nodes among the descendants of given node
+      def find_strings(node)
+        return [] if node.nil?
+        node.each_descendant(:str)
+      end
+
       # Returns method_node matching method_name
       def find_node_method_by_name(node, method_name)
         return if node.nil?
@@ -233,6 +243,11 @@ module RuboCop
         true
       end
 
+      # Return all the caveats' string nodes in an array
+      def caveats_strings
+        find_strings(find_method_def(@body, :caveats))
+      end
+
       # Returns the array of arguments of the method_node
       def parameters(method_node)
         return unless method_node.send_type?
@@ -308,6 +323,8 @@ module RuboCop
           return node.each_child_node(:str).map(&:str_content).join("") if node.type == :dstr
         when :const
           return node.const_name if node.type == :const
+        else
+          ""
         end
       end
 
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb
index b90a21b55..25a179342 100644
--- a/Library/Homebrew/test/dev-cmd/audit_spec.rb
+++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb
@@ -305,23 +305,6 @@ describe FormulaAuditor do
     end
   end
 
-  specify "#audit_caveats" do
-    fa = formula_auditor "foo", <<-EOS.undent
-      class Foo < Formula
-        homepage "http://example.com/foo"
-        url "http://example.com/foo-1.0.tgz"
-
-        def caveats
-          "setuid"
-        end
-      end
-    EOS
-
-    fa.audit_caveats
-    expect(fa.problems)
-      .to eq(["Don't recommend setuid in the caveats, suggest sudo instead."])
-  end
-
   describe "#audit_keg_only_style" do
     specify "keg_only_needs_downcasing" do
       fa = formula_auditor "foo", <<-EOS.undent, strict: true
diff --git a/Library/Homebrew/test/rubocops/caveats_cop_spec.rb b/Library/Homebrew/test/rubocops/caveats_cop_spec.rb
new file mode 100644
index 000000000..d44808a5d
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/caveats_cop_spec.rb
@@ -0,0 +1,42 @@
+require "rubocop"
+require "rubocop/rspec/support"
+require_relative "../../extend/string"
+require_relative "../../rubocops/caveats_cop"
+
+describe RuboCop::Cop::FormulaAudit::Caveats do
+  subject(:cop) { described_class.new }
+
+  context "When auditing caveats" do
+    it "When there is setuid mentioned in caveats" do
+      source = <<-EOS.undent
+      class Foo < Formula
+        homepage "http://example.com/foo"
+        url "http://example.com/foo-1.0.tgz"
+
+        def caveats
+          "setuid"
+        end
+      end
+      EOS
+
+      expected_offenses = [{  message: "Don't recommend setuid in the caveats, suggest sudo instead.",
+                              severity: :convention,
+                              line: 6,
+                              column: 5,
+                              source: source }]
+
+      inspect_source(cop, source)
+
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    def expect_offense(expected, actual)
+      expect(actual.message).to eq(expected[:message])
+      expect(actual.severity).to eq(expected[:severity])
+      expect(actual.line).to eq(expected[:line])
+      expect(actual.column).to eq(expected[:column])
+    end
+  end
+end