From 19f693d25ba514a6b604128474763e83de55b548 Mon Sep 17 00:00:00 2001
From: Gautham Goli <goligautham@gmail.com>
Date: Thu, 2 Mar 2017 01:10:36 +0530
Subject: [PATCH 1/5] Port audit_desc rules to cop

---
 Library/Homebrew/dev-cmd/audit.rb             | 34 ----------
 Library/Homebrew/rubocops.rb                  |  1 +
 Library/Homebrew/rubocops/formula_desc_cop.rb | 67 +++++++++++++++++++
 Library/Homebrew/test/dev-cmd/audit_spec.rb   | 30 ---------
 4 files changed, 68 insertions(+), 64 deletions(-)
 create mode 100644 Library/Homebrew/rubocops/formula_desc_cop.rb

diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index a8c18f7b63..b26f830721 100644
--- a/Library/Homebrew/dev-cmd/audit.rb
+++ b/Library/Homebrew/dev-cmd/audit.rb
@@ -569,39 +569,6 @@ class FormulaAuditor
     problem "New formulae should not use `deprecated_option`."
   end
 
-  def audit_desc
-    # For now, only check the description when using `--strict`
-    return unless @strict
-
-    desc = formula.desc
-
-    unless desc && !desc.empty?
-      problem "Formula should have a desc (Description)."
-      return
-    end
-
-    # Make sure the formula name plus description is no longer than 80 characters
-    # Note full_name includes the name of the tap, while name does not
-    linelength = "#{formula.name}: #{desc}".length
-    if linelength > 80
-      problem <<-EOS.undent
-        Description is too long. \"name: desc\" should be less than 80 characters.
-        Length is calculated as #{formula.name} + desc. (currently #{linelength})
-      EOS
-    end
-
-    if desc =~ /([Cc]ommand ?line)/
-      problem "Description should use \"command-line\" instead of \"#{$1}\""
-    end
-
-    if desc =~ /^([Aa]n?)\s/
-      problem "Description shouldn't start with an indefinite article (#{$1})"
-    end
-
-    return unless desc.downcase.start_with? "#{formula.name} "
-    problem "Description shouldn't include the formula name"
-  end
-
   def audit_homepage
     homepage = formula.homepage
 
@@ -1254,7 +1221,6 @@ class FormulaAuditor
     audit_class
     audit_specs
     audit_revision_and_version_scheme
-    audit_desc
     audit_homepage
     audit_bottle_spec
     audit_github_repository
diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb
index 1a28dd2134..3625f20049 100644
--- a/Library/Homebrew/rubocops.rb
+++ b/Library/Homebrew/rubocops.rb
@@ -1 +1,2 @@
 require_relative "./rubocops/bottle_block_cop"
