diff options
Diffstat (limited to 'Library')
| -rw-r--r-- | Library/.rubocop.yml | 3 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/audit.rb | 34 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops.rb | 1 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/bottle_block_cop.rb | 27 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/extend/formula_cop.rb | 144 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/formula_desc_cop.rb | 48 | ||||
| -rw-r--r-- | Library/Homebrew/test/Gemfile | 1 | ||||
| -rw-r--r-- | Library/Homebrew/test/Gemfile.lock | 14 | ||||
| -rw-r--r-- | Library/Homebrew/test/dev-cmd/audit_spec.rb | 30 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb | 67 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb | 121 |
11 files changed, 410 insertions, 80 deletions
diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 314467ef0..a782c1117 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -11,6 +11,9 @@ require: ./Homebrew/rubocops.rb Homebrew/CorrectBottleBlock: Enabled: true +Homebrew/FormulaDesc: + Enabled: true + Metrics/AbcSize: Enabled: false diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 5f4eda08e..ba6c3f333 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -586,39 +586,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 @@ -1281,7 +1248,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 1a28dd213..3625f2004 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/bottle_block_cop.rb b/Library/Homebrew/rubocops/bottle_block_cop.rb index 55eb55152..4d7a94461 100644 --- a/Library/Homebrew/rubocops/bottle_block_cop.rb +++ b/Library/Homebrew/rubocops/bottle_block_cop.rb @@ -1,16 +1,19 @@ +require_relative "./extend/formula_cop" + module RuboCop module Cop module Homebrew - class CorrectBottleBlock < Cop - MSG = "Use rebuild instead of revision in bottle block".freeze + # This cop audits `bottle` block in Formulae + # + # - `rebuild` should be used instead of `revision` in `bottle` block - def on_block(node) - return if block_length(node).zero? - method, _args, body = *node - _keyword, method_name = *method + class CorrectBottleBlock < FormulaCop + MSG = "Use rebuild instead of revision in bottle block".freeze - return unless method_name == :bottle - check_revision?(body) + def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node) + bottle = find_block(formula_class_body_node, :bottle) + return if bottle.nil? || block_size(bottle).zero? + problem "Use rebuild instead of revision in bottle block" if method_called?(bottle, :revision) end private @@ -22,14 +25,6 @@ module RuboCop corrector.remove(node.source_range) end end - - def check_revision?(body) - body.children.each do |method_call_node| - _receiver, method_name, _args = *method_call_node - next unless method_name == :revision - add_offense(method_call_node, :expression) - end - end end end end diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb new file mode 100644 index 000000000..49108bd48 --- /dev/null +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -0,0 +1,144 @@ +module RuboCop + module Cop + module Homebrew + class FormulaCop < Cop + @registry = Cop.registry + + def on_class(node) + # This method is called by RuboCop and is the main entry point + file_path = processed_source.buffer.name + return unless file_path_allowed?(file_path) + class_node, parent_class_node, body = *node + return unless formula_class?(parent_class_node) + return unless respond_to?(:audit_formula) + @formula_name = class_name(class_node) + audit_formula(node, class_node, parent_class_node, body) + end + + def regex_match_group(node, pattern) + # Checks for regex match of pattern in the node and + # Sets the appropriate instance variables to report the match + string_repr = string_content(node) + match_object = string_repr.match(pattern) + return unless match_object + node_begin_pos = start_column(node) + line_begin_pos = line_start_column(node) + @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1 + @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 + + def find_node_method_by_name(node, method_name) + # Returns method_node matching method_name + return if node.nil? + node.each_child_node(:send) do |method_node| + next unless method_node.method_name == method_name + @offensive_node = method_node + @offense_source_range = method_node.source_range + return method_node + end + # If not found then, parent node becomes the offensive node + @offensive_node = node.parent + @offense_source_range = node.parent.source_range + nil + end + + def find_block(node, block_name) + # Returns a block named block_name inside node + return if node.nil? + node.each_child_node(:block) do |block_node| + next if block_node.method_name != block_name + @offensive_node = block_node + @offense_source_range = block_node.source_range + return block_node + end + # If not found then, parent node becomes the offensive node + @offensive_node = node.parent + @offense_source_range = node.parent.source_range + nil + end + + def method_called?(node, method_name) + # Check if a method is called inside a block + block_body = node.children[2] + block_body.each_child_node(:send) do |call_node| + next unless call_node.method_name == method_name + @offensive_node = call_node + @offense_source_range = call_node.source_range + return true + end + false + end + + def parameters(method_node) + # Returns the array of arguments of the method_node + return unless method_node.send_type? + method_node.method_args + end + + def line_start_column(node) + # Returns the begin position of the node's line in source code + node.source_range.source_buffer.line_range(node.loc.line).begin_pos + end + + def start_column(node) + # Returns the begin position of the node in source code + node.source_range.begin_pos + end + + def line_number(node) + # Returns the line number of the node + node.loc.line + end + + def class_name(node) + # Returns the class node's name, nil if not a class node + @offensive_node = node + @offense_source_range = node.source_range + node.const_name + end + + def size(node) + # Returns the node size in the source code + node.source_range.size + end + + def block_size(block) + # Returns the block length of the block node + block_length(block) + end + + def source_buffer(node) + # Source buffer is required as an argument to report style violations + node.source_range.source_buffer + end + + def string_content(node) + # Returns the string representation if node is of type str + node.str_content if node.type == :str + end + + def problem(msg) + add_offense(@offensive_node, @offense_source_range, msg) + end + + private + + def formula_class?(parent_class_node) + parent_class_node && parent_class_node.const_name == "Formula" + end + + def file_path_allowed?(file_path) + paths_to_exclude = [%r{/Library/Homebrew/compat/}, + %r{/Library/Homebrew/test/}] + return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs + file_path !~ Regexp.union(paths_to_exclude) + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb new file mode 100644 index 000000000..7d69a48e7 --- /dev/null +++ b/Library/Homebrew/rubocops/formula_desc_cop.rb @@ -0,0 +1,48 @@ +require_relative "./extend/formula_cop" +require_relative "../extend/string" + +module RuboCop + module Cop + module Homebrew + # This cop audits `desc` in Formulae + # + # - 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 FormulaDesc < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body) + desc_call = find_node_method_by_name(body, :desc) + + if desc_call.nil? + problem "Formula should have a desc (Description)." + return + end + + 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 + + # Check if command-line is wrongly used in formula's desc + if match = regex_match_group(desc, /(command ?line)/i) + problem "Description should use \"command-line\" instead of \"#{match}\"" + end + + if match = regex_match_group(desc, /^(an?)\s/i) + problem "Description shouldn't start with an indefinite article (#{match})" + 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}/i) + end + end + end + end +end diff --git a/Library/Homebrew/test/Gemfile b/Library/Homebrew/test/Gemfile index bc9bccfbc..f3c16c710 100644 --- a/Library/Homebrew/test/Gemfile +++ b/Library/Homebrew/test/Gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" gem "parallel_tests" gem "rspec" +gem "rubocop" gem "rspec-its", require: false gem "rspec-wait", require: false diff --git a/Library/Homebrew/test/Gemfile.lock b/Library/Homebrew/test/Gemfile.lock index 64561be71..4d4eefd7d 100644 --- a/Library/Homebrew/test/Gemfile.lock +++ b/Library/Homebrew/test/Gemfile.lock @@ -1,6 +1,7 @@ GEM remote: https://rubygems.org/ specs: + ast (2.3.0) codecov (0.1.9) json simplecov @@ -11,6 +12,10 @@ GEM parallel (1.10.0) parallel_tests (2.13.0) parallel + parser (2.4.0.0) + ast (~> 2.2) + powerpack (0.1.1) + rainbow (2.2.1) rspec (3.5.0) rspec-core (~> 3.5.0) rspec-expectations (~> 3.5.0) @@ -29,11 +34,19 @@ GEM rspec-support (3.5.0) rspec-wait (0.0.9) rspec (>= 3, < 4) + rubocop (0.47.1) + parser (>= 2.3.3.1, < 3.0) + powerpack (~> 0.1) + rainbow (>= 1.99.1, < 3.0) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) + ruby-progressbar (1.8.1) simplecov (0.13.0) docile (~> 1.1.0) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.0) + unicode-display_width (1.1.3) url (0.3.2) PLATFORMS @@ -45,6 +58,7 @@ DEPENDENCIES rspec rspec-its rspec-wait + rubocop simplecov BUNDLED WITH diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index ec1a34fb4..a6bb22837 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 diff --git a/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb new file mode 100644 index 000000000..5be2d6cf5 --- /dev/null +++ b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb @@ -0,0 +1,67 @@ +require "rubocop" +require "rubocop/rspec/support" +require_relative "../../extend/string" +require_relative "../../rubocops/bottle_block_cop" + +describe RuboCop::Cop::Homebrew::CorrectBottleBlock do + subject(:cop) { described_class.new } + + context "When auditing Bottle Block" do + it "When there is revision in bottle block" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + bottle do + cellar :any + revision 2 + end + end + EOS + + expected_offenses = [{ message: "Use rebuild instead of revision in bottle block", + severity: :convention, + line: 5, + column: 4, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + 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]) + end + end + + context "When auditing Bottle Block with auto correct" do + it "When there is revision in bottle block" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + bottle do + cellar :any + revision 2 + end + end + EOS + corrected_source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + bottle do + cellar :any + rebuild 2 + end + end + EOS + + new_source = autocorrect_source(cop, source) + expect(new_source).to eq(corrected_source) + 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 new file mode 100644 index 000000000..04c4c27da --- /dev/null +++ b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb @@ -0,0 +1,121 @@ +require "rubocop" +require "rubocop/rspec/support" +require_relative "../../extend/string" +require_relative "../../rubocops/formula_desc_cop" + +describe RuboCop::Cop::Homebrew::FormulaDesc do + subject(:cop) { described_class.new } + + context "When auditing formula desc" do + it "When there is no desc" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + end + EOS + + expected_offenses = [{ message: "Formula should have a desc (Description).", + severity: :convention, + line: 1, + column: 0, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "When desc is too long" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + desc '#{"bar"*30}' + end + EOS + + msg = <<-EOS.undent + Description is too long. "name: desc" should be less than 80 characters. + Length is calculated as Foo + desc. (currently 95) + EOS + expected_offenses = [{ message: msg, + severity: :convention, + line: 3, + column: 2, + source: source }] + + inspect_source(cop, source) + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "When wrong \"command-line\" usage in desc" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + desc 'command line' + end + EOS + + expected_offenses = [{ message: "Description should use \"command-line\" instead of \"command line\"", + 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 article is used in desc" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + desc 'An ' + end + EOS + + expected_offenses = [{ message: "Description shouldn't start with an indefinite article (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 formula name is in desc" do + source = <<-EOS.undent + class Foo < Formula + url 'http://example.com/foo-1.0.tgz' + desc 'Foo' + end + EOS + + expected_offenses = [{ message: "Description shouldn't include the formula name", + 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 + + 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]) + end + end +end |