Extract formula desc auditing code to a separate module

The extracted module will be used for Cask descs as well.

Co-authored-by: Markus Reiter <me@reitermark.us>

Library/Homebrew/rubocops/extend/formula.rb

@@ -10,11 +10,13 @@ ensure
 end
 
 require "extend/string"
+require "rubocops/shared/helper_functions"
 
 module RuboCop
   module Cop
     class FormulaCop < Cop
       include RangeHelp
+      include HelperFunctions
 
       attr_accessor :file_path
 
@@ -32,28 +34,6 @@ module RuboCop
         audit_formula(node, class_node, parent_class_node, @body)
       end
 
-      # Checks for regex match of pattern in the node and
-      # sets the appropriate instance variables to report the match
-      def regex_match_group(node, pattern)
-        string_repr = string_content(node).encode("UTF-8", invalid: :replace)
-        match_object = string_repr.match(pattern)
-        return unless match_object
-
-        node_begin_pos = start_column(node)
-        line_begin_pos = line_start_column(node)
-        @column = if node_begin_pos == line_begin_pos
-          node_begin_pos + match_object.begin(0) - line_begin_pos
-        else
-          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)
-        @offense_source_range = source_range(@source_buf, @line_no, @column, @length)
-        @offensive_node = node
-        match_object
-      end
-
       # Yields to block when there is a match.
       # @param urls [Array] url/mirror method call nodes
       # @param regex [Regexp] pattern to match urls
@@ -442,26 +422,11 @@ module RuboCop
         end
       end
 
-      # Returns the begin position of the node's line in source code
-      def line_start_column(node)
-        node.source_range.source_buffer.line_range(node.loc.line).begin_pos
-      end
-
-      # Returns the begin position of the node in source code
-      def start_column(node)
-        node.source_range.begin_pos
-      end
-
       # Returns the ending position of the node in source code
       def end_column(node)
         node.source_range.end_pos
       end
 
-      # Returns the line number of the node
-      def line_number(node)
-        node.loc.line
-      end
-
       # Returns the class node's name, nil if not a class node
       def class_name(node)
         @offensive_node = node
@@ -484,35 +449,6 @@ module RuboCop
         block.loc.end.line - block.loc.begin.line
       end
 
-      # Source buffer is required as an argument to report style violations
-      def source_buffer(node)
-        node.source_range.source_buffer
-      end
-
-      # Returns the string representation if node is of type str(plain) or dstr(interpolated) or const
-      def string_content(node)
-        case node.type
-        when :str
-          node.str_content
-        when :dstr
-          content = ""
-          node.each_child_node(:str, :begin) do |child|
-            content += if child.begin_type?
-              child.source
-            else
-              child.str_content
-            end
-          end
-          content
-        when :const
-          node.const_name
-        when :sym
-          node.children.first.to_s
-        else
-          ""
-        end
-      end
-
       # Returns true if the formula is versioned
       def versioned_formula?
         @formula_name.include?("@")
@@ -532,10 +468,6 @@ module RuboCop
         match_obj[1]
       end
 
-      def problem(msg)
-        add_offense(@offensive_node, location: @offense_source_range, message: msg)
-      end
-
       private
 
       def formula_class?(node)

Library/Homebrew/rubocops/formula_desc.rb

@@ -1,105 +1,24 @@
 # frozen_string_literal: true
 
 require "rubocops/extend/formula"
+require "rubocops/shared/desc_helper"
 require "extend/string"
 
 module RuboCop
   module Cop
     module FormulaAudit
       # This cop audits `desc` in Formulae.
-      #
+      # See the `DescHelper` module for details of the checks.
-      # - Checks for existence of `desc`
-      # - Checks if size of `desc` > 80
-      # - Checks for leading/trailing whitespace in `desc`
-      # - 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
-      # - Checks if `desc` ends with a full stop (apart from in the case of "etc.")
       class Desc < FormulaCop
-        VALID_LOWERCASE_WORDS = %w[
+        include DescHelper
-          macOS
-        ].freeze
 
         def audit_formula(_node, _class_node, _parent_class_node, body_node)
           desc_call = find_node_method_by_name(body_node, :desc)
