diff options
Diffstat (limited to 'Library')
| -rw-r--r-- | Library/.rubocop.yml | 4 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/audit.rb | 168 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/extend/formula_cop.rb | 39 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/lines_cop.rb | 259 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/lines_cop_spec.rb | 1133 |
5 files changed, 1291 insertions, 312 deletions
diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index d1a3860db..ca7ff4dc8 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -123,6 +123,10 @@ Style/Documentation: Style/Encoding: Enabled: true +# use spaces for indentation; detect tabs +Layout/Tab: + Enabled: true + # dashes in filenames are typical Naming/FileName: Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/ diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index ec26eb9cd..bfe4dbc00 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -820,168 +820,12 @@ class FormulaAuditor problem "\"(#{Regexp.last_match(1)}...#{Regexp.last_match(2)})\" should be \"(#{Regexp.last_match(3).downcase}+...)\"" end - if line =~ /((man)\s*\+\s*(['"])(man[1-8])(['"]))/ - problem "\"#{Regexp.last_match(1)}\" should be \"#{Regexp.last_match(4)}\"" - end - - # Prefer formula path shortcuts in strings - if line =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share|Frameworks))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2).downcase}}\"" - end - - if line =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\"" - end - - if line =~ %r[((\#\{share\}/(man)))[/'"]] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\"" - end - - if line =~ %r[(\#\{prefix\}/share/(info|man))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2)}}\"" - end - - if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+:(lua|perl|python|ruby)(\d*)/ - problem "#{Regexp.last_match(2)} modules should be vendored rather than use deprecated `depends_on \"#{Regexp.last_match(1)}\" => :#{Regexp.last_match(2)}#{Regexp.last_match(3)}`" - end - - if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+(.*)/ - dep = Regexp.last_match(1) - Regexp.last_match(2).split(" ").map do |o| - break if ["if", "unless"].include?(o) - next unless o =~ /^\[?['"](.*)['"]/ - problem "Dependency #{dep} should not use option #{Regexp.last_match(1)}" - end - end - - if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/ - problem "Use \"if build.#{Regexp.last_match(1).downcase}?\" instead" - end - problem "Use separate make calls" if line.include?("make && make") - problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/ - - if line.include?("ENV.java_cache") - problem "In-formula ENV.java_cache usage has been deprecated & should be removed." - end - - # Avoid hard-coding compilers - if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?(gcc|llvm-gcc|clang)['" ]} - problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{Regexp.last_match(3)}\"" - end - - if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]} - problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{Regexp.last_match(3)}\"" - end - - if line =~ /system\s+['"](env|export)(\s+|['"])/ - problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment" - end - - if line =~ /version == ['"]HEAD['"]/ - problem "Use 'build.head?' instead of inspecting 'version'" - end - - if line =~ /build\.include\?[\s\(]+['"]\-\-(.*)['"]/ - problem "Reference '#{Regexp.last_match(1)}' without dashes" - end - - if line =~ /build\.include\?[\s\(]+['"]with(out)?-(.*)['"]/ - problem "Use build.with#{Regexp.last_match(1)}? \"#{Regexp.last_match(2)}\" instead of build.include? 'with#{Regexp.last_match(1)}-#{Regexp.last_match(2)}'" - end - - if line =~ /build\.with\?[\s\(]+['"]-?-?with-(.*)['"]/ - problem "Don't duplicate 'with': Use `build.with? \"#{Regexp.last_match(1)}\"` to check for \"--with-#{Regexp.last_match(1)}\"" - end - - if line =~ /build\.without\?[\s\(]+['"]-?-?without-(.*)['"]/ - problem "Don't duplicate 'without': Use `build.without? \"#{Regexp.last_match(1)}\"` to check for \"--without-#{Regexp.last_match(1)}\"" - end - - if line =~ /unless build\.with\?(.*)/ - problem "Use if build.without?#{Regexp.last_match(1)} instead of unless build.with?#{Regexp.last_match(1)}" - end - - if line =~ /unless build\.without\?(.*)/ - problem "Use if build.with?#{Regexp.last_match(1)} instead of unless build.without?#{Regexp.last_match(1)}" - end - - if line =~ /(not\s|!)\s*build\.with?\?/ - problem "Don't negate 'build.with?': use 'build.without?'" - end - - if line =~ /(not\s|!)\s*build\.without?\?/ - problem "Don't negate 'build.without?': use 'build.with?'" - end - - if line =~ /ARGV\.(?!(debug\?|verbose\?|value[\(\s]))/ - problem "Use build instead of ARGV to check options" - end - - if line.include?("MACOS_VERSION") - problem "Use MacOS.version instead of MACOS_VERSION" - end - - if line.include?("MACOS_FULL_VERSION") - problem "Use MacOS.full_version instead of MACOS_FULL_VERSION" - end - - cats = %w[leopard snow_leopard lion mountain_lion].join("|") - if line =~ /MacOS\.(?:#{cats})\?/ - problem "\"#{$&}\" is deprecated, use a comparison to MacOS.version instead" - end - - if line =~ /depends_on [A-Z][\w:]+\.new$/ - problem "`depends_on` can take requirement classes instead of instances" - end - - if line =~ /^def (\w+).*$/ - problem "Define method #{Regexp.last_match(1).inspect} in the class body, not at the top-level" - end - - if line.include?("ENV.fortran") && !formula.requirements.map(&:class).include?(FortranRequirement) - problem "Use `depends_on :fortran` instead of `ENV.fortran`" - end - if line =~ /JAVA_HOME/i && !formula.requirements.map(&:class).include?(JavaRequirement) problem "Use `depends_on :java` to set JAVA_HOME" end - if line =~ /depends_on :(.+) (if.+|unless.+)$/ - conditional_dep_problems(Regexp.last_match(1).to_sym, Regexp.last_match(2), $&) - end - - if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/ - conditional_dep_problems(Regexp.last_match(1), Regexp.last_match(2), $&) - end - - if line =~ /(Dir\[("[^\*{},]+")\])/ - problem "#{Regexp.last_match(1)} is unnecessary; just use #{Regexp.last_match(2)}" - end - - if line =~ /system (["'](#{FILEUTILS_METHODS})["' ])/o - system = Regexp.last_match(1) - method = Regexp.last_match(2) - problem "Use the `#{method}` Ruby method instead of `system #{system}`" - end - - if line =~ /assert [^!]+\.include?/ - problem "Use `assert_match` instead of `assert ...include?`" - end - - if line =~ /(assert File\.exist\?|assert \(.*\)\.exist\?)/ - problem "Use `assert_predicate <path_to_file>, :exist?` instead of `#{Regexp.last_match(1)}`" - end - - if line =~ /(assert !File\.exist\?|assert !\(.*\)\.exist\?)/ - problem "Use `refute_predicate <path_to_file>, :exist?` instead of `#{Regexp.last_match(1)}`" - end - - if line =~ /(assert File\.executable\?|assert \(.*\)\.executable\?)/ - problem "Use `assert_predicate <path_to_file>, :executable?` instead of `#{Regexp.last_match(1)}`" - end - return unless @strict problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/ @@ -1028,18 +872,6 @@ class FormulaAuditor EOS end - def conditional_dep_problems(dep, condition, line) - quoted_dep = quote_dep(dep) - dep = Regexp.escape(dep.to_s) - - case condition - when /if build\.include\? ['"]with-#{dep}['"]$/, /if build\.with\? ['"]#{dep}['"]$/ - problem %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :optional") - when /unless build\.include\? ['"]without-#{dep}['"]$/, /unless build\.without\? ['"]#{dep}['"]$/ - problem %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :recommended") - end - end - def quote_dep(dep) dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'" end diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index 7531e62b7..7da4d0f10 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -56,6 +56,7 @@ module RuboCop # Returns all string nodes among the descendants of given node def find_strings(node) return [] if node.nil? + return node if node.str_type? node.each_descendant(:str) end @@ -101,7 +102,9 @@ module RuboCop def find_method_with_args(node, method_name, *args) methods = find_every_method_call_by_name(node, method_name) methods.each do |method| - yield method if parameters_passed?(method, *args) + next unless parameters_passed?(method, *args) + return true unless block_given? + yield method end end @@ -119,6 +122,7 @@ module RuboCop method.receiver.method_name != instance @offense_source_range = method.source_range @offensive_node = method + return true unless block_given? yield method end end @@ -167,6 +171,20 @@ module RuboCop type_match && name_match end + # Find CONSTANTs in the source + # if block given, yield matching nodes + def find_const(node, const_name) + return if node.nil? + node.each_descendant(:const) do |const_node| + next unless const_node.const_name == const_name + @offensive_node = const_node + @offense_source_range = const_node.source_range + yield const_node if block_given? + return true + end + nil + end + def_node_search :required_dependency?, <<~EOS (send nil :depends_on ({str sym} _)) EOS @@ -222,15 +240,17 @@ module RuboCop end # Returns a method definition node with method_name - def find_method_def(node, method_name) + # Returns first method def if method_name is nil + def find_method_def(node, method_name = nil) return if node.nil? node.each_child_node(:def) do |def_node| def_method_name = method_name(def_node) - next unless method_name == def_method_name + next unless method_name == def_method_name || method_name.nil? @offensive_node = def_node @offense_source_range = def_node.source_range return def_node end + return if node.parent.nil? # If not found then, parent node becomes the offensive node @offensive_node = node.parent @offense_source_range = node.parent.source_range @@ -250,11 +270,15 @@ module RuboCop end # Check if method_name is called among the direct children nodes in the given node + # Check if the node itself is the method def method_called?(node, method_name) + if node.send_type? && node.method_name == method_name + offending_node(node) + return true + end node.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 + offending_node(call_node) return true end false @@ -291,6 +315,11 @@ module RuboCop true end + # Check if negation is present in the given node + def negated?(node) + method_called?(node, :!) + end + # Return all the caveats' string nodes in an array def caveats_strings find_strings(find_method_def(@body, :caveats)) diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb index 4172aad96..9354f41f6 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/Library/Homebrew/rubocops/lines_cop.rb @@ -21,7 +21,7 @@ module RuboCop 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 #{class_name(class_node)} < #{class_name(parent_class_node)}" + problem "Use a space in class inheritance: class #{@formula_name.capitalize} < #{class_name(parent_class_node)}" end end @@ -53,6 +53,87 @@ module RuboCop end end + class AssertStatements < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + find_every_method_call_by_name(body_node, :assert).each do |method| + if method_called_ever?(method, :include?) && !method_called_ever?(method, :!) + problem "Use `assert_match` instead of `assert ...include?`" + end + + if method_called_ever?(method, :exist?) && !method_called_ever?(method, :!) + problem "Use `assert_predicate <path_to_file>, :exist?` instead of `#{method.source}`" + end + + if method_called_ever?(method, :exist?) && method_called_ever?(method, :!) + problem "Use `refute_predicate <path_to_file>, :exist?` instead of `#{method.source}`" + end + + if method_called_ever?(method, :executable?) && !method_called_ever?(method, :!) + problem "Use `assert_predicate <path_to_file>, :executable?` instead of `#{method.source}`" + end + end + end + end + + class OptionDeclarations < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + if find_method_def(body_node, :options) + problem "Use new-style option definitions" + end + + find_instance_method_call(body_node, :build, :without?) do |method| + next unless unless_modifier?(method.parent) + correct = method.source.gsub("out?", "?") + problem "Use if #{correct} instead of unless #{method.source}" + end + + find_instance_method_call(body_node, :build, :with?) do |method| + next unless unless_modifier?(method.parent) + correct = method.source.gsub("?", "out?") + problem "Use if #{correct} instead of unless #{method.source}" + end + + find_instance_method_call(body_node, :build, :with?) do |method| + next unless negated?(method.parent) + problem "Don't negate 'build.with?': use 'build.without?'" + end + + find_instance_method_call(body_node, :build, :without?) do |method| + next unless negated?(method.parent) + problem "Don't negate 'build.without?': use 'build.with?'" + end + + find_instance_method_call(body_node, :build, :without?) do |method| + arg = parameters(method).first + next unless match = regex_match_group(arg, /-?-?without-(.*)/) + problem "Don't duplicate 'without': Use `build.without? \"#{match[1]}\"` to check for \"--without-#{match[1]}\"" + end + + find_instance_method_call(body_node, :build, :with?) do |method| + arg = parameters(method).first + next unless match = regex_match_group(arg, /-?-?with-(.*)/) + problem "Don't duplicate 'with': Use `build.with? \"#{match[1]}\"` to check for \"--with-#{match[1]}\"" + end + + find_instance_method_call(body_node, :build, :include?) do |method| + arg = parameters(method).first + next unless match = regex_match_group(arg, /with(out)?-(.*)/) + problem "Use build.with#{match[1]}? \"#{match[2]}\" instead of build.include? 'with#{match[1]}-#{match[2]}'" + end + + find_instance_method_call(body_node, :build, :include?) do |method| + arg = parameters(method).first + next unless match = regex_match_group(arg, /\-\-(.*)/) + problem "Reference '#{match[1]}' without dashes" + end + end + + def unless_modifier?(node) + return false unless node.if_type? + node.modifier_form? && node.unless? + end + end + class Miscellaneous < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) # FileUtils is included in Formula @@ -81,18 +162,128 @@ module RuboCop end end + [:debug?, :verbose?, :value].each do |method_name| + find_instance_method_call(body_node, "ARGV", method_name) do + problem "Use build instead of ARGV to check options" + end + end + + find_instance_method_call(body_node, :man, :+) do |method| + next unless match = regex_match_group(parameters(method).first, /man[1-8]/) + problem "\"#{method.source}\" should be \"#{match[0]}\"" + end + + # Avoid hard-coding compilers + find_every_method_call_by_name(body_node, :system).each do |method| + param = parameters(method).first + if match = regex_match_group(param, %r{(/usr/bin/)?(gcc|llvm-gcc|clang)\s?}) + problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{match[2]}\"" + elsif match = regex_match_group(param, %r{(/usr/bin/)?((g|llvm-g|clang)\+\+)\s?}) + problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{match[2]}\"" + end + end + + find_instance_method_call(body_node, "ENV", :[]=) do |method| + param = parameters(method)[1] + if match = regex_match_group(param, %r{(/usr/bin/)?(gcc|llvm-gcc|clang)\s?}) + problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{match[2]}\"" + elsif match = regex_match_group(param, %r{(/usr/bin/)?((g|llvm-g|clang)\+\+)\s?}) + problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{match[2]}\"" + end + end + + # Prefer formula path shortcuts in strings + formula_path_strings(body_node, :share) do |p| + next unless match = regex_match_group(p, %r{(/(man))/?}) + problem "\"\#{share}#{match[1]}\" should be \"\#{#{match[2]}}\"" + end + + formula_path_strings(body_node, :prefix) do |p| + if match = regex_match_group(p, %r{(/share/(info|man))$}) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[2]}}\"" + end + if match = regex_match_group(p, %r{((/share/man/)(man[1-8]))}) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[3]}}\"" + end + if match = regex_match_group(p, %r{(/(bin|include|libexec|lib|sbin|share|Frameworks))}i) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[2].downcase}}\"" + end + end + + find_every_method_call_by_name(body_node, :depends_on).each do |method| + key, value = destructure_hash(parameters(method).first) + next if key.nil? || value.nil? + next unless match = regex_match_group(value, /(lua|perl|python|ruby)(\d*)/) + problem "#{match[1]} modules should be vendored rather than use deprecated #{method.source}`" + end + + find_every_method_call_by_name(body_node, :system).each do |method| + next unless match = regex_match_group(parameters(method).first, /(env|export)(\s+)?/) + problem "Use ENV instead of invoking '#{match[1]}' to modify the environment" + end + + find_every_method_call_by_name(body_node, :depends_on).each do |method| + next if modifier?(method.parent) + param = parameters(method).first + dep, option = hash_dep(param) + next if dep.nil? || option.nil? + offending_node(param) + problem "Dependency #{string_content(dep)} should not use option #{string_content(option)}" + end + + find_instance_method_call(body_node, :version, :==) do |method| + next unless parameters_passed?(method, "HEAD") + problem "Use 'build.head?' instead of inspecting 'version'" + end + + find_instance_method_call(body_node, "ENV", :fortran) do + next if depends_on?(:fortran) + problem "Use `depends_on :fortran` instead of `ENV.fortran`" + end + + find_instance_method_call(body_node, "ARGV", :include?) do |method| + param = parameters(method).first + next unless match = regex_match_group(param, /--(HEAD|devel)/) + problem "Use \"if build.#{match[1].downcase}?\" instead" + end + + find_const(body_node, "MACOS_VERSION") do + problem "Use MacOS.version instead of MACOS_VERSION" + end + + find_const(body_node, "MACOS_FULL_VERSION") do + problem "Use MacOS.full_version instead of MACOS_FULL_VERSION" + end + + conditional_dependencies(body_node) do |node, method, param, dep_node| + dep = string_content(dep_node) + if node.if? + if (method == :include? && regex_match_group(param, /with-#{dep}$/)) || + (method == :with? && regex_match_group(param, /#{dep}$/)) + offending_node(dep_node.parent) + problem "Replace #{node.source} with #{dep_node.parent.source} => :optional" + end + elsif node.unless? + if (method == :include? && regex_match_group(param, /without-#{dep}$/)) || + (method == :without? && regex_match_group(param, /#{dep}$/)) + offending_node(dep_node.parent) + problem "Replace #{node.source} with #{dep_node.parent.source} => :recommended" + end + end + end + find_method_with_args(body_node, :fails_with, :llvm) do problem "'fails_with :llvm' is now a no-op so should be removed" end - find_method_with_args(body_node, :system, /^(otool|install_name_tool|lipo)$/) do + find_method_with_args(body_node, :system, /(otool|install_name_tool|lipo)/) do next if @formula_name == "cctools" problem "Use ruby-macho instead of calling #{@offensive_node.source}" end find_every_method_call_by_name(body_node, :system).each do |method_node| # Skip Kibana: npm cache edge (see formula for more details) - next if @formula_name =~ /^kibana(\@\d+(\.\d+)?)?$/ + next if @formula_name =~ /^kibana(\@\d+(\.\d+)?)?$/i first_param, second_param = parameters(method_node) next if !node_equals?(first_param, "npm") || !node_equals?(second_param, "install") @@ -104,10 +295,6 @@ module RuboCop problem "Use new-style test definitions (test do)" end - if find_method_def(body_node, :options) - problem "Use new-style option definitions" - end - find_method_with_args(body_node, :skip_clean, :all) do problem <<~EOS.chomp `skip_clean :all` is deprecated; brew no longer strips symbols @@ -115,6 +302,10 @@ module RuboCop EOS end + if find_method_def(@processed_source.ast) + problem "Define method #{method_name(@offensive_node)} in the class body, not at the top-level" + end + find_instance_method_call(body_node, :build, :universal?) do next if @formula_name == "wine" problem "macOS has been 64-bit only since 10.6 so build.universal? is deprecated." @@ -128,8 +319,62 @@ module RuboCop find_instance_method_call(body_node, "ENV", :x11) do problem 'Use "depends_on :x11" instead of "ENV.x11"' end + + find_every_method_call_by_name(body_node, :depends_on).each do |method| + next unless method_called?(method, :new) + problem "`depends_on` can take requirement classes instead of instances" + end + + os = [:leopard?, :snow_leopard?, :lion?, :mountain_lion?] + os.each do |version| + find_instance_method_call(body_node, "MacOS", version) do |method| + problem "\"#{method.source}\" is deprecated, use a comparison to MacOS.version instead" + end + end + + find_instance_method_call(body_node, "Dir", :[]) do |method| + path = parameters(method).first + next unless path.str_type? + next unless match = regex_match_group(path, /^[^\*{},]+$/) + problem "Dir([\"#{string_content(path)}\"]) is unnecessary; just use \"#{match[0]}\"" + end + + fileutils_methods = Regexp.new(FileUtils.singleton_methods(false).map { |m| "(?-mix:^" + Regexp.escape(m) + "$)" }.join("|")) + find_every_method_call_by_name(body_node, :system).each do |method| + param = parameters(method).first + next unless match = regex_match_group(param, fileutils_methods) + problem "Use the `#{match}` Ruby method instead of `#{method.source}`" + end + end + + def modifier?(node) + return false unless node.if_type? + node.modifier_form? end + def_node_search :conditional_dependencies, <<~EOS + {$(if (send (send nil :build) ${:include? :with? :without?} $(str _)) + (send nil :depends_on $({str sym} _)) nil) + + $(if (send (send nil :build) ${:include? :with? :without?} $(str _)) nil + (send nil :depends_on $({str sym} _)))} + EOS + + # Match depends_on with hash as argument + def_node_matcher :hash_dep, <<~EOS + {(hash (pair $(str _) $(str _))) + (hash (pair $(str _) (array $(str _) ...)))} + EOS + + def_node_matcher :destructure_hash, <<~EOS + (hash (pair $(str _) $(sym _))) + EOS + + def_node_search :formula_path_strings, <<~EOS + {(dstr (begin (send nil %1)) $(str _ )) + (dstr _ (begin (send nil %1)) $(str _ ))} + EOS + # Node Pattern search for Language::Node def_node_search :languageNodeModule?, <<~EOS (const (const nil :Language) :Node) diff --git a/Library/Homebrew/test/rubocops/lines_cop_spec.rb b/Library/Homebrew/test/rubocops/lines_cop_spec.rb index 7e93bee75..f96a4fd48 100644 --- a/Library/Homebrew/test/rubocops/lines_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_cop_spec.rb @@ -7,7 +7,7 @@ describe RuboCop::Cop::FormulaAudit::Lines do subject(:cop) { described_class.new } context "When auditing lines" do - it "with correctable deprecated dependencies" do + it "correctable deprecated dependencies usage" do formulae = [{ "dependency" => :automake, "correct" => "automake", @@ -36,11 +36,11 @@ describe RuboCop::Cop::FormulaAudit::Lines do else offense = ":#{formula["dependency"]} is deprecated" end - expected_offenses = [{ message: offense, - severity: :convention, - line: 3, - column: 2, - source: source }] + expected_offenses = [{ message: offense, + severity: :convention, + line: 3, + column: 2, + source: source }] inspect_source(source) @@ -56,7 +56,7 @@ describe RuboCop::Cop::FormulaAudit::ClassInheritance do subject(:cop) { described_class.new } context "When auditing lines" do - it "with no space in class inheritance" do + it "inconsistent space in class inheritance" do source = <<~EOS class Foo<Formula desc "foo" @@ -64,13 +64,13 @@ describe RuboCop::Cop::FormulaAudit::ClassInheritance do end EOS - expected_offenses = [{ message: "Use a space in class inheritance: class Foo < Formula", - severity: :convention, - line: 1, - column: 10, - source: source }] + expected_offenses = [{ message: "Use a space in class inheritance: class Foo < Formula", + severity: :convention, + line: 1, + column: 10, + source: source }] - inspect_source(source) + inspect_source(source, "/homebrew-core/Formula/foo.rb") expected_offenses.zip(cop.offenses).each do |expected, actual| expect_offense(expected, actual) @@ -82,8 +82,8 @@ end describe RuboCop::Cop::FormulaAudit::Comments do subject(:cop) { described_class.new } - context "When auditing formulae" do - it "with commented cmake call" do + context "When auditing formula" do + it "commented cmake call" do source = <<~EOS class Foo < Formula desc "foo" @@ -92,11 +92,11 @@ describe RuboCop::Cop::FormulaAudit::Comments do end EOS - expected_offenses = [{ message: "Please remove default template comments", - severity: :convention, - line: 4, - column: 2, - source: source }] + expected_offenses = [{ message: "Please remove default template comments", + severity: :convention, + line: 4, + column: 2, + source: source }] inspect_source(source) @@ -105,7 +105,7 @@ describe RuboCop::Cop::FormulaAudit::Comments do end end - it "with default template comments" do + it "default template comments" do source = <<~EOS class Foo < Formula # PLEASE REMOVE @@ -114,11 +114,11 @@ describe RuboCop::Cop::FormulaAudit::Comments do end EOS - expected_offenses = [{ message: "Please remove default template comments", - severity: :convention, - line: 2, - column: 2, - source: source }] + expected_offenses = [{ message: "Please remove default template comments", + severity: :convention, + line: 2, + column: 2, + source: source }] inspect_source(source) @@ -127,7 +127,7 @@ describe RuboCop::Cop::FormulaAudit::Comments do end end - it "with commented out depends_on" do + it "commented out depends_on" do source = <<~EOS class Foo < Formula desc "foo" @@ -136,11 +136,11 @@ describe RuboCop::Cop::FormulaAudit::Comments do end EOS - expected_offenses = [{ message: 'Commented-out dependency "foo"', - severity: :convention, - line: 4, - column: 2, - source: source }] + expected_offenses = [{ message: 'Commented-out dependency "foo"', + severity: :convention, + line: 4, + column: 2, + source: source }] inspect_source(source) @@ -151,11 +151,324 @@ describe RuboCop::Cop::FormulaAudit::Comments do end end +describe RuboCop::Cop::FormulaAudit::AssertStatements do + subject(:cop) { described_class.new } + + it "assert ...include usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + assert File.read("inbox").include?("Sample message 1") + end + EOS + + expected_offenses = [{ message: "Use `assert_match` instead of `assert ...include?`", + severity: :convention, + line: 4, + column: 9, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "assert ...exist? without a negation" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + assert File.exist? "default.ini" + end + EOS + + expected_offenses = [{ message: 'Use `assert_predicate <path_to_file>, :exist?` instead of `assert File.exist? "default.ini"`', + severity: :convention, + line: 4, + column: 9, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "assert ...exist? with a negation" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + assert !File.exist?("default.ini") + end + EOS + + expected_offenses = [{ message: 'Use `refute_predicate <path_to_file>, :exist?` instead of `assert !File.exist?("default.ini")`', + severity: :convention, + line: 4, + column: 9, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "assert ...executable? without a negation" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + assert File.executable? f + end + EOS + + expected_offenses = [{ message: "Use `assert_predicate <path_to_file>, :executable?` instead of `assert File.executable? f`", + severity: :convention, + line: 4, + column: 9, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end +end + +describe RuboCop::Cop::FormulaAudit::OptionDeclarations do + subject(:cop) { described_class.new } + + it "unless build.without? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return unless build.without? "bar" + end + end + EOS + + expected_offenses = [{ message: 'Use if build.with? "bar" instead of unless build.without? "bar"', + severity: :convention, + line: 5, + column: 18, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "unless build.with? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return unless build.with? "bar" + end + end + EOS + + expected_offenses = [{ message: 'Use if build.without? "bar" instead of unless build.with? "bar"', + severity: :convention, + line: 5, + column: 18, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "negated build.with? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if !build.with? "bar" + end + end + EOS + + expected_offenses = [{ message: "Don't negate 'build.with?': use 'build.without?'", + severity: :convention, + line: 5, + column: 14, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "negated build.without? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if !build.without? "bar" + end + end + EOS + + expected_offenses = [{ message: "Don't negate 'build.without?': use 'build.with?'", + severity: :convention, + line: 5, + column: 14, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "unnecessary build.without? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if build.without? "--without-bar" + end + end + EOS + + expected_offenses = [{ message: "Don't duplicate 'without': Use `build.without? \"bar\"` to check for \"--without-bar\"", + severity: :convention, + line: 5, + column: 30, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "unnecessary build.with? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if build.with? "--with-bar" + end + end + EOS + + expected_offenses = [{ message: "Don't duplicate 'with': Use `build.with? \"bar\"` to check for \"--with-bar\"", + severity: :convention, + line: 5, + column: 27, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "build.include? conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if build.include? "without-bar" + end + end + EOS + + expected_offenses = [{ message: "Use build.without? \"bar\" instead of build.include? 'without-bar'", + severity: :convention, + line: 5, + column: 30, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "build.include? with dashed args conditional" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def post_install + return if build.include? "--bar" + end + end + EOS + + expected_offenses = [{ message: "Reference 'bar' without dashes", + severity: :convention, + line: 5, + column: 30, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "def options usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + + def options + [["--bar", "desc"]] + end + end + EOS + + expected_offenses = [{ message: "Use new-style option definitions", + severity: :convention, + line: 5, + column: 2, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end +end + describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } - context "When auditing formulae" do - it "with FileUtils" do + context "When auditing formula" do + it "FileUtils usage" do source = <<~EOS class Foo < Formula desc "foo" @@ -164,11 +477,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "Don't need 'FileUtils.' before mv", - severity: :convention, - line: 4, - column: 2, - source: source }] + expected_offenses = [{ message: "Don't need 'FileUtils.' before mv", + severity: :convention, + line: 4, + column: 2, + source: source }] inspect_source(source) @@ -177,7 +490,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with long inreplace block vars" do + it "long inreplace block vars" do source = <<~EOS class Foo < Formula desc "foo" @@ -188,11 +501,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "\"inreplace <filenames> do |s|\" is preferred over \"|longvar|\".", - severity: :convention, - line: 4, - column: 2, - source: source }] + expected_offenses = [{ message: "\"inreplace <filenames> do |s|\" is preferred over \"|longvar|\".", + severity: :convention, + line: 4, + column: 2, + source: source }] inspect_source(source) @@ -201,7 +514,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with invalid rebuild" do + it "an invalid rebuild statement" do source = <<~EOS class Foo < Formula desc "foo" @@ -213,11 +526,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "'rebuild 0' should be removed", - severity: :convention, - line: 5, - column: 4, - source: source }] + expected_offenses = [{ message: "'rebuild 0' should be removed", + severity: :convention, + line: 5, + column: 4, + source: source }] inspect_source(source) @@ -226,7 +539,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with OS.linux? check" do + it "OS.linux? check" do source = <<~EOS class Foo < Formula desc "foo" @@ -240,11 +553,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "Don't use OS.linux?; Homebrew/core only supports macOS", - severity: :convention, - line: 5, - column: 7, - source: source }] + expected_offenses = [{ message: "Don't use OS.linux?; Homebrew/core only supports macOS", + severity: :convention, + line: 5, + column: 7, + source: source }] inspect_source(source, "/homebrew-core/") @@ -253,7 +566,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with fails_with :llvm" do + it "fails_with :llvm block" do source = <<~EOS class Foo < Formula desc "foo" @@ -268,11 +581,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "'fails_with :llvm' is now a no-op so should be removed", - severity: :convention, - line: 7, - column: 2, - source: source }] + expected_offenses = [{ message: "'fails_with :llvm' is now a no-op so should be removed", + severity: :convention, + line: 7, + column: 2, + source: source }] inspect_source(source) @@ -281,7 +594,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with def test" do + it "def test's deprecated usage" do source = <<~EOS class Foo < Formula desc "foo" @@ -293,36 +606,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "Use new-style test definitions (test do)", - severity: :convention, - line: 5, - column: 2, - source: source }] - - inspect_source(source) - - expected_offenses.zip(cop.offenses).each do |expected, actual| - expect_offense(expected, actual) - end - end - - it "with def options" do - source = <<~EOS - class Foo < Formula - desc "foo" - url 'http://example.com/foo-1.0.tgz' - - def options - [["--bar", "desc"]] - end - end - EOS - - expected_offenses = [{ message: "Use new-style option definitions", - severity: :convention, - line: 5, - column: 2, - source: source }] + expected_offenses = [{ message: "Use new-style test definitions (test do)", + severity: :convention, + line: 5, + column: 2, + source: source }] inspect_source(source) @@ -331,7 +619,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with deprecated skip_clean call" do + it "deprecated skip_clean call" do source = <<~EOS class Foo < Formula desc "foo" @@ -356,7 +644,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with build.universal?" do + it "build.universal? deprecated usage" do source = <<~EOS class Foo < Formula desc "foo" @@ -367,11 +655,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so build.universal? is deprecated.", - severity: :convention, - line: 4, - column: 5, - source: source }] + expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so build.universal? is deprecated.", + severity: :convention, + line: 4, + column: 5, + source: source }] inspect_source(source) @@ -380,7 +668,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with a build.universal? exemption reports no offenses" do + it "build.universal? deprecation exempted formula" do source = <<~EOS class Wine < Formula desc "foo" @@ -395,7 +683,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do expect(cop.offenses).to be_empty end - it "with ENV.universal_binary" do + it "deprecated ENV.universal_binary usage" do source = <<~EOS class Foo < Formula desc "foo" @@ -406,11 +694,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated.", - severity: :convention, - line: 5, - column: 5, - source: source }] + expected_offenses = [{ message: "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated.", + severity: :convention, + line: 5, + column: 5, + source: source }] inspect_source(source) @@ -419,13 +707,13 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with an ENV.universal_binary exemption reports no offenses" do + it "ENV.universal_binary deprecation exempted formula" do source = <<~EOS class Wine < Formula desc "foo" url 'http://example.com/foo-1.0.tgz' if build? - ENV.universal_binary + ENV.universal_binary end end EOS @@ -434,7 +722,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do expect(cop.offenses).to be_empty end - it "with ENV.x11" do + it "deprecated ENV.x11 usage" do source = <<~EOS class Foo < Formula desc "foo" @@ -445,11 +733,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: 'Use "depends_on :x11" instead of "ENV.x11"', - severity: :convention, - line: 5, - column: 5, - source: source }] + expected_offenses = [{ message: 'Use "depends_on :x11" instead of "ENV.x11"', + severity: :convention, + line: 5, + column: 5, + source: source }] inspect_source(source) @@ -458,7 +746,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with ruby-macho alternatives" do + it "install_name_tool usage instead of ruby-macho" do source = <<~EOS class Foo < Formula desc "foo" @@ -467,11 +755,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: 'Use ruby-macho instead of calling "install_name_tool"', - severity: :convention, - line: 4, - column: 10, - source: source }] + expected_offenses = [{ message: 'Use ruby-macho instead of calling "install_name_tool"', + severity: :convention, + line: 4, + column: 10, + source: source }] inspect_source(source) @@ -480,7 +768,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with ruby-macho alternatives audit exempted formula" do + it "ruby-macho alternatives audit exempted formula" do source = <<~EOS class Cctools < Formula desc "foo" @@ -490,10 +778,10 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do EOS inspect_source(source, "/homebrew-core/Formula/cctools.rb") - expect(cop.offenses).to eq([]) + expect(cop.offenses).to be_empty end - it "with npm install without language::Node args" do + it "npm install without language::Node args" do source = <<~EOS class Foo < Formula desc "foo" @@ -502,11 +790,11 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end EOS - expected_offenses = [{ message: "Use Language::Node for npm install args", - severity: :convention, - line: 4, - column: 2, - source: source }] + expected_offenses = [{ message: "Use Language::Node for npm install args", + severity: :convention, + line: 4, + column: 2, + source: source }] inspect_source(source) @@ -515,7 +803,7 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do end end - it "with npm install without language::Node args in kibana" do + it "npm install without language::Node args in kibana(exempted formula)" do source = <<~EOS class KibanaAT44 < Formula desc "foo" @@ -525,7 +813,588 @@ describe RuboCop::Cop::FormulaAudit::Miscellaneous do EOS inspect_source(source, "/homebrew-core/Formula/kibana@4.4.rb") - expect(cop.offenses).to eq([]) + expect(cop.offenses).to be_empty + end + + it "depends_on with an instance as argument" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on FOO::BAR.new + end + EOS + + expected_offenses = [{ message: "`depends_on` can take requirement classes instead of instances", + severity: :convention, + line: 4, + column: 13, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "old style OS check" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on :foo if MacOS.snow_leopard? + end + EOS + + expected_offenses = [{ message: "\"MacOS.snow_leopard?\" is deprecated, use a comparison to MacOS.version instead", + severity: :convention, + line: 4, + column: 21, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "non glob DIR usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + rm_rf Dir["src/{llvm,test,librustdoc,etc/snapshot.pyc}"] + rm_rf Dir["src/snapshot.pyc"] + end + EOS + + expected_offenses = [{ message: 'Dir(["src/snapshot.pyc"]) is unnecessary; just use "src/snapshot.pyc"', + severity: :convention, + line: 5, + column: 13, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "system call to fileUtils Method" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + system "mkdir", "foo" + end + EOS + + expected_offenses = [{ message: 'Use the `mkdir` Ruby method instead of `system "mkdir", "foo"`', + severity: :convention, + line: 4, + column: 10, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "top-level function def outside class body" do + source = <<~EOS + def test + nil + end + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + end + EOS + + expected_offenses = [{ message: "Define method test in the class body, not at the top-level", + severity: :convention, + line: 1, + column: 0, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "Using ARGV to check options" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + verbose = ARGV.verbose? + end + end + EOS + + expected_offenses = [{ message: "Use build instead of ARGV to check options", + severity: :convention, + line: 5, + column: 14, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it 'man+"man8" usage' do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + man1.install man+"man8" => "faad.1" + end + end + EOS + + expected_offenses = [{ message: '"man+"man8"" should be "man8"', + severity: :convention, + line: 5, + column: 22, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "hardcoded gcc compiler" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + system "/usr/bin/gcc", "foo" + end + end + EOS + + expected_offenses = [{ message: "Use \"\#{ENV.cc}\" instead of hard-coding \"gcc\"", + severity: :convention, + line: 5, + column: 12, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "hardcoded g++ compiler" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + system "/usr/bin/g++", "-o", "foo", "foo.cc" + end + end + EOS + + expected_offenses = [{ message: "Use \"\#{ENV.cxx}\" instead of hard-coding \"g++\"", + severity: :convention, + line: 5, + column: 12, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "hardcoded llvm-g++ compiler" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + ENV["COMPILER_PATH"] = "/usr/bin/llvm-g++" + end + end + EOS + + expected_offenses = [{ message: "Use \"\#{ENV.cxx}\" instead of hard-coding \"llvm-g++\"", + severity: :convention, + line: 5, + column: 28, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "hardcoded gcc compiler" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + ENV["COMPILER_PATH"] = "/usr/bin/gcc" + end + end + EOS + + expected_offenses = [{ message: "Use \"\#{ENV.cc}\" instead of hard-coding \"gcc\"", + severity: :convention, + line: 5, + column: 28, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "formula path shortcut : man" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + mv "\#{share}/man", share + end + end + EOS + + expected_offenses = [{ message: '"#{share}/man" should be "#{man}"', + severity: :convention, + line: 5, + column: 17, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "formula path shortcut : libexec" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + mv "\#{prefix}/libexec", share + end + end + EOS + + expected_offenses = [{ message: "\"\#\{prefix}/libexec\" should be \"\#{libexec}\"", + severity: :convention, + line: 5, + column: 18, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "formula path shortcut : info" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + system "./configure", "--INFODIR=\#{prefix}/share/info" + end + end + EOS + + expected_offenses = [{ message: "\"\#\{prefix}/share/info\" should be \"\#{info}\"", + severity: :convention, + line: 5, + column: 47, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "formula path shortcut : man8" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + def install + system "./configure", "--MANDIR=\#{prefix}/share/man/man8" + end + end + EOS + + expected_offenses = [{ message: "\"\#\{prefix}/share/man/man8\" should be \"\#{man8}\"", + severity: :convention, + line: 5, + column: 46, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "dependecies which have to vendored" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on "lpeg" => :lua51 + end + EOS + + expected_offenses = [{ message: "lua modules should be vendored rather than use deprecated depends_on \"lpeg\" => :lua51`", + severity: :convention, + line: 4, + column: 24, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "manually setting env" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + system "export", "var=value" + end + EOS + + expected_offenses = [{ message: "Use ENV instead of invoking 'export' to modify the environment", + severity: :convention, + line: 4, + column: 10, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "dependencies with invalid options" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on "foo" => "with-bar" + end + EOS + + expected_offenses = [{ message: "Dependency foo should not use option with-bar", + severity: :convention, + line: 4, + column: 13, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "inspecting version manually" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + if version == "HEAD" + foo() + end + end + EOS + + expected_offenses = [{ message: "Use 'build.head?' instead of inspecting 'version'", + severity: :convention, + line: 4, + column: 5, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "deprecated ENV.fortran usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + test do + ENV.fortran + end + end + EOS + + expected_offenses = [{ message: "Use `depends_on :fortran` instead of `ENV.fortran`", + severity: :convention, + line: 5, + column: 4, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "deprecated ARGV.include? (--HEAD) usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + test do + head = ARGV.include? "--HEAD" + end + end + EOS + + expected_offenses = [{ message: 'Use "if build.head?" instead', + severity: :convention, + line: 5, + column: 26, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "deprecated MACOS_VERSION const usage" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + test do + version = MACOS_VERSION + end + end + EOS + + expected_offenses = [{ message: "Use MacOS.version instead of MACOS_VERSION", + severity: :convention, + line: 5, + column: 14, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "deprecated if build.with? conditional dependency" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on "foo" if build.with? "with-foo" + end + EOS + + expected_offenses = [{ message: 'Replace depends_on "foo" if build.with? "with-foo" with depends_on "foo" => :optional', + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "unless conditional dependency with build.without?" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on :foo unless build.without? "foo" + end + EOS + + expected_offenses = [{ message: 'Replace depends_on :foo unless build.without? "foo" with depends_on :foo => :recommended', + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end + end + + it "unless conditional dependency with build.include?" do + source = <<~EOS + class Foo < Formula + desc "foo" + url 'http://example.com/foo-1.0.tgz' + depends_on :foo unless build.include? "without-foo" + end + EOS + + expected_offenses = [{ message: 'Replace depends_on :foo unless build.include? "without-foo" with depends_on :foo => :recommended', + severity: :convention, + line: 4, + column: 2, + source: source }] + + inspect_source(source) + + expected_offenses.zip(cop.offenses).each do |expected, actual| + expect_offense(expected, actual) + end end end end |