+require_relative "./rubocops/formula_desc_cop"
diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb
new file mode 100644
index 0000000000..8728c6fc02
--- /dev/null
+++ b/Library/Homebrew/rubocops/formula_desc_cop.rb
@@ -0,0 +1,67 @@
+module RuboCop
+  module Cop
+    module Homebrew
+      class FormulaDesc < Cop
+        def on_class(node)
+          class_node, parent_class_node, body = *node
+          formula_name = class_node.const_name
+          return unless parent_class_node && parent_class_node.const_name == "Formula" && body
+          check(node, body, formula_name)
+        end
+
+        private
+
+        def check(node, body, formula_name)
+          body.each_child_node(:send) do |call_node|
+            _receiver, call_name, args = *call_node
+            next unless call_name == :desc && !args.children[0].empty?
+            description = args.children[0]
+
+            source_buffer = call_node.source_range.source_buffer
+            line_number = call_node.loc.line
+            line_begin_pos = call_node.source_range.source_buffer.line_range(call_node.loc.line).begin_pos
+            desc_begin_pos = call_node.children[2].source_range.begin_pos
+
+            linelength = "#{formula_name}: #{description}".length
+            if linelength > 80
+              column = desc_begin_pos - line_begin_pos
+              length = call_node.children[2].source_range.size
+              sourcerange = source_range(source_buffer, line_number, column, length)
+              message = <<-EOS.strip_indent
+                        Description is too long. \"name: desc\" should be less than 80 characters.
+                        Length is calculated as #{formula_name} + desc. (currently #{linelength})
+                        EOS
+              add_offense(call_node, sourcerange, message)
+            end
+
+            match_object = description.match(/([Cc]ommand ?line)/)
+            if match_object
+              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
+              length = match_object.to_s.length
+              sourcerange = source_range(source_buffer, line_number, column, length)
+              add_offense(call_node, sourcerange, "Description should use \"command-line\" instead of \"#{match_object}\"")
+            end
+
+            match_object = description.match(/^([Aa]n?)\s/)
+            if match_object
+              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
+              length = match_object.to_s.length
+              sourcerange = source_range(source_buffer, line_number, column, length)
+              add_offense(call_node, sourcerange, "Description shouldn't start with an indefinite article (#{match_object})")
+            end
+
+            match_object = description.match(/^#{formula_name}/i)
+            if match_object
+              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
+              length = match_object.to_s.length
+              sourcerange = source_range(source_buffer, line_number, column, length)
+              add_offense(call_node, sourcerange, "Description shouldn't include the formula name")
+            end
+            return
+          end
+          add_offense(node, node.source_range, "Formula should have a desc (Description).")
+        end
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb
index ec1a34fb4a..a6bb22837d 100644
--- a/Library/Homebrew/test/dev-cmd/audit_spec.rb
+++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb
@@ -344,10 +344,6 @@ describe FormulaAuditor do
         end
       EOS
 
-      fa.audit_desc
-      expect(fa.problems.shift)
-        .to eq("Description shouldn't include the formula name")
-
       fa.audit_line 'ohai "#{share}/foolibc++"', 3
       expect(fa.problems.shift)
         .to eq("Use \#{pkgshare} instead of \#{share}/foolibc++")
@@ -413,32 +409,6 @@ describe FormulaAuditor do
       .to eq(["Don't recommend setuid in the caveats, suggest sudo instead."])
   end
 
-  specify "#audit_desc" do
-    formula_descriptions = [
-      { name: "foo", desc: nil,
-        problem: "Formula should have a desc" },
-      { name: "bar", desc: "bar" * 30,
-        problem: "Description is too long" },
-      { name: "baz", desc: "Baz commandline tool",
-        problem: "Description should use \"command-line\"" },
-      { name: "qux", desc: "A tool called Qux",
-        problem: "Description shouldn't start with an indefinite article" },
-    ]
-
-    formula_descriptions.each do |formula|
-      content = <<-EOS.undent
-        class #{Formulary.class_s(formula[:name])} < Formula
-          url "http://example.com/#{formula[:name]}-1.0.tgz"
-          desc "#{formula[:desc]}"
-        end
-      EOS
-
-      fa = formula_auditor formula[:name], content, strict: true
-      fa.audit_desc
-      expect(fa.problems.first).to match(formula[:problem])
-    end
-  end
-
   describe "#audit_homepage" do
     specify "homepage URLs" do
       fa = formula_auditor "foo", <<-EOS.undent, online: true

From d32978b859d18c68bad20614287f3485c52ad98f Mon Sep 17 00:00:00 2001
From: Gautham Goli <goligautham@gmail.com>
Date: Thu, 2 Mar 2017 20:26:29 +0530
Subject: [PATCH 2/5] Create FormulaCop base class to reuse helper functions in
 custom cops

---
 Library/.rubocop.yml                          | 10 +++++
 Library/Homebrew/rubocops/bottle_block_cop.rb | 27 +++++++----
 .../Homebrew/rubocops/extend/formula_cop.rb   | 22 +++++++++
 Library/Homebrew/rubocops/formula_desc_cop.rb | 45 ++++++++++++-------
 4 files changed, 79 insertions(+), 25 deletions(-)
 create mode 100644 Library/Homebrew/rubocops/extend/formula_cop.rb

diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml
index 314467ef0e..41219f2cd3 100644
--- a/Library/.rubocop.yml
+++ b/Library/.rubocop.yml
@@ -8,7 +8,17 @@ AllCops:
 
 require: ./Homebrew/rubocops.rb
 
+Homebrew/FormulaCop:
+  Enabled: false
+
 Homebrew/CorrectBottleBlock:
+  Include:
+    - '**/Taps/homebrew/**/*.rb'
+  Enabled: true
+
+Homebrew/FormulaDesc:
+  Include:
+    - '**/Taps/homebrew/**/*.rb'
   Enabled: true
 
 Metrics/AbcSize:
diff --git a/Library/Homebrew/rubocops/bottle_block_cop.rb b/Library/Homebrew/rubocops/bottle_block_cop.rb
index 55eb551524..733c492e88 100644
--- a/Library/Homebrew/rubocops/bottle_block_cop.rb
+++ b/Library/Homebrew/rubocops/bottle_block_cop.rb
@@ -1,20 +1,31 @@
+require_relative "./extend/formula_cop"
+
 module RuboCop
   module Cop
     module Homebrew
-      class CorrectBottleBlock < Cop
+      # This cop audits `bottle` block in Formulae
+      #
+      # - `rebuild` should be used instead of `revision` in `bottle` block
+
+      class CorrectBottleBlock < FormulaCop
         MSG = "Use rebuild instead of revision in bottle block".freeze
 
-        def on_block(node)
-          return if block_length(node).zero?
-          method, _args, body = *node
-          _keyword, method_name = *method
-
-          return unless method_name == :bottle
-          check_revision?(body)
+        def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node)
+          check(formula_class_body_node)
         end
 
         private
 
