From c572081f8b56f3be24f24e85497ae780a25cbcba Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 8 Jun 2017 15:13:06 +0300 Subject: [PATCH] formula_desc_cop: tweak some rules. Allow some specific lowercase words and provide an autocorrect for some of these rules. --- Library/Homebrew/rubocops/formula_desc_cop.rb | 70 +++++++++++++++---- .../test/rubocops/formula_desc_cop_spec.rb | 55 ++++++++++++--- Library/Homebrew/test/spec_helper.rb | 2 + .../Homebrew/test/support/helper/rubocop.rb | 13 ++++ 4 files changed, 118 insertions(+), 22 deletions(-) create mode 100644 Library/Homebrew/test/support/helper/rubocop.rb diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb index 54a14b39d3..27b00b4164 100644 --- a/Library/Homebrew/rubocops/formula_desc_cop.rb +++ b/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 - # - Checks for correct usage of `command-line` in `desc` - # - Checks if `desc` contains the formula name - class Desc < FormulaCop + class DescLength < 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 - 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 #{desc_length}) - EOS - end + return if desc_length <= max_desc_length + 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 #{desc_length}) + EOS + 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 diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb index f6436d6a36..4e684be46a 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb +++ b/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) - 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]) + it "autocorrects all rules" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + 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 diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index e193b91a0f..03b14720b8 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/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"] diff --git a/Library/Homebrew/test/support/helper/rubocop.rb b/Library/Homebrew/test/support/helper/rubocop.rb new file mode 100644 index 0000000000..351f2ad826 --- /dev/null +++ b/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