diff options
Diffstat (limited to 'Library')
| -rw-r--r-- | Library/Homebrew/dev-cmd/audit.rb | 36 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/extend/formula_cop.rb | 32 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/lines_cop.rb | 54 | ||||
| -rw-r--r-- | Library/Homebrew/test/dev-cmd/audit_spec.rb | 22 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/lines_cop_spec.rb | 151 |
5 files changed, 233 insertions, 62 deletions
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 170fb6d5f..5e1220e23 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -808,39 +808,6 @@ class FormulaAuditor end def line_problems(line, _lineno) - if line =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/ - problem "Use a space in class inheritance: class Foo < #{Regexp.last_match(1)}" - end - - # Commented-out cmake support from default template - problem "Commented cmake call found" if line.include?('# system "cmake') - - # Comments from default template - [ - "# PLEASE REMOVE", - "# Documentation:", - "# if this fails, try separate make/make install steps", - "# The URL of the archive", - "## Naming --", - "# if your formula requires any X11/XQuartz components", - "# if your formula fails when building in parallel", - "# Remove unrecognized options if warned by configure", - ].each do |comment| - next unless line.include?(comment) - problem "Please remove default template comments" - end - - # FileUtils is included in Formula - # encfs modifies a file with this name, so check for some leading characters - if line =~ %r{[^'"/]FileUtils\.(\w+)} - problem "Don't need 'FileUtils.' before #{Regexp.last_match(1)}." - end - - # Check for long inreplace block vars - if line =~ /inreplace .* do \|(.{2,})\|/ - problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{Regexp.last_match(1)}|\"." - end - # Check for string interpolation of single values. if line =~ /(system|inreplace|gsub!|change_make_var!).*[ ,]"#\{([\w.]+)\}"/ problem "Don't need to interpolate \"#{Regexp.last_match(2)}\" with #{Regexp.last_match(1)}" @@ -890,9 +857,6 @@ class FormulaAuditor end end - # Commented-out depends_on - problem "Commented-out dep #{Regexp.last_match(1)}" if line =~ /#\s*depends_on\s+(.+)\s*$/ - if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/ problem "Use \"if build.#{Regexp.last_match(1).downcase}?\" instead" end diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index 862664010..7844f7bf2 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -86,9 +86,13 @@ module RuboCop end # Returns an array of method call nodes matching method_name in every descendant of node - def find_every_method_call_by_name(node, method_name) + # Returns every method call if no method_name is passed + def find_every_method_call_by_name(node, method_name = nil) return if node.nil? - node.each_descendant(:send).select { |method_node| method_name == method_node.method_name } + node.each_descendant(:send).select do |method_node| + method_name.nil? || + method_name == method_node.method_name + end end # Given a method_name and arguments, yields to a block with @@ -108,7 +112,7 @@ module RuboCop def find_instance_method_call(node, instance, method_name) methods = find_every_method_call_by_name(node, method_name) methods.each do |method| - next unless method.receiver.const_name == instance + next unless method.receiver && method.receiver.const_name == instance @offense_source_range = method.source_range @offensive_node = method yield method @@ -202,9 +206,15 @@ module RuboCop end # Returns an array of block nodes of any depth below node in AST + # If a block is given then yields matching block node to the block! def find_all_blocks(node, block_name) return if node.nil? - node.each_descendant(:block).select { |block_node| block_name == block_node.method_name } + blocks = node.each_descendant(:block).select { |block_node| block_name == block_node.method_name } + return blocks unless block_given? + blocks.each do |block_node| + offending_node(block_node) + yield block_node + end end # Returns a method definition node with method_name @@ -316,6 +326,15 @@ module RuboCop end end + # Yields to a block with comment text as parameter + def audit_comments + @processed_source.comments.each do |comment_node| + @offensive_node = comment_node + @offense_source_range = :expression + yield comment_node.text + 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 @@ -326,6 +345,11 @@ module RuboCop 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 diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb index 22039869b..ed50ba49c 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/Library/Homebrew/rubocops/lines_cop.rb @@ -15,6 +15,60 @@ module RuboCop problem ":tex is deprecated" if depends_on?(:tex) end end + + class ClassInheritance < FormulaCop + def audit_formula(_node, class_node, parent_class_node, _body_node) + begin_pos = start_column(parent_class_node) + end_pos = end_column(class_node) + return unless begin_pos-end_pos != 3 + problem "Use a space in class inheritance: class #{@formula_name} < #{class_name(parent_class_node)}" + end + end + + class Comments < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, _body_node) + audit_comments do |comment| + [ + "# PLEASE REMOVE", + "# Documentation:", + "# if this fails, try separate make/make install steps", + "# The URL of the archive", + "## Naming --", + "# if your formula requires any X11/XQuartz components", + "# if your formula fails when building in parallel", + "# Remove unrecognized options if warned by configure", + '# system "cmake', + ].each do |template_comment| + next unless comment.include?(template_comment) + problem "Please remove default template comments" + break + end + end + + audit_comments do |comment| + # Commented-out depends_on + next unless comment =~ /#\s*depends_on\s+(.+)\s*$/ + problem "Commented-out dependency #{Regexp.last_match(1)}" + end + end + end + + class Miscellaneous < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + # FileUtils is included in Formula + # encfs modifies a file with this name, so check for some leading characters + find_instance_method_call(body_node, "FileUtils", nil) do |method_node| + problem "Don't need 'FileUtils.' before #{method_node.method_name}" + end + + # Check for long inreplace block vars + find_all_blocks(body_node, :inreplace) do |node| + block_arg = node.arguments.children.first + next unless block_arg.source.size>1 + problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{block_arg.source}|\"." + end + 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 f2d8a8e7c..037865fdf 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -263,28 +263,6 @@ describe FormulaAuditor do expect(fa.problems.shift) .to eq('Use pkgshare instead of (share/"foolibc++")') end - - specify "no space in class inheritance" do - fa = formula_auditor "foo", <<-EOS.undent - class Foo<Formula - url '/foo-1.0.tgz' - end - EOS - - fa.line_problems "class Foo<Formula", 1 - expect(fa.problems.shift) - .to eq("Use a space in class inheritance: class Foo < Formula") - end - - specify "default template" do - fa = formula_auditor "foo", "class Foo < Formula; url '/foo-1.0.tgz'; end" - - fa.line_problems '# system "cmake", ".", *std_cmake_args', 3 - expect(fa.problems.shift).to eq("Commented cmake call found") - - fa.line_problems "# PLEASE REMOVE", 3 - expect(fa.problems.shift).to eq("Please remove default template comments") - end end describe "#audit_github_repository" do diff --git a/Library/Homebrew/test/rubocops/lines_cop_spec.rb b/Library/Homebrew/test/rubocops/lines_cop_spec.rb index c865e1480..b0ed8f4d1 100644 --- a/Library/Homebrew/test/rubocops/lines_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_cop_spec.rb @@ -51,3 +51,154 @@ describe RuboCop::Cop::FormulaAudit::Lines do end end end + +describe RuboCop::Cop::FormulaAudit::ClassInheritance do + subject(:cop) { described_class.new } + + context "When auditing lines" do + it "with no space in class inheritance" do + source = <<-EOS.undent + class Foo<Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + end + EOS + + expected_offenses = [{ message: "Use a space in class inheritance: class Foo < Formula", + severity: :convention, + line: 1, + column: 10, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + end +end + +describe RuboCop::Cop::FormulaAudit::Comments do + subject(:cop) { described_class.new } + + context "When auditing formula" do + it "with commented cmake call" do + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + # system "cmake", ".", *std_cmake_args + end + EOS + + expected_offenses = [{ message: "Please remove default template comments", + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "with default template comments" do + source = <<-EOS.undent + class Foo < Formula + # PLEASE REMOVE + desc "foo" + url 'http://example.com/foo-1.0.tgz' + end + EOS + + expected_offenses = [{ message: "Please remove default template comments", + severity: :convention, + line: 2, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "with commented out depends_on" do + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + # depends_on "foo" + end + EOS + + expected_offenses = [{ message: 'Commented-out dependency "foo"', + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + end +end + +describe RuboCop::Cop::FormulaAudit::Miscellaneous do + subject(:cop) { described_class.new } + + context "When auditing formula" do + it "with FileUtils" do + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + FileUtils.mv "hello" + end + EOS + + expected_offenses = [{ message: "Don't need 'FileUtils.' before mv", + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "with long inreplace block vars" do + source = <<-EOS.undent + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + inreplace "foo" do |longvar| + somerandomCall(longvar) + end + end + EOS + + expected_offenses = [{ message: "\"inreplace <filenames> do |s|\" is preferred over \"|longvar|\".", + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(cop, source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + end +end |