+        def check(formula_class_body_node)
+          formula_class_body_node.each_child_node(:block) do |block_node|
+            next if block_length(block_node).zero?
+            method, _args, block_body = *block_node
+            _keyword, method_name = *method
+            next unless method_name == :bottle
+            check_revision?(block_body)
+          end
+        end
+
         def autocorrect(node)
           lambda do |corrector|
             correction = node.source.sub("revision", "rebuild")
diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
new file mode 100644
index 0000000000..cf5d513c93
--- /dev/null
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -0,0 +1,22 @@
+module RuboCop
+  module Cop
+    module Homebrew
+      class FormulaCop < Cop
+        @registry = Cop.registry
+
+        def on_class(node)
+          # This method is called by RuboCop and is the main entry point
+          class_node, parent_class_node, body = *node
+          return unless a_formula_class?(parent_class_node)
+          audit_formula(node, class_node, parent_class_node, body)
+        end
+
+        private
+
+        def a_formula_class?(parent_class_node)
+          parent_class_node && parent_class_node.const_name == "Formula"
+        end
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb
index 8728c6fc02..dcd928d286 100644
--- a/Library/Homebrew/rubocops/formula_desc_cop.rb
+++ b/Library/Homebrew/rubocops/formula_desc_cop.rb
@@ -1,12 +1,20 @@
+require_relative "./extend/formula_cop"
+require_relative "../extend/string"
+
 module RuboCop
   module Cop
     module Homebrew
-      class FormulaDesc < Cop
-        def on_class(node)
-          class_node, parent_class_node, body = *node
-          formula_name = class_node.const_name
-          return unless parent_class_node && parent_class_node.const_name == "Formula" && body
-          check(node, body, formula_name)
+      # This cop audits `desc` in Formulae
+      #
+      # - Checks for existence of `desc`
+      # - Checks if size of `desc` > 80
+      # - Checks if `desc` begins with an article
+      # - Checks for correct usage of `command-line` in `desc`
+      # - Checks if `desc` contains the formula name
+
+      class FormulaDesc < FormulaCop
+        def audit_formula(node, class_node, _parent_class_node, body)
+          check(node, body, class_node.const_name)
         end
 
         private
@@ -14,7 +22,7 @@ module RuboCop
         def check(node, body, formula_name)
           body.each_child_node(:send) do |call_node|
             _receiver, call_name, args = *call_node
-            next unless call_name == :desc && !args.children[0].empty?
+            next if call_name != :desc || args.children[0].empty?
             description = args.children[0]
 
             source_buffer = call_node.source_range.source_buffer
@@ -27,27 +35,29 @@ module RuboCop
               column = desc_begin_pos - line_begin_pos
               length = call_node.children[2].source_range.size
               sourcerange = source_range(source_buffer, line_number, column, length)
-              message = <<-EOS.strip_indent
-                        Description is too long. \"name: desc\" should be less than 80 characters.
-                        Length is calculated as #{formula_name} + desc. (currently #{linelength})
-                        EOS
+              message = <<-EOS.undent
+                Description is too long. "name: desc" should be less than 80 characters.
+                Length is calculated as #{formula_name} + desc. (currently #{linelength})
+              EOS
               add_offense(call_node, sourcerange, message)
             end
 
-            match_object = description.match(/([Cc]ommand ?line)/)
+            match_object = description.match(/(command ?line)/i)
             if match_object
               column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
               length = match_object.to_s.length
               sourcerange = source_range(source_buffer, line_number, column, length)
-              add_offense(call_node, sourcerange, "Description should use \"command-line\" instead of \"#{match_object}\"")
+              message = "Description should use \"command-line\" instead of \"#{match_object}\""
+              add_offense(call_node, sourcerange, message)
             end
 
-            match_object = description.match(/^([Aa]n?)\s/)
+            match_object = description.match(/^(an?)\s/i)
             if match_object
               column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
               length = match_object.to_s.length
               sourcerange = source_range(source_buffer, line_number, column, length)
