dev-cmd/audit: fix Rubocop warnings.

This commit is contained in:
Mike McQuaid 2016-09-11 17:41:51 +01:00
parent 90d16a8f86
commit 264afb67df
2 changed files with 39 additions and 46 deletions

View File

@ -101,20 +101,20 @@ class FormulaText
@text.split("\n__END__").first @text.split("\n__END__").first
end end
def has_DATA? def data?
/^[^#]*\bDATA\b/ =~ @text /^[^#]*\bDATA\b/ =~ @text
end end
def has_END? def end?
/^__END__$/ =~ @text /^__END__$/ =~ @text
end end
def has_trailing_newline? def trailing_newline?
/\Z\n/ =~ @text /\Z\n/ =~ @text
end end
def =~(regex) def =~(other)
regex =~ @text other =~ @text
end end
def include?(s) def include?(s)
@ -152,15 +152,15 @@ class FormulaAuditor
smake smake
sphinx-doc sphinx-doc
swig swig
] ].freeze
FILEUTILS_METHODS = FileUtils.singleton_methods(false).map { |m| Regexp.escape(m) }.join "|" FILEUTILS_METHODS = FileUtils.singleton_methods(false).map { |m| Regexp.escape(m) }.join "|"
def initialize(formula, options = {}) def initialize(formula, options = {})
@formula = formula @formula = formula
@new_formula = !!options[:new_formula] @new_formula = options[:new_formula]
@strict = !!options[:strict] @strict = options[:strict]
@online = !!options[:online] @online = options[:online]
# Accept precomputed style offense results, for efficiency # Accept precomputed style offense results, for efficiency
@style_offenses = options[:style_offenses] @style_offenses = options[:style_offenses]
@problems = [] @problems = []
@ -214,7 +214,7 @@ class FormulaAuditor
previous_before = previous_pair[0] previous_before = previous_pair[0]
previous_after = previous_pair[1] previous_after = previous_pair[1]
end end
offset = (previous_after && previous_after[0] && previous_after[0] >= 1) ? previous_after[0] - 1 : 0 offset = previous_after && previous_after[0] && previous_after[0] >= 1 ? previous_after[0] - 1 : 0
present = component_list.map do |regex, name| present = component_list.map do |regex, name|
lineno = if reverse lineno = if reverse
text.reverse_line_number regex text.reverse_line_number regex
@ -229,7 +229,7 @@ class FormulaAuditor
if reverse if reverse
# scan in the forward direction from the offset # scan in the forward direction from the offset
audit_components(false, [c1, c2]) if c1[0] > c2[0] # at least one more offense 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]) elsif c1[0] > c2[0] && (offset.zero? || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
component_problem c1, c2, offset component_problem c1, c2, offset
no_problem = false no_problem = false
end end
@ -251,11 +251,11 @@ class FormulaAuditor
actual_mode & 0777, wanted_mode & 0777, formula.path) actual_mode & 0777, wanted_mode & 0777, formula.path)
end end
if text.has_DATA? && !text.has_END? if text.data? && !text.end?
problem "'DATA' was found, but no '__END__'" problem "'DATA' was found, but no '__END__'"
end end
if text.has_END? && !text.has_DATA? if text.end? && !text.data?
problem "'__END__' was found, but 'DATA' is not used" problem "'__END__' was found, but 'DATA' is not used"
end end
@ -263,7 +263,7 @@ class FormulaAuditor
problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)." problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
end end
unless text.has_trailing_newline? unless text.trailing_newline?
problem "File should end with a newline" problem "File should end with a newline"
end end
@ -328,8 +328,8 @@ class FormulaAuditor
return return
end end
@@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names). @@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names)
reduce(Hash.new) do |name_map, tap_formula_full_name| .each_with_object({}) do |tap_formula_full_name, name_map|
tap_formula_name = tap_formula_full_name.split("/").last tap_formula_name = tap_formula_full_name.split("/").last
name_map[tap_formula_name] ||= [] name_map[tap_formula_name] ||= []
name_map[tap_formula_name] << tap_formula_full_name name_map[tap_formula_name] << tap_formula_full_name
@ -396,14 +396,6 @@ class FormulaAuditor
end end
case dep.name case dep.name
when *BUILD_TIME_DEPS
next if dep.build? || dep.run?
problem <<-EOS.undent
#{dep} dependency should be
depends_on "#{dep}" => :build
Or if it is indeed a runtime dependency
depends_on "#{dep}" => :run
EOS
when "git" when "git"
problem "Don't use git as a dependency" problem "Don't use git as a dependency"
when "mercurial" when "mercurial"
@ -419,6 +411,9 @@ class FormulaAuditor
where "1.8" is the minimum version of Ruby required. where "1.8" is the minimum version of Ruby required.
EOS EOS
when "open-mpi", "mpich" when "open-mpi", "mpich"
problem <<-EOS.undent
when *BUILD_TIME_DEPS
next if dep.build? || dep.run?
problem <<-EOS.undent problem <<-EOS.undent
There are multiple conflicting ways to install MPI. Use an MPIRequirement: There are multiple conflicting ways to install MPI. Use an MPIRequirement:
depends_on :mpi => [<lang list>] depends_on :mpi => [<lang list>]
@ -452,13 +447,12 @@ class FormulaAuditor
problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`." problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`."
end end
if o.name =~ /^with(out)?-(?:checks?|tests)$/ next unless o.name =~ /^with(out)?-(?:checks?|tests)$/
unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) } unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`." problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
end end
end end
end end
end
def audit_desc def audit_desc
# For now, only check the description when using `--strict` # For now, only check the description when using `--strict`
@ -667,15 +661,14 @@ class FormulaAuditor
attributes.each do |attribute| attributes.each do |attribute|
attributes_for_version = attributes_map[attribute][formula.version] attributes_for_version = attributes_map[attribute][formula.version]
if !attributes_for_version.empty? next if attributes_for_version.empty?
if formula.send(attribute) < attributes_for_version.max if formula.send(attribute) < attributes_for_version.max
problem "#{attribute} should not decrease" problem "#{attribute} should not decrease"
end end
end end
end
revision_map = attributes_map[:revision] revision_map = attributes_map[:revision]
if formula.revision != 0 if formula.revision.nonzero?
if formula.stable if formula.stable
if revision_map[formula.stable.version].empty? # check stable spec if revision_map[formula.stable.version].empty? # check stable spec
problem "'revision #{formula.revision}' should be removed" problem "'revision #{formula.revision}' should be removed"
@ -772,7 +765,7 @@ class FormulaAuditor
# FileUtils is included in Formula # FileUtils is included in Formula
# encfs modifies a file with this name, so check for some leading characters # encfs modifies a file with this name, so check for some leading characters
if line =~ /[^'"\/]FileUtils\.(\w+)/ if line =~ %r{[^'"/]FileUtils\.(\w+)}
problem "Don't need 'FileUtils.' before #{$1}." problem "Don't need 'FileUtils.' before #{$1}."
end end
@ -1038,7 +1031,7 @@ class FormulaAuditor
end end
def quote_dep(dep) def quote_dep(dep)
Symbol === dep ? dep.inspect : "'#{dep}'" dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
end end
def audit_check_output(output) def audit_check_output(output)

View File

@ -29,9 +29,9 @@ class FormulaTextTests < Homebrew::TestCase
def test_simple_valid_formula def test_simple_valid_formula
ft = formula_text "valid", 'url "http://www.example.com/valid-1.0.tar.gz"' ft = formula_text "valid", 'url "http://www.example.com/valid-1.0.tar.gz"'
refute ft.has_DATA?, "The formula should not have DATA" refute ft.data?, "The formula should not have DATA"
refute ft.has_END?, "The formula should not have __END__" refute ft.end?, "The formula should not have __END__"
assert ft.has_trailing_newline?, "The formula should have a trailing newline" assert ft.trailing_newline?, "The formula should have a trailing newline"
assert ft =~ /\burl\b/, "The formula should match 'url'" assert ft =~ /\burl\b/, "The formula should match 'url'"
assert_nil ft.line_number(/desc/), "The formula should not match 'desc'" assert_nil ft.line_number(/desc/), "The formula should not match 'desc'"
@ -41,17 +41,17 @@ class FormulaTextTests < Homebrew::TestCase
def test_trailing_newline def test_trailing_newline
ft = formula_text "newline" ft = formula_text "newline"
assert ft.has_trailing_newline?, "The formula must have a trailing newline" assert ft.trailing_newline?, "The formula must have a trailing newline"
end end
def test_has_data def test_has_data
ft = formula_text "data", "patch :DATA" ft = formula_text "data", "patch :DATA"
assert ft.has_DATA?, "The formula must have DATA" assert ft.data?, "The formula must have DATA"
end end
def test_has_end def test_has_end
ft = formula_text "end", "", :patch => "__END__\na patch here" ft = formula_text "end", "", :patch => "__END__\na patch here"
assert ft.has_END?, "The formula must have __END__" assert ft.end?, "The formula must have __END__"
assert_equal "class End < Formula\n \nend", ft.without_patch assert_equal "class End < Formula\n \nend", ft.without_patch
end end
end end