Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lines_cop: Convert ARGV audit to negative look ahead #3518

Merged
merged 1 commit into from
Dec 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Library/Homebrew/rubocops/extend/formula_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ def find_instance_method_call(node, instance, method_name)
end
end

# Matches receiver part of method,
# EX: to match `ARGV.<whatever>()`
# call `find_instance_call(node, "ARGV")`
# yields to a block with parent node of receiver
def find_instance_call(node, name)
node.each_descendant(:send) do |method_node|
next if method_node.receiver.nil?
next if method_node.receiver.const_name != name &&
method_node.receiver.method_name != name
@offense_source_range = method_node.receiver.source_range
@offensive_node = method_node.receiver
return true unless block_given?
yield method_node
end
end

# Returns nil if does not depend on dependency_name
# args: node - dependency_name - dependency's name
def depends_on?(dependency_name, *types)
Expand Down
7 changes: 3 additions & 4 deletions Library/Homebrew/rubocops/lines_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,9 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
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
find_instance_call(body_node, "ARGV") do |method_node|
next if [:debug?, :verbose?, :value].index(method_node.method_name)
problem "Use build instead of ARGV to check options"
end

find_instance_method_call(body_node, :man, :+) do |method|
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/rubocops/lines_cop_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,12 @@ class Foo < Formula
end

it "Using ARGV to check options" do
expect_offense(<<~RUBY)
expect_no_offenses(<<~RUBY)
class Foo < Formula
desc "foo"
url 'http://example.com/foo-1.0.tgz'
def install
verbose = ARGV.verbose?
^^^^^^^^^^^^^ Use build instead of ARGV to check options
end
end
RUBY
Expand Down Expand Up @@ -739,6 +738,7 @@ class Foo < Formula
test do
head = ARGV.include? "--HEAD"
^^^^^^ Use "if build.head?" instead
^^^^ Use build instead of ARGV to check options
end
end
RUBY
Expand Down