Merge pull request #807 from ilovezfs/partial_order_compliance

audit: detect partial component order compliance

Library/Homebrew/cmd/audit.rb

@@ -72,7 +72,7 @@ module Homebrew
       fa.audit
 
       next if fa.problems.empty?
-
+      fa.problems
       formula_count += 1
       problem_count += fa.problems.size
       problem_lines = fa.problems.map { |p| "* #{p.chomp.gsub("\n", "\n    ")}" }
@@ -121,10 +121,15 @@ class FormulaText
     @text.include? s
   end
 
-  def line_number(regex)
-    index = @lines.index { |line| line =~ regex }
+  def line_number(regex, skip = 0)
+    index = @lines.drop(skip).index { |line| line =~ regex }
     index ? index + 1 : nil
   end
+
+  def reverse_line_number(regex)
+    index = @lines.reverse.index { |line| line =~ regex }
+    index ? @lines.count - index : nil
+  end
 end
 
 class FormulaAuditor
@@ -171,35 +176,14 @@ class FormulaAuditor
     end
   end
 
-  def audit_file
-    # Under normal circumstances (umask 0022), we expect a file mode of 644. If
-    # the user's umask is more restrictive, respect that by masking out the
-    # corresponding bits. (The also included 0100000 flag means regular file.)
-    wanted_mode = 0100644 & ~File.umask
-    actual_mode = formula.path.stat.mode
-    unless actual_mode == wanted_mode
-      problem format("Incorrect file permissions (%03o): chmod %03o %s",
-                     actual_mode & 0777, wanted_mode & 0777, formula.path)
-    end
-
-    if text.has_DATA? && !text.has_END?
-      problem "'DATA' was found, but no '__END__'"
-    end
-
-    if text.has_END? && !text.has_DATA?
-      problem "'__END__' was found, but 'DATA' is not used"
-    end
-
-    if text =~ /inreplace [^\n]* do [^\n]*\n[^\n]*\.gsub![^\n]*\n\ *end/m
-      problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
-    end
-
-    unless text.has_trailing_newline?
-      problem "File should end with a newline"
-    end
-
-    return unless @strict
+  def component_problem(before, after, offset = 0)
+    problem "`#{before[1]}` (line #{before[0] + offset}) should be put before `#{after[1]}` (line #{after[0] + offset})"
+  end
 
+  # scan in the reverse direction for remaining problems but report problems
+  # in the forward direction so that contributors don't reverse the order of
+  # lines in the file simply by following instructions
+  def audit_components(reverse = true, previous_pair = nil)
     component_list = [
       [/^  include Language::/,            "include directive"],
       [/^  desc ["'][\S\ ]+["']/,          "desc"],
@@ -226,17 +210,67 @@ class FormulaAuditor
       [/^  (plist_options|def plist)/,     "plist block"],
       [/^  test do/,                       "test block"],
     ]
-
+    if previous_pair
+      previous_before = previous_pair[0]
+      previous_after = previous_pair[1]
+    end
+    offset = (previous_after && previous_after[0] && previous_after[0] >= 1) ? previous_after[0] - 1 : 0
     present = component_list.map do |regex, name|
-      lineno = text.line_number regex
+      lineno = if reverse
+        text.reverse_line_number regex
+      else
+        text.line_number regex, offset
+      end
       next unless lineno
       [lineno, name]
     end.compact
+    no_problem = true
     present.each_cons(2) do |c1, c2|
-      unless c1[0] < c2[0]
-        problem "`#{c1[1]}` (line #{c1[0]}) should be put before `#{c2[1]}` (line #{c2[0]})"
+      if reverse
+        # scan in the forward direction from the offset
+        audit_components(false, [c1, c2]) if c1[0] > c2[0] # at least one more offense
+      elsif c1[0] > c2[0] && (offset == 0 || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
+        component_problem c1, c2, offset
+        no_problem = false
       end
     end
+    if no_problem && previous_pair
+      component_problem previous_before, previous_after
+    end
+    present
+  end
+
+  def audit_file
+    # Under normal circumstances (umask 0022), we expect a file mode of 644. If
+    # the user's umask is more restrictive, respect that by masking out the
+    # corresponding bits. (The also included 0100000 flag means regular file.)
+    wanted_mode = 0100644 & ~File.umask
+    actual_mode = formula.path.stat.mode
+    unless actual_mode == wanted_mode
+      problem format("Incorrect file permissions (%03o): chmod %03o %s",
+                     actual_mode & 0777, wanted_mode & 0777, formula.path)
+    end
+
+    if text.has_DATA? && !text.has_END?
+      problem "'DATA' was found, but no '__END__'"
+    end
+
+    if text.has_END? && !text.has_DATA?
+      problem "'__END__' was found, but 'DATA' is not used"
+    end
+
+    if text =~ /inreplace [^\n]* do [^\n]*\n[^\n]*\.gsub![^\n]*\n\ *end/m
+      problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
+    end
+
+    unless text.has_trailing_newline?
+      problem "File should end with a newline"
+    end
+
+    return unless @strict
+
+    present = audit_components
+
     present.map!(&:last)
     if present.include?("stable block")
       %w[url checksum mirror].each do |component|