From affd9055c2aaaf3535ed1cb778b65a4bee5ef0e2 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Sun, 30 Jul 2017 20:14:59 +0530 Subject: audit: Port classname and template comments audit rules from line_problems method to rubocop --- Library/Homebrew/dev-cmd/audit.rb | 25 ----------------- Library/Homebrew/rubocops/extend/formula_cop.rb | 14 ++++++++++ Library/Homebrew/rubocops/lines_cop.rb | 37 +++++++++++++++++++++++++ Library/Homebrew/test/dev-cmd/audit_spec.rb | 22 --------------- 4 files changed, 51 insertions(+), 47 deletions(-) (limited to 'Library') diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 1f07fa89e..323b37170 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -809,28 +809,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+)} @@ -891,9 +869,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 4be0c0fe3..e7a23ecae 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -302,6 +302,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 @@ -312,6 +321,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..abd46162f 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/Library/Homebrew/rubocops/lines_cop.rb @@ -15,6 +15,43 @@ 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 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 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)}" diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index e7a23ecae..f8864538a 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -85,9 +85,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 @@ -107,7 +111,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 @@ -188,9 +192,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 diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb index abd46162f..ed50ba49c 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/Library/Homebrew/rubocops/lines_cop.rb @@ -52,6 +52,23 @@ module RuboCop 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 do |s|\" is preferred over \"|#{block_arg.source}|\"." + end + end + end end end end -- cgit v1.2.3 From 781da49df87f9f2ddbb3f812e3d53963500cc423 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Wed, 2 Aug 2017 23:49:51 +0530 Subject: audit: Add tests for audit rules ported from line_problems method to rubocops --- Library/Homebrew/test/rubocops/lines_cop_spec.rb | 151 +++++++++++++++++++++++ 1 file changed, 151 insertions(+) (limited to 'Library') 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 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 -- cgit v1.2.3