-              add_offense(call_node, sourcerange, "Description shouldn't start with an indefinite article (#{match_object})")
+              message = "Description shouldn't start with an indefinite article (#{match_object})"
+              add_offense(call_node, sourcerange, message)
             end
 
             match_object = description.match(/^#{formula_name}/i)
@@ -55,9 +65,10 @@ module RuboCop
               column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
               length = match_object.to_s.length
               sourcerange = source_range(source_buffer, line_number, column, length)
-              add_offense(call_node, sourcerange, "Description shouldn't include the formula name")
+              message = "Description shouldn't include the formula name"
+              add_offense(call_node, sourcerange, message)
             end
-            return
+            return nil
           end
           add_offense(node, node.source_range, "Formula should have a desc (Description).")
         end

From 4e57f4279aaf32855c6cc0ebc96bf9352feb1e17 Mon Sep 17 00:00:00 2001
From: Gautham Goli <goligautham@gmail.com>
Date: Wed, 8 Mar 2017 13:51:35 +0530
Subject: [PATCH 3/5] Add RSpec tests for bottle_block and formula_desc cops

---
 Library/Homebrew/test/.bundle/config          |   2 +-
 Library/Homebrew/test/Gemfile                 |   1 +
 Library/Homebrew/test/Gemfile.lock            |  14 ++
 .../test/rubocops/bottle_block_cop_spec.rb    |  67 ++++++++++
 .../test/rubocops/formula_desc_cop_spec.rb    | 121 ++++++++++++++++++
 5 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb
 create mode 100644 Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb

diff --git a/Library/Homebrew/test/.bundle/config b/Library/Homebrew/test/.bundle/config
index dcf3d1c4cf..e451829e9a 100644
--- a/Library/Homebrew/test/.bundle/config
+++ b/Library/Homebrew/test/.bundle/config
@@ -1,3 +1,3 @@
 ---
 BUNDLE_PATH: "../vendor/bundle"
-BUNDLE_DISABLE_SHARED_GEMS: '1'
+BUNDLE_DISABLE_SHARED_GEMS: "true"
diff --git a/Library/Homebrew/test/Gemfile b/Library/Homebrew/test/Gemfile
index bc9bccfbca..f3c16c710b 100644
--- a/Library/Homebrew/test/Gemfile
+++ b/Library/Homebrew/test/Gemfile
@@ -2,6 +2,7 @@ source "https://rubygems.org"
 
 gem "parallel_tests"
 gem "rspec"
+gem "rubocop"
 gem "rspec-its", require: false
 gem "rspec-wait", require: false
 
diff --git a/Library/Homebrew/test/Gemfile.lock b/Library/Homebrew/test/Gemfile.lock
index 64561be715..4d4eefd7d7 100644
--- a/Library/Homebrew/test/Gemfile.lock
+++ b/Library/Homebrew/test/Gemfile.lock
@@ -1,6 +1,7 @@
 GEM
   remote: https://rubygems.org/
   specs:
+    ast (2.3.0)
     codecov (0.1.9)
       json
       simplecov
@@ -11,6 +12,10 @@ GEM
     parallel (1.10.0)
     parallel_tests (2.13.0)
       parallel
+    parser (2.4.0.0)
+      ast (~> 2.2)
+    powerpack (0.1.1)
+    rainbow (2.2.1)
     rspec (3.5.0)
       rspec-core (~> 3.5.0)
       rspec-expectations (~> 3.5.0)
@@ -29,11 +34,19 @@ GEM
     rspec-support (3.5.0)
     rspec-wait (0.0.9)
       rspec (>= 3, < 4)
+    rubocop (0.47.1)
+      parser (>= 2.3.3.1, < 3.0)
+      powerpack (~> 0.1)
+      rainbow (>= 1.99.1, < 3.0)
+      ruby-progressbar (~> 1.7)
+      unicode-display_width (~> 1.0, >= 1.0.1)
+    ruby-progressbar (1.8.1)
     simplecov (0.13.0)
       docile (~> 1.1.0)
       json (>= 1.8, < 3)
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.0)
+    unicode-display_width (1.1.3)
     url (0.3.2)
 
 PLATFORMS
@@ -45,6 +58,7 @@ DEPENDENCIES
   rspec
   rspec-its
   rspec-wait
+  rubocop
   simplecov
 
 BUNDLED WITH
