Merge pull request #2748 from MikeMcQuaid/desc-cop-tweaks

formula_desc_cop: tweak some rules.

Library/Homebrew/rubocops/formula_desc_cop.rb

@@ -8,27 +8,46 @@ module RuboCop
       #
       # - Checks for existence of `desc`
       # - Checks if size of `desc` > 80
-      # - Checks if `desc` begins with an article
+      class DescLength < FormulaCop
-      # - Checks for correct usage of `command-line` in `desc`
-      # - Checks if `desc` contains the formula name
-      class Desc < FormulaCop
         def audit_formula(_node, _class_node, _parent_class_node, body)
           desc_call = find_node_method_by_name(body, :desc)
 
+          # Check if a formula's desc is present
           if desc_call.nil?
             problem "Formula should have a desc (Description)."
             return
           end
 
+          # Check if a formula's desc is too long
           desc = parameters(desc_call).first
           desc_length = "#{@formula_name}: #{string_content(desc)}".length
           max_desc_length = 80
-          if desc_length > max_desc_length
+          return if desc_length <= max_desc_length
-            problem <<-EOS.undent
+          problem <<-EOS.undent
-              Description is too long. "name: desc" should be less than #{max_desc_length} characters.
+            Description is too long. "name: desc" should be less than #{max_desc_length} characters.
-              Length is calculated as #{@formula_name} + desc. (currently #{desc_length})
+            Length is calculated as #{@formula_name} + desc. (currently #{desc_length})
-            EOS
+          EOS
-          end
+        end
+      end
+
+      # This cop audits `desc` in Formulae
+      #
+      # - Checks if `desc` begins with an article
+      # - Checks for correct usage of `command-line` in `desc`
+      # - Checks description starts with a capital letter
+      # - Checks if `desc` contains the formula name
+      class Desc < FormulaCop
+        VALID_LOWERCASE_WORDS = %w[
+          iOS
+          macOS
+          xUnit
+        ].freeze
+
+        def audit_formula(_node, _class_node, _parent_class_node, body)
+          desc_call = find_node_method_by_name(body, :desc)
+          return if desc_call.nil?
+
+          desc = parameters(desc_call).first
 
           # Check if command-line is wrongly used in formula's desc
           if match = regex_match_group(desc, /(command ?line)/i)
@@ -36,16 +55,41 @@ module RuboCop
             problem "Description should use \"#{c}ommand-line\" instead of \"#{match}\""
           end
 
+          # Check if a/an are used in a formula's desc
           if match = regex_match_group(desc, /^(an?)\s/i)
-            problem "Description shouldn't start with an indefinite article (#{match})"
+            problem "Description shouldn't start with an indefinite article i.e. \"#{match.to_s.strip}\""
           end
 
-          if regex_match_group(desc, /^[a-z]/)
+          # Check if invalid uppercase words are at the start of a
+          # formula's desc
+          if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) &&
+             regex_match_group(desc, /^[a-z]/)
             problem "Description should start with a capital letter"
           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}\b/i)