-
+          audit_desc(:formula, @formula_name, desc_call)
-          # Check if a formula's desc is present
-          if desc_call.nil?
-            problem "Formula should have a desc (Description)."
-            return
-          end
-
-          desc = parameters(desc_call).first
-
-          # Check the formula's desc length. Should be >0 and <80 characters.
-          pure_desc_length = string_content(desc).length
-          if pure_desc_length.zero?
-            problem "The desc (description) should not be an empty string."
-            return
-          end
-
-          # Check for leading whitespace.
-          problem "Description shouldn't have a leading space" if regex_match_group(desc, /^\s+/)
-
-          # Check for trailing whitespace.
-          problem "Description shouldn't have a trailing space" if regex_match_group(desc, /\s+$/)
-
-          # Check if command-line is wrongly used in formula's desc
-          if match = regex_match_group(desc, /(command ?line)/i)
-            c = match.to_s[0]
-            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, i.e. \"#{match.to_s.strip}\""
-          end
-
-          # 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 desc starts with formula's name
-          if regex_match_group(desc, /^#{@formula_name} /i)
-            problem "Description shouldn't start with the formula name"
-          end
-
-          # Check if a full stop is used at the end of a formula's desc (apart from in the case of "etc.")
-          if regex_match_group(desc, /\.$/) && !string_content(desc).end_with?("etc.")
-            problem "Description shouldn't end with a full stop"
-          end
-
-          desc_length = "#{@formula_name}: #{string_content(desc)}".length
-          max_desc_length = 80
-          return if desc_length <= max_desc_length
-
-          problem "Description is too long. \"name: desc\" should be less than #{max_desc_length} characters. " \
-                  "Length is calculated as #{@formula_name} + desc. (currently #{desc_length})"
         end
 
         def autocorrect(node)
-          lambda do |corrector|
+          autocorrect_desc(node, @formula_name)
-            correction = node.source
-            first_word = string_content(node).split.first
-            unless VALID_LOWERCASE_WORDS.include?(first_word)
-              first_char = first_word.to_s[0]
-              correction.sub!(/^(['"]?)([a-z])/, "\\1#{first_char.upcase}") if first_char
-            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")
-            correction.gsub!(/\.(['"]?)$/, "\\1")
-            correction.gsub!(/^\s+/, "")
-            correction.gsub!(/\s+$/, "")
-            corrector.insert_before(node.source_range, correction)
-            corrector.remove(node.source_range)
-          end
         end
       end
     end

Library/Homebrew/rubocops/shared/desc_helper.rb

@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+require "rubocops/shared/helper_functions"
+
+module RuboCop
+  module Cop
+    # This module performs common checks the `desc` field in both Formulae and Casks.
+    module DescHelper
+      include HelperFunctions
+
+      VALID_LOWERCASE_WORDS = %w[
+        macOS
+      ].freeze
+
+      def audit_desc(type, name, desc_call)
+        # Check if a desc is present.
+        if desc_call.nil?
+          problem "#{type.to_s.capitalize} should have a desc (Description)."
+          return
+        end
+
+        desc = desc_call.first_argument
+
+        # Check if the desc is empty.
+        pure_desc_length = string_content(desc).length
+        if pure_desc_length.zero?
+          problem "The desc (description) should not be an empty string."
+          return
+        end
+
+        # Check the desc for leading whitespace.
+        problem "Description shouldn't have leading spaces." if regex_match_group(desc, /^\s+/)
+
+        # Check the desc for trailing whitespace.
+        problem "Description shouldn't have trailing spaces." if regex_match_group(desc, /\s+$/)
+
+        # Check if "command-line" is spelled incorrectly in the desc.
+        if match = regex_match_group(desc, /(command ?line)/i)
+          c = match.to_s[0]
+          problem "Description should use \"#{c}ommand-line\" instead of \"#{match}\"."
+        end
+
+        # Check if the desc starts with "A" or "An".
+        if match = regex_match_group(desc, /^(an?)(?=\s)/i)
+          problem "Description shouldn't start with an indefinite article, i.e. \"#{match}\"."
+        end
+
+        # Check if invalid lowercase words are at the start of a 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 the desc starts with the formula's or cask's name.
+        problem "Description shouldn't start with the #{type} name." if regex_match_group(desc, /^#{name} /i)
+
+        # Check if a full stop is used at the end of a desc (apart from in the case of "etc.").
+        if regex_match_group(desc, /\.$/) && !string_content(desc).end_with?("etc.")
+          problem "Description shouldn't end with a full stop."
+        end
+
+        # Check if the desc length exceeds 80 characters.
+        desc_length = "#{name}: #{string_content(desc)}".length
+        max_desc_length = 80
+        return if desc_length <= max_desc_length
+
+        problem "Description is too long. \"name: desc\" should be less than #{max_desc_length} characters. " \
+                "The current combined length is #{desc_length}."
+      end
+
+      def autocorrect_desc(node, name)
+        lambda do |corrector|
+          /\A(?<quote>["'])(?<correction>.*)(?:\k<quote>)\Z/ =~ node.source
+
+          next if correction.nil?
+
+          correction.gsub!(/^\s+/, "")
+          correction.gsub!(/\s+$/, "")
+
+          correction.sub!(/^an?\s+/i, "")
+
+          first_word = correction.split.first
+          unless VALID_LOWERCASE_WORDS.include?(first_word)
+            first_char = first_word.to_s[0]
+            correction[0] = first_char.upcase if first_char
+          end
+
+          correction.gsub!(/(ommand ?line)/i, "ommand-line")
+          correction.gsub!(/(^|[^a-z])#{name}([^a-z]|$)/i, "\\1\\2")
+          correction.gsub!(/^\s+/, "")
+          correction.gsub!(/\s+$/, "")
+          correction.gsub!(/\.$/, "")
+
+          corrector.replace(node.source_range, "#{quote}#{correction}#{quote}")
+        end
+      end
+    end
+  end
+end

Library/Homebrew/rubocops/shared/helper_functions.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module HelperFunctions
+      include RangeHelp
+
+      # Checks for regex match of pattern in the node and
+      # sets the appropriate instance variables to report the match
+      def regex_match_group(node, pattern)
+        string_repr = string_content(node).encode("UTF-8", invalid: :replace)
+        match_object = string_repr.match(pattern)
+        return unless match_object
+
+        node_begin_pos = start_column(node)
+        line_begin_pos = line_start_column(node)
+        @column = if node_begin_pos == line_begin_pos
+          node_begin_pos + match_object.begin(0) - line_begin_pos
+        else
+          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)
+        @offense_source_range = source_range(@source_buf, @line_no, @column, @length)
+        @offensive_node = node
+        match_object
+      end
+
+      # Returns the begin position of the node's line in source code
+      def line_start_column(node)
+        node.source_range.source_buffer.line_range(node.loc.line).begin_pos
+      end
+
+      # Returns the begin position of the node in source code
+      def start_column(node)
+        node.source_range.begin_pos
+      end
+
+      # Returns the line number of the node
+      def line_number(node)
+        node.loc.line
+      end
+
+      # Source buffer is required as an argument to report style violations
+      def source_buffer(node)
+        node.source_range.source_buffer
+      end
+
+      # Returns the string representation if node is of type str(plain) or dstr(interpolated) or const
+      def string_content(node)
+        case node.type
+        when :str
+          node.str_content
+        when :dstr
+          content = ""
+          node.each_child_node(:str, :begin) do |child|
+            content += if child.begin_type?
+              child.source
+            else
+              child.str_content
+            end
+          end
+          content
+        when :const
+          node.const_name
+        when :sym
+          node.children.first.to_s
+        else
+          ""
+        end
+      end
+
+      def problem(msg)
+        add_offense(@offensive_node, location: @offense_source_range, message: msg)
+      end
+    end
+  end
+end

Library/Homebrew/test/rubocops/formula_desc_spec.rb

@@ -15,7 +15,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do
       RUBY
     end
 
-    it "reports an offense when desc is an empty string" do
+    it "When desc is an empty string" do
       expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
@@ -30,81 +30,30 @@ describe RuboCop::Cop::FormulaAudit::Desc do
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
           desc 'Bar#{"bar" * 29}'
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Description is too long. "name: desc" should be less than 80 characters. Length is calculated as foo + desc. (currently 95)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Description is too long. "name: desc" should be less than 80 characters. The current combined length is 95.
         end
       RUBY
     end
 
-    it "When desc is multiline string" do
+    it "When desc is a multiline string" do
       expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
           desc 'Bar#{"bar" * 9}'\
             '#{"foo" * 21}'
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Description is too long. "name: desc" should be less than 80 characters. Length is calculated as foo + desc. (currently 98)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Description is too long. "name: desc" should be less than 80 characters. The current combined length is 98.
         end
       RUBY
     end
   end
 
   context "When auditing formula desc" do
-    it "When wrong \"command-line\" usage in desc" do
-      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
-        class Foo < Formula
-          url 'https://brew.sh/foo-1.0.tgz'
-          desc 'command line'
-                ^ Description should start with a capital letter
-                ^^^^^^^^^^^^ Description should use \"command-line\" instead of \"command line\"
-        end
-      RUBY
-    end
-
-    it "When an article is used in desc" do
-      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
-        class Foo < Formula
-          url 'https://brew.sh/foo-1.0.tgz'
-          desc 'An aardvark'
-                ^^^ Description shouldn\'t start with an indefinite article, i.e. \"An\"
-        end
-      RUBY
-    end
-
-    it "When an lowercase letter starts a desc" do
-      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
-        class Foo < Formula
-          url 'https://brew.sh/foo-1.0.tgz'
-          desc 'bar'
-                ^ Description should start with a capital letter
-        end
-      RUBY
-    end
-
-    it "When formula name is in desc" do
-      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
-        class Foo < Formula
-          url 'https://brew.sh/foo-1.0.tgz'
-          desc 'Foo is a foobar'
-                ^^^^ Description shouldn\'t start with the formula name
-        end
-      RUBY
-    end
-
-    it "When the description ends with a full stop" do
-      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
-        class Foo < Formula
-          url 'https://brew.sh/foo-1.0.tgz'
-          desc 'Description with a full stop at the end.'
-                                                       ^ Description shouldn\'t end with a full stop
-        end
-      RUBY
-    end
-
     it "When the description starts with a leading space" do
       expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
           desc ' Description with a leading space'
-                ^ Description shouldn\'t have a leading space
+                ^ Description shouldn\'t have leading spaces.
         end
       RUBY
     end
@@ -114,7 +63,58 @@ describe RuboCop::Cop::FormulaAudit::Desc do
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
           desc 'Description with a trailing space '
-                                                 ^ Description shouldn\'t have a trailing space
+                                                 ^ Description shouldn\'t have trailing spaces.
+        end
+      RUBY
+    end
+
+    it "When \"command-line\" is incorrectly spelled in the desc" do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          desc 'command line'
+                ^ Description should start with a capital letter.
+                ^^^^^^^^^^^^ Description should use \"command-line\" instead of \"command line\".
+        end
+      RUBY
+    end
+
+    it "When an article is used in the desc" do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          desc 'An aardvark'
+                ^^ Description shouldn\'t start with an indefinite article, i.e. \"An\".
+        end
+      RUBY
+    end
+
+    it "When the desc starts with a lowercase letter" do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          desc 'bar'
+                ^ Description should start with a capital letter.
+        end
+      RUBY
+    end
+
+    it "When the desc starts with the formula name" do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          desc 'Foo is a foobar'
+                ^^^^ Description shouldn\'t start with the formula name.
+        end
+      RUBY
+    end
+
+    it "When the description ends with a full stop" do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          desc 'Description with a full stop at the end.'
+                                                       ^ Description shouldn\'t end with a full stop.
         end
       RUBY
     end
@@ -130,7 +130,7 @@ describe RuboCop::Cop::FormulaAudit::Desc do
       correct_source = <<~RUBY
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
-          desc 'an bar: command-line'
+          desc 'Bar: command-line'
         end
       RUBY