style: refactor OptionDeclarations cop

This commit is contained in:
Rylan Polster 2020-07-05 23:43:09 -04:00
parent 1859162735
commit e0b5c2093a
3 changed files with 50 additions and 44 deletions

View File

@ -847,11 +847,6 @@ module Homebrew
# TODO: check could be in RuboCop
problem "`#{Regexp.last_match(1)}` is now unnecessary" if line =~ /(require ["']formula["'])/
return unless !@core_tap && line =~ /depends_on .+ if build\.with(out)?\?\(?["']\w+["']\)?/
# TODO: check could be in RuboCop
problem "`Use :optional` or `:recommended` instead of `#{Regexp.last_match(0)}`"
end
def audit_reverse_migration

View File

@ -102,6 +102,33 @@ module RuboCop
def audit_formula(_node, _class_node, _parent_class_node, body_node)
problem "Use new-style option definitions" if find_method_def(body_node, :options)
if formula_tap == "homebrew-core"
# Use of build.with? implies options, which are forbidden in homebrew/core
find_instance_method_call(body_node, :build, :without?) do
problem "Formulae in homebrew/core should not use `build.without?`."
end
find_instance_method_call(body_node, :build, :with?) do
problem "Formulae in homebrew/core should not use `build.with?`."
end
return
end
# Matches `depends_on "foo" if build.with?("bar")` or depends_on "foo" if build.without?("bar")`
depends_on_build_regex = /depends_on .+ (if build\.with(out)?\?\(["']\w+["']\))/
find_instance_method_call(body_node, :build, :with?) do |n|
next unless match = n.parent.source.match(depends_on_build_regex)
problem "Use `:optional` or `:recommended` instead of `#{match[1]}`"
end
find_instance_method_call(body_node, :build, :without?) do |n|
next unless match = n.parent.source.match(depends_on_build_regex)
problem "Use `:optional` or `:recommended` instead of `#{match[1]}`"
end
find_instance_method_call(body_node, :build, :without?) do |method|
next unless unless_modifier?(method.parent)
@ -143,29 +170,8 @@ module RuboCop
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
return if formula_tap != "homebrew-core"
# Use of build.with? implies options, which are forbidden in homebrew/core
find_instance_method_call(body_node, :build, :without?) do
problem "Formulae in homebrew/core should not use `build.without?`."
end
find_instance_method_call(body_node, :build, :with?) do
problem "Formulae in homebrew/core should not use `build.with?`."
find_instance_method_call(body_node, :build, :include?) do
problem "`build.include?` is deprecated"
end
end

View File

@ -201,6 +201,24 @@ describe RuboCop::Cop::FormulaAudit::OptionDeclarations do
RUBY
end
it "build.without? in dependencies" do
expect_offense(<<~RUBY)
class Foo < Formula
depends_on "bar" if build.without?("baz")
^^^^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.without?("baz")`
end
RUBY
end
it "build.with? in dependencies" do
expect_offense(<<~RUBY)
class Foo < Formula
depends_on "bar" if build.with?("baz")
^^^^^^^^^^^^^^^^^^ Use `:optional` or `:recommended` instead of `if build.with?("baz")`
end
RUBY
end
it "unless build.without? conditional" do
expect_offense(<<~RUBY)
class Foo < Formula
@ -279,27 +297,14 @@ describe RuboCop::Cop::FormulaAudit::OptionDeclarations do
RUBY
end
it "build.include? conditional" do
it "build.include? deprecated" do
expect_offense(<<~RUBY)
class Foo < Formula
desc "foo"
url 'https://brew.sh/foo-1.0.tgz'
def post_install
return if build.include? "without-bar"
^^^^^^^^^^^ Use build.without? \"bar\" instead of build.include? 'without-bar'
end
end
RUBY
end
it "build.include? with dashed args conditional" do
expect_offense(<<~RUBY)
class Foo < Formula
desc "foo"
url 'https://brew.sh/foo-1.0.tgz'
def post_install
return if build.include? "--bar"
^^^^^ Reference 'bar' without dashes
return if build.include? "foo"
^^^^^^^^^^^^^^^^^^^^ `build.include?` is deprecated
end
end
RUBY