+          return unless regex_match_group(desc, /(^|[^a-z])#{@formula_name}([^a-z]|$)/i)
+          problem "Description shouldn't include the formula name"
+        end
+
+        private
+
+        def autocorrect(node)
+          lambda do |corrector|
+            correction = node.source
+            first_word = string_content(node).split.first
+            unless VALID_LOWERCASE_WORDS.include?(first_word)
+              first_char = first_word.to_s.chars.first
+              correction.sub!(/^(['"]?)([a-z])/, "\\1#{first_char.upcase}")
+            end
+            correction.sub!(/^(['"]?)an?\s/i, "\\1")
+            correction.gsub!(/(ommand ?line)/i, "ommand-line")
+            correction.gsub!(/(^|[^a-z])#{@formula_name}([^a-z]|$)/i, "\\1\\2")
+            correction.gsub!(/^(['"]?)\s+/, "\\1")
+            correction.gsub!(/\s+(['"]?)$/, "\\1")
+            corrector.insert_before(node.source_range, correction)
+            corrector.remove(node.source_range)
+          end
         end
       end
     end

Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb

@@ -3,7 +3,7 @@ require "rubocop/rspec/support"
 require_relative "../../extend/string"
 require_relative "../../rubocops/formula_desc_cop"
 
-describe RuboCop::Cop::FormulaAuditStrict::Desc do
+describe RuboCop::Cop::FormulaAuditStrict::DescLength do
   subject(:cop) { described_class.new }
 
   context "When auditing formula desc" do
@@ -31,7 +31,7 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
       source = <<-EOS.undent
         class Foo < Formula
           url 'http://example.com/foo-1.0.tgz'
-          desc '#{"bar" * 30}'
+          desc 'Bar#{"bar" * 29}'
         end
       EOS
 
@@ -55,7 +55,7 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
       source = <<-EOS.undent
         class Foo < Formula
           url 'http://example.com/foo-1.0.tgz'
-          desc '#{"bar" * 10}'\
+          desc 'Bar#{"bar" * 9}'\
             '#{"foo" * 21}'
         end
       EOS
@@ -75,7 +75,13 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
         expect_offense(expected, actual)
       end
     end
+  end
+end
 
+describe RuboCop::Cop::FormulaAuditStrict::Desc do
+  subject(:cop) { described_class.new }
+
+  context "When auditing formula desc" do
     it "When wrong \"command-line\" usage in desc" do
       source = <<-EOS.undent
         class Foo < Formula
@@ -104,7 +110,27 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
         end
       EOS
 
-      expected_offenses = [{ message: "Description shouldn't start with an indefinite article (An )",
+      expected_offenses = [{ message: "Description shouldn't start with an indefinite article i.e. \"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 an lowercase letter starts a desc" do
+      source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc 'bar'
+        end
+      EOS
+
+      expected_offenses = [{ message: "Description should start with a capital letter",
                              severity: :convention,
                              line: 3,
                              column: 8,
@@ -136,11 +162,22 @@ describe RuboCop::Cop::FormulaAuditStrict::Desc do
       end
     end
 
-    def expect_offense(expected, actual)
+    it "autocorrects all rules" do
-      expect(actual.message).to eq(expected[:message])
+      source = <<-EOS.undent
-      expect(actual.severity).to eq(expected[:severity])
+        class Foo < Formula
-      expect(actual.line).to eq(expected[:line])
+          url 'http://example.com/foo-1.0.tgz'
-      expect(actual.column).to eq(expected[:column])
+          desc ' an bar: commandline foo '
+        end
+      EOS
+      correct_source = <<-EOS.undent
+        class Foo < Formula
+          url 'http://example.com/foo-1.0.tgz'
+          desc 'an bar: command-line'
+        end
+      EOS
+
+      corrected_source = autocorrect_source(cop, source)
+      expect(corrected_source).to eq(correct_source)
     end
   end
 end

Library/Homebrew/test/spec_helper.rb

@@ -23,6 +23,7 @@ require "test/support/helper/shutup"
 require "test/support/helper/fixtures"
 require "test/support/helper/formula"
 require "test/support/helper/mktmpdir"
+require "test/support/helper/rubocop"
 
 require "test/support/helper/spec/shared_context/homebrew_cask" if OS.mac?
 require "test/support/helper/spec/shared_context/integration_test"
@@ -44,6 +45,7 @@ RSpec.configure do |config|
   config.include(Test::Helper::Fixtures)
   config.include(Test::Helper::Formula)
   config.include(Test::Helper::MkTmpDir)
+  config.include(Test::Helper::RuboCop)
 
   config.before(:each, :needs_compat) do
     skip "Requires compatibility layer." if ENV["HOMEBREW_NO_COMPAT"]

Library/Homebrew/test/support/helper/rubocop.rb

@@ -0,0 +1,13 @@
+module Test
+  module Helper
+    module RuboCop
+      def expect_offense(expected, actual)
+        expect(actual).to_not be_nil
+        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
+end