From fc6813f0947d732d97e76fe1f588c3a0e8bf1463 Mon Sep 17 00:00:00 2001 From: Waldir Pimenta Date: Thu, 30 Jul 2020 16:29:12 +0100 Subject: [PATCH] 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 --- Library/Homebrew/rubocops/extend/formula.rb | 72 +---------- Library/Homebrew/rubocops/formula_desc.rb | 91 +------------- .../Homebrew/rubocops/shared/desc_helper.rb | 99 +++++++++++++++ .../rubocops/shared/helper_functions.rb | 79 ++++++++++++ .../test/rubocops/formula_desc_spec.rb | 116 +++++++++--------- 5 files changed, 243 insertions(+), 214 deletions(-) create mode 100644 Library/Homebrew/rubocops/shared/desc_helper.rb create mode 100644 Library/Homebrew/rubocops/shared/helper_functions.rb diff --git a/Library/Homebrew/rubocops/extend/formula.rb b/Library/Homebrew/rubocops/extend/formula.rb index b7e5bfccb8..8f165acc68 100644 --- a/Library/Homebrew/rubocops/extend/formula.rb +++ b/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) diff --git a/Library/Homebrew/rubocops/formula_desc.rb b/Library/Homebrew/rubocops/formula_desc.rb index 3449ee43a8..57e5066e13 100644 --- a/Library/Homebrew/rubocops/formula_desc.rb +++ b/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. - # - # - 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.") + # See the `DescHelper` module for details of the checks. class Desc < FormulaCop - VALID_LOWERCASE_WORDS = %w[ - macOS - ].freeze + include DescHelper def audit_formula(_node, _class_node, _parent_class_node, body_node) desc_call = find_node_method_by_name(body_node, :desc) - - # 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})" + audit_desc(:formula, @formula_name, desc_call) end 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[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 + autocorrect_desc(node, @formula_name) end end end diff --git a/Library/Homebrew/rubocops/shared/desc_helper.rb b/Library/Homebrew/rubocops/shared/desc_helper.rb new file mode 100644 index 0000000000..1ff7a80ad7 --- /dev/null +++ b/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(?["'])(?.*)(?:\k)\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 diff --git a/Library/Homebrew/rubocops/shared/helper_functions.rb b/Library/Homebrew/rubocops/shared/helper_functions.rb new file mode 100644 index 0000000000..a5e893c6ae --- /dev/null +++ b/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 diff --git a/Library/Homebrew/test/rubocops/formula_desc_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_spec.rb index 0b9f7dcb28..8f7ea8f806 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_spec.rb +++ b/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