diff --git a/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb
new file mode 100644
index 0000000000..5be2d6cf50
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb
@@ -0,0 +1,67 @@
+require "rubocop"
+require "rubocop/rspec/support"
+require_relative "../../extend/string"
+require_relative "../../rubocops/bottle_block_cop"
+
+describe RuboCop::Cop::Homebrew::CorrectBottleBlock do
+  subject(:cop) { described_class.new }
+
+  context "When auditing Bottle Block" do
+    it "When there is revision in bottle block" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          bottle do
+            cellar :any
+            revision 2
+          end
+        end
+      EOS
+
+      expected_offenses = [{  message: "Use rebuild instead of revision in bottle block",
+                              severity: :convention,
+                              line: 5,
+                              column: 4,
+                              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
+
+  context "When auditing Bottle Block with auto correct" do
+    it "When there is revision in bottle block" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          bottle do
+            cellar :any
+            revision 2
+          end
+        end
+      EOS
+      corrected_source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          bottle do
+            cellar :any
+            rebuild 2
+          end
+        end
+      EOS
+
+      new_source = autocorrect_source(cop, source)
+      expect(new_source).to eq(corrected_source)
+    end
+  end
+end
diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
new file mode 100644
index 0000000000..cd506623fc
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
@@ -0,0 +1,121 @@
+require "rubocop"
+require "rubocop/rspec/support"
+require_relative "../../extend/string"
+require_relative "../../rubocops/formula_desc_cop"
+
+describe RuboCop::Cop::Homebrew::FormulaDesc do
+  subject(:cop) { described_class.new }
+
+  context "When auditing formula desc" do
+    it "When there is no desc" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+        end
+      EOS
+
+      expected_offenses = [{  message: "Formula should have a desc (Description).",
+                              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
+
+    it "When desc is too long" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc '#{"bar"*30}'
+        end
+      EOS
+
+      msg = <<-EOS.undent
+        Description is too long. "name: desc" should be less than 80 characters.
+        Length is calculated as Foo + desc. (currently 95)
+      EOS
+      expected_offenses = [{ message: msg,
+                             severity: :convention,
+                             line: 3,
+                             column: 7,
+                             source: source }]
+
+      inspect_source(cop, source)
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "When wrong \"command-line\" usage in desc" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc 'command line'
+        end
+      EOS
+
+      expected_offenses = [{ message: "Description should use \"command-line\" instead of \"command line\"",
+                             severity: :convention,
+                             line: 3,
+                             column: 8,
+                             source: source }]
+
+      inspect_source(cop, source)
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "When an article is used in desc" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc 'An '
+        end
+      EOS
+
+      expected_offenses = [{ message: "Description shouldn't start with an indefinite article (An )",
+                             severity: :convention,
+                             line: 3,
+                             column: 8,
+                             source: source }]
+
+      inspect_source(cop, source)
+      expected_offenses.zip(cop.offenses).each do |expected, actual|
+        expect_offense(expected, actual)
+      end
+    end
+
+    it "When formula name is in desc" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc 'Foo'
+        end
+      EOS
+
+      expected_offenses = [{ message: "Description shouldn't include the formula name",
+                             severity: :convention,
+                             line: 3,
+                             column: 8,
+                             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

From febc1085984e3d7fa23ee4c18d11e087afe20cbe Mon Sep 17 00:00:00 2001
From: Gautham Goli <goligautham@gmail.com>
Date: Sun, 12 Mar 2017 02:55:21 +0800
Subject: [PATCH 4/5] Encapsulate formula desc offense checking logic into
 methods

---
 Library/Homebrew/rubocops/formula_desc_cop.rb | 88 ++++++++++---------
 1 file changed, 47 insertions(+), 41 deletions(-)

diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb
index dcd928d286..d276be5cc6 100644
--- a/Library/Homebrew/rubocops/formula_desc_cop.rb
+++ b/Library/Homebrew/rubocops/formula_desc_cop.rb
@@ -13,6 +13,9 @@ module RuboCop
       # - Checks if `desc` contains the formula name
 
       class FormulaDesc < FormulaCop
+        attr_accessor :formula_name, :description, :source_buffer, :line_number, :line_begin_pos,
+                      :desc_begin_pos, :call_node
+
         def audit_formula(node, class_node, _parent_class_node, body)
           check(node, body, class_node.const_name)
         end
@@ -23,55 +26,58 @@ module RuboCop
           body.each_child_node(:send) do |call_node|
             _receiver, call_name, args = *call_node
             next if call_name != :desc || args.children[0].empty?
-            description = args.children[0]
+            @formula_name = formula_name
+            @description = args.children[0]
+            @source_buffer = call_node.source_range.source_buffer
+            @line_number = call_node.loc.line
+            @line_begin_pos = call_node.source_range.source_buffer.line_range(call_node.loc.line).begin_pos
+            @desc_begin_pos = call_node.children[2].source_range.begin_pos
+            @call_node = call_node
 
-            source_buffer = call_node.source_range.source_buffer
-            line_number = call_node.loc.line
-            line_begin_pos = call_node.source_range.source_buffer.line_range(call_node.loc.line).begin_pos
-            desc_begin_pos = call_node.children[2].source_range.begin_pos
+            check_for_desc_length_offense
 
-            linelength = "#{formula_name}: #{description}".length
-            if linelength > 80
-              column = desc_begin_pos - line_begin_pos
-              length = call_node.children[2].source_range.size
-              sourcerange = source_range(source_buffer, line_number, column, length)
-              message = <<-EOS.undent
-                Description is too long. "name: desc" should be less than 80 characters.
-                Length is calculated as #{formula_name} + desc. (currently #{linelength})
-              EOS
-              add_offense(call_node, sourcerange, message)
-            end
+            check_for_offense(/(command ?line)/i,
+                              "Description should use \"command-line\" instead of \"%s\"")
 
-            match_object = description.match(/(command ?line)/i)
-            if match_object
-              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
-              length = match_object.to_s.length
-              sourcerange = source_range(source_buffer, line_number, column, length)
-              message = "Description should use \"command-line\" instead of \"#{match_object}\""
-              add_offense(call_node, sourcerange, message)
-            end
+            check_for_offense(/^(an?)\s/i,
+                              "Description shouldn't start with an indefinite article (%s)")
 
-            match_object = description.match(/^(an?)\s/i)
-            if match_object
-              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
-              length = match_object.to_s.length
-              sourcerange = source_range(source_buffer, line_number, column, length)
-              message = "Description shouldn't start with an indefinite article (#{match_object})"
-              add_offense(call_node, sourcerange, message)
-            end
-
-            match_object = description.match(/^#{formula_name}/i)
-            if match_object
-              column = desc_begin_pos+match_object.begin(0)-line_begin_pos+1
-              length = match_object.to_s.length
-              sourcerange = source_range(source_buffer, line_number, column, length)
-              message = "Description shouldn't include the formula name"
-              add_offense(call_node, sourcerange, message)
-            end
+            check_for_offense(/^#{formula_name}/i,
+                              "Description shouldn't include the formula name")
             return nil
           end
           add_offense(node, node.source_range, "Formula should have a desc (Description).")
         end
+
+        def check_for_offense(regex, offense_msg)
+          # This method checks if particular regex has a match within formula's desc
+          # If so, adds a violation
+          match_object = @description.match(regex)
+          if match_object
+            column = @desc_begin_pos + match_object.begin(0) - @line_begin_pos + 1
+            length = match_object.to_s.length
+            offense_source_range = source_range(source_buffer, @line_number, column, length)
+            offense_msg = offense_msg % [match_object]
+            add_offense(@call_node, offense_source_range, offense_msg)
+          end
+        end
+
+        def check_for_desc_length_offense
+          # This method checks if desc length > max_desc_length
+          # If so, adds a violation
+          desc_length = "#{@formula_name}: #{@description}".length
+          max_desc_length = 80
+          if desc_length > max_desc_length
+            column = @desc_begin_pos - @line_begin_pos
+            length = @call_node.children[2].source_range.size
+            offense_source_range = source_range(source_buffer, @line_number, column, length)
+            desc_length_offense_msg = <<-EOS.undent
+              Description is too long. "name: desc" should be less than #{max_desc_length} characters.
+              Length is calculated as #{@formula_name} + desc. (currently #{"#{@formula_name}: #{@description}".length})
+            EOS
+            add_offense(@call_node, offense_source_range, desc_length_offense_msg)
+          end
+        end
       end
     end
   end

From a693ca332efcc56e0d7dbb3f43952226225ccf75 Mon Sep 17 00:00:00 2001
From: Gautham Goli <goligautham@gmail.com>
Date: Thu, 16 Mar 2017 23:49:43 +0530
Subject: [PATCH 5/5] Wrap rubocop specific code into methods inside FormulaCop

---
 Library/.rubocop.yml                          |   7 -
 Library/Homebrew/rubocops/bottle_block_cop.rb |  22 +--
 .../Homebrew/rubocops/extend/formula_cop.rb   | 126 +++++++++++++++++-
 Library/Homebrew/rubocops/formula_desc_cop.rb |  78 +++--------
 .../test/rubocops/formula_desc_cop_spec.rb    |   2 +-
 5 files changed, 149 insertions(+), 86 deletions(-)

diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml
index 41219f2cd3..a782c11172 100644
--- a/Library/.rubocop.yml
+++ b/Library/.rubocop.yml
@@ -8,17 +8,10 @@ AllCops:
 
 require: ./Homebrew/rubocops.rb
 
-Homebrew/FormulaCop:
-  Enabled: false
-
 Homebrew/CorrectBottleBlock:
-  Include:
-    - '**/Taps/homebrew/**/*.rb'
   Enabled: true
 
 Homebrew/FormulaDesc:
-  Include:
-    - '**/Taps/homebrew/**/*.rb'
   Enabled: true
 
 Metrics/AbcSize:
diff --git a/Library/Homebrew/rubocops/bottle_block_cop.rb b/Library/Homebrew/rubocops/bottle_block_cop.rb
index 733c492e88..4d7a944616 100644
--- a/Library/Homebrew/rubocops/bottle_block_cop.rb
+++ b/Library/Homebrew/rubocops/bottle_block_cop.rb
@@ -11,21 +11,13 @@ module RuboCop
         MSG = "Use rebuild instead of revision in bottle block".freeze
 
         def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node)
-          check(formula_class_body_node)
+          bottle = find_block(formula_class_body_node, :bottle)
+          return if bottle.nil? || block_size(bottle).zero?
+          problem "Use rebuild instead of revision in bottle block" if method_called?(bottle, :revision)
         end
 
         private
 
-        def check(formula_class_body_node)
-          formula_class_body_node.each_child_node(:block) do |block_node|
-            next if block_length(block_node).zero?
-            method, _args, block_body = *block_node
-            _keyword, method_name = *method
-            next unless method_name == :bottle
-            check_revision?(block_body)
-          end
-        end
-
         def autocorrect(node)
           lambda do |corrector|
             correction = node.source.sub("revision", "rebuild")
@@ -33,14 +25,6 @@ module RuboCop
             corrector.remove(node.source_range)
           end
         end
-
-        def check_revision?(body)
-          body.children.each do |method_call_node|
-            _receiver, method_name, _args = *method_call_node
-            next unless method_name == :revision
-            add_offense(method_call_node, :expression)
-          end
-        end
       end
     end
   end
diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
index cf5d513c93..49108bd48a 100644
--- a/Library/Homebrew/rubocops/extend/formula_cop.rb
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -6,16 +6,138 @@ module RuboCop
 
         def on_class(node)
           # This method is called by RuboCop and is the main entry point
+          file_path = processed_source.buffer.name
+          return unless file_path_allowed?(file_path)
           class_node, parent_class_node, body = *node
-          return unless a_formula_class?(parent_class_node)
+          return unless formula_class?(parent_class_node)
+          return unless respond_to?(:audit_formula)
+          @formula_name = class_name(class_node)
           audit_formula(node, class_node, parent_class_node, body)
         end
 
+        def regex_match_group(node, pattern)
+          # Checks for regex match of pattern in the node and
+          # Sets the appropriate instance variables to report the match
+          string_repr = string_content(node)
+          match_object = string_repr.match(pattern)
+          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
+          @length = match_object.to_s.length
+          @line_no = line_number(node)
+          @source_buf = source_buffer(node)
+          @offense_source_range = source_range(@source_buf, @line_no, @column, @length)
+          @offensive_node = node
+          match_object
+        end
+
+        def find_node_method_by_name(node, method_name)
+          # Returns method_node matching method_name
+          return if node.nil?
+          node.each_child_node(:send) do |method_node|
+            next unless method_node.method_name == method_name
+            @offensive_node = method_node
+            @offense_source_range = method_node.source_range
+            return method_node
+          end
+          # If not found then, parent node becomes the offensive node
+          @offensive_node = node.parent
+          @offense_source_range = node.parent.source_range
+          nil
+        end
+
+        def find_block(node, block_name)
+          # Returns a block named block_name inside node
+          return if node.nil?
+          node.each_child_node(:block) do |block_node|
+            next if block_node.method_name != block_name
+            @offensive_node = block_node
+            @offense_source_range = block_node.source_range
+            return block_node
+          end
+          # If not found then, parent node becomes the offensive node
+          @offensive_node = node.parent
+          @offense_source_range = node.parent.source_range
+          nil
+        end
+
+        def method_called?(node, method_name)
+          # Check if a method is called inside a block
+          block_body = node.children[2]
+          block_body.each_child_node(:send) do |call_node|
+            next unless call_node.method_name == method_name
+            @offensive_node = call_node
+            @offense_source_range = call_node.source_range
+            return true
+          end
+          false
+        end
+
+        def parameters(method_node)
+          # Returns the array of arguments of the method_node
+          return unless method_node.send_type?
+          method_node.method_args
+        end
+
+        def line_start_column(node)
+          # Returns the begin position of the node's line in source code
+          node.source_range.source_buffer.line_range(node.loc.line).begin_pos
+        end
+
+        def start_column(node)
+          # Returns the begin position of the node in source code
+          node.source_range.begin_pos
+        end
+
+        def line_number(node)
+          # Returns the line number of the node
+          node.loc.line
+        end
+
+        def class_name(node)
+          # Returns the class node's name, nil if not a class node
+          @offensive_node = node
+          @offense_source_range = node.source_range
+          node.const_name
+        end
+
+        def size(node)
+          # Returns the node size in the source code
+          node.source_range.size
+        end
+
+        def block_size(block)
+          # Returns the block length of the block node
+          block_length(block)
+        end
+
+        def source_buffer(node)
+          # Source buffer is required as an argument to report style violations
+          node.source_range.source_buffer
+        end
+
+        def string_content(node)
+          # Returns the string representation if node is of type str
+          node.str_content if node.type == :str
+        end
+
+        def problem(msg)
+          add_offense(@offensive_node, @offense_source_range, msg)
+        end
+
         private
 
-        def a_formula_class?(parent_class_node)
+        def formula_class?(parent_class_node)
           parent_class_node && parent_class_node.const_name == "Formula"
         end
+
+        def file_path_allowed?(file_path)
+          paths_to_exclude = [%r{/Library/Homebrew/compat/},
+                              %r{/Library/Homebrew/test/}]
+          return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs
+          file_path !~ Regexp.union(paths_to_exclude)
+        end
       end
     end
   end
diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb
index d276be5cc6..7d69a48e7a 100644
--- a/Library/Homebrew/rubocops/formula_desc_cop.rb
+++ b/Library/Homebrew/rubocops/formula_desc_cop.rb
@@ -11,72 +11,36 @@ module RuboCop
       # - Checks if `desc` begins with an article
       # - Checks for correct usage of `command-line` in `desc`
       # - Checks if `desc` contains the formula name
-
       class FormulaDesc < FormulaCop
-        attr_accessor :formula_name, :description, :source_buffer, :line_number, :line_begin_pos,
-                      :desc_begin_pos, :call_node
+        def audit_formula(_node, _class_node, _parent_class_node, body)
+          desc_call = find_node_method_by_name(body, :desc)
 
-        def audit_formula(node, class_node, _parent_class_node, body)
-          check(node, body, class_node.const_name)
-        end
-
-        private
-
-        def check(node, body, formula_name)
-          body.each_child_node(:send) do |call_node|
-            _receiver, call_name, args = *call_node
-            next if call_name != :desc || args.children[0].empty?
-            @formula_name = formula_name
-            @description = args.children[0]
-            @source_buffer = call_node.source_range.source_buffer
-            @line_number = call_node.loc.line
-            @line_begin_pos = call_node.source_range.source_buffer.line_range(call_node.loc.line).begin_pos
-            @desc_begin_pos = call_node.children[2].source_range.begin_pos
-            @call_node = call_node
-
-            check_for_desc_length_offense
-
-            check_for_offense(/(command ?line)/i,
-                              "Description should use \"command-line\" instead of \"%s\"")
-
-            check_for_offense(/^(an?)\s/i,
-                              "Description shouldn't start with an indefinite article (%s)")
-
-            check_for_offense(/^#{formula_name}/i,
-                              "Description shouldn't include the formula name")
-            return nil
+          if desc_call.nil?
+            problem "Formula should have a desc (Description)."
+            return
           end
-          add_offense(node, node.source_range, "Formula should have a desc (Description).")
-        end
 
-        def check_for_offense(regex, offense_msg)
-          # This method checks if particular regex has a match within formula's desc
-          # If so, adds a violation
-          match_object = @description.match(regex)
-          if match_object
-            column = @desc_begin_pos + match_object.begin(0) - @line_begin_pos + 1
-            length = match_object.to_s.length
-            offense_source_range = source_range(source_buffer, @line_number, column, length)
-            offense_msg = offense_msg % [match_object]
-            add_offense(@call_node, offense_source_range, offense_msg)
-          end
-        end
-
-        def check_for_desc_length_offense
-          # This method checks if desc length > max_desc_length
-          # If so, adds a violation
-          desc_length = "#{@formula_name}: #{@description}".length
+          desc = parameters(desc_call).first
+          desc_length = "#{@formula_name}: #{string_content(desc)}".length
           max_desc_length = 80
           if desc_length > max_desc_length
-            column = @desc_begin_pos - @line_begin_pos
-            length = @call_node.children[2].source_range.size
-            offense_source_range = source_range(source_buffer, @line_number, column, length)
-            desc_length_offense_msg = <<-EOS.undent
+            problem <<-EOS.undent
               Description is too long. "name: desc" should be less than #{max_desc_length} characters.
-              Length is calculated as #{@formula_name} + desc. (currently #{"#{@formula_name}: #{@description}".length})
+              Length is calculated as #{@formula_name} + desc. (currently #{desc_length})
             EOS
-            add_offense(@call_node, offense_source_range, desc_length_offense_msg)
           end
+
+          # Check if command-line is wrongly used in formula's desc
+          if match = regex_match_group(desc, /(command ?line)/i)
+            problem "Description should use \"command-line\" instead of \"#{match}\""
+          end
+
+          if match = regex_match_group(desc, /^(an?)\s/i)
+            problem "Description shouldn't start with an indefinite article (#{match})"
+          end
+
+          # Check if formula's name is used in formula's desc
+          problem "Description shouldn't include the formula name" if regex_match_group(desc, /^#{@formula_name}/i)
         end
       end
     end
diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
index cd506623fc..04c4c27da6 100644
--- a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
+++ b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb
@@ -42,7 +42,7 @@ describe RuboCop::Cop::Homebrew::FormulaDesc do
       expected_offenses = [{ message: msg,
                              severity: :convention,
                              line: 3,
-                             column: 7,
+                             column: 2,
                              source: source }]
 
       inspect_source(cop, source)