From 19f693d25ba514a6b604128474763e83de55b548 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Thu, 2 Mar 2017 01:10:36 +0530 Subject: [PATCH] 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