Skip to content

Commit

Permalink
Use node type predicates and #each_child_node(type) where possible (r…
Browse files Browse the repository at this point in the history
…ubocop#3364)

This change replaces constructs like:

```
node.type == :dstr
```

with:

```
node.dstr_type?
```

and constructs like:

```
node.children.select { |child| child.type == :dstr }.each do |n|
```

with:

```
node.each_child_node(:dstr) do |n|
```
  • Loading branch information
Drenmi authored and Neodelf committed Oct 15, 2016
1 parent e9cd27c commit 0d048ac
Show file tree
Hide file tree
Showing 72 changed files with 175 additions and 187 deletions.
12 changes: 6 additions & 6 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2016-08-03 09:52:07 +0300 using RuboCop version 0.42.0.
# on 2016-08-04 02:09:05 +0800 using RuboCop version 0.42.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 141
# Offense count: 131
Metrics/AbcSize:
Max: 21

# Offense count: 30
# Offense count: 31
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 158

# Offense count: 58
# Offense count: 57
Metrics/CyclomaticComplexity:
Max: 8

# Offense count: 192
# Offense count: 191
# Configuration parameters: CountComments.
Metrics/MethodLength:
Max: 16
Expand All @@ -29,6 +29,6 @@ Metrics/MethodLength:
Metrics/ModuleLength:
Max: 156

# Offense count: 30
# Offense count: 29
Metrics/PerceivedComplexity:
Max: 8
10 changes: 4 additions & 6 deletions lib/rubocop/cop/lint/assignment_in_condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ def check(node)
condition, = *node

# assignments inside blocks are not what we're looking for
return if condition.type == :block
return if condition.block_type?
traverse_node(condition, ASGN_TYPES) do |asgn_node|
if asgn_node.type == :send
if asgn_node.send_type?
_receiver, method_name, *_args = *asgn_node
next :skip_children if method_name !~ /=\z/
end
Expand All @@ -43,7 +43,7 @@ def check(node)
end

# assignment nodes from shorthand ops like ||= don't have operator
if asgn_node.type != :begin && asgn_node.loc.operator
if !asgn_node.begin_type? && asgn_node.loc.operator
add_offense(asgn_node, :operator)
end
end
Expand All @@ -54,9 +54,7 @@ def traverse_node(node, types, &block)
result = yield node if types.include?(node.type)
# return to skip all descendant nodes
return if result == :skip_children
node.children.each do |child|
traverse_node(child, types, &block) if child.is_a?(Node)
end
node.each_child_node { |child| traverse_node(child, types, &block) }
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/else_layout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def check(node)

def check_else(node)
_cond, _if_branch, else_branch = *node
return unless else_branch && else_branch.type == :begin
return unless else_branch && else_branch.begin_type?

first_else_expr = else_branch.children.first
return unless first_else_expr.source_range.line == node.loc.else.line
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/empty_interpolation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class EmptyInterpolation < Cop
MSG = 'Empty interpolation detected.'.freeze

def on_dstr(node)
node.children.select { |n| n.type == :begin }.each do |begin_node|
node.each_child_node(:begin) do |begin_node|
add_offense(begin_node, :expression) if begin_node.children.empty?
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/eval.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def on_send(node)
return unless receiver.nil? &&
method_name == :eval &&
!args.empty? &&
args.first.type != :str
!args.first.str_type?
add_offense(node, :selector)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/format_parameter_mismatch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def node_with_splat_args?(node)

_receiver_node, _method_name, *args = *node

args.butfirst.any? { |arg| arg.type == :splat }
args.butfirst.any?(&:splat_type?)
end

def heredoc?(node)
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/lint/literal_in_condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ def check_for_literal(node)
end

def not?(node)
return false unless node && node.type == :send
return false unless node && node.send_type?

_receiver, method_name, *_args = *node

method_name == :!
end

def basic_literal?(node)
if node && node.type == :array
if node && node.array_type?
primitive_array?(node)
else
node.basic_literal?
Expand All @@ -104,7 +104,7 @@ def check_node(node)
operands.each do |op|
handle_node(op)
end
elsif node.type == :begin && node.children.size == 1
elsif node.begin_type? && node.children.one?
child_node = node.children.first
handle_node(child_node)
end
Expand All @@ -119,8 +119,8 @@ def handle_node(node)
end

def check_case_cond(node)
return if node.type == :array && !primitive_array?(node)
return if node.type == :dstr
return if node.array_type? && !primitive_array?(node)
return if node.dstr_type?

handle_node(node)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/literal_in_interpolation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class LiteralInInterpolation < Cop
COMPOSITE = [:array, :hash, :pair, :irange, :erange].freeze

def on_dstr(node)
node.children.select { |n| n.type == :begin }.each do |begin_node|
node.each_child_node(:begin) do |begin_node|
final_node = begin_node.children.last
next unless final_node
next if special_keyword?(final_node)
Expand All @@ -35,7 +35,7 @@ def autocorrect(node)

def special_keyword?(node)
# handle strings like __FILE__
(node.type == :str && !node.loc.respond_to?(:begin)) ||
(node.str_type? && !node.loc.respond_to?(:begin)) ||
node.source_range.is?('__LINE__')
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/next_without_accumulator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def on_block(node)
private

def parent_block_node(node)
node.each_ancestor.find { |n| n.type == :block }
node.each_ancestor(:block).first
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/string_conversion_in_interpolation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ class StringConversionInInterpolation < Cop
'interpolation.'.freeze

def on_dstr(node)
node.children.select { |n| n.type == :begin }.each do |begin_node|
node.each_child_node(:begin) do |begin_node|
final_node = begin_node.children.last
next unless final_node && final_node.type == :send
next unless final_node && final_node.send_type?

receiver, method_name, *args = *final_node
next unless method_name == :to_s && args.empty?
Expand Down
3 changes: 1 addition & 2 deletions lib/rubocop/cop/lint/underscore_prefixed_variable_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ def after_leaving_scope(scope, _variable_table)

def check_variable(variable)
return unless variable.should_be_unused?
return if variable.references.empty?
return if variable.references.none?(&:explicit?)

node = variable.declaration_node

location = if node.type == :match_with_lvasgn
location = if node.match_with_lvasgn_type?
node.children.first.source_range
else
node.loc.name
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/useless_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def message_for_useless_assignment(assignment)
def return_value_node_of_scope(scope)
body_node = scope.body_node

if body_node.type == :begin
if body_node.begin_type?
body_node.children.last
else
body_node
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/lint/useless_setter_call.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class UselessSetterCall < Cop
def on_method_def(_node, _method_name, _args, body)
return unless body

expression = if body.type == :begin
expression = if body.begin_type?
body.children
else
body
Expand All @@ -45,9 +45,9 @@ def on_method_def(_node, _method_name, _args, body)
end

def setter_call_to_local_variable?(node)
return unless node && node.type == :send
return unless node && node.send_type?
receiver, method, _args = *node
return unless receiver && receiver.type == :lvar
return unless receiver && receiver.lvar_type?
method =~ /(?:\w|\[\])=$/
end

Expand Down Expand Up @@ -100,7 +100,7 @@ def process_multiple_assignment(masgn_node)
lhs_variable_name, = *lhs_node
rhs_node = mrhs_node.children[index]

if mrhs_node.type == :array && rhs_node
if mrhs_node.array_type? && rhs_node
process_assignment(lhs_variable_name, rhs_node)
else
@local[lhs_variable_name] = true
Expand Down Expand Up @@ -140,7 +140,7 @@ def process_assignment(asgn_node, rhs_node)

def constructor?(node)
return true if node.literal?
return false unless node.type == :send
return false unless node.send_type?
_receiver, method = *node
method == :new
end
Expand Down
6 changes: 2 additions & 4 deletions lib/rubocop/cop/lint/void.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ def check_begin(node)
end

def check_for_void_op(node)
return unless node.type == :send
return unless node.loc.selector
return unless node.send_type? && node.loc.selector

op = node.loc.selector.source

Expand All @@ -48,8 +47,7 @@ def check_for_var(node)
end

def check_for_literal(node)
return unless node.literal?
return if node.xstr_type?
return if !node.literal? || node.xstr_type?

add_offense(node, :expression, format(LIT_MSG, node.source))
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/mixin/access_modifier_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def modifier_structure?(node)
# is a Class or Module. Filters out simple method calls to similarly
# named private, protected or public.
def class_or_module_parent?(node)
node.each_ancestor do |a|
if a.type == :block
return true if a.class_constructor?
elsif a.type != :begin
return [:casgn, :sclass, :class, :module].include?(a.type)
node.each_ancestor do |ancestor|
if ancestor.block_type?
return true if ancestor.class_constructor?
elsif !ancestor.begin_type?
return [:casgn, :sclass, :class, :module].include?(ancestor.type)
end
end
end
Expand Down
9 changes: 4 additions & 5 deletions lib/rubocop/cop/mixin/configurable_naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@ def valid_name?(node, name)
# A class emitter method is a singleton method in a class/module, where
# the method has the same name as a class defined in the class/module.
def class_emitter_method?(node, name)
return false unless node.defs_type?
return false unless node.parent && node.defs_type?
# a class emitter method may be defined inside `def self.included`,
# `def self.extended`, etc.
node = node.parent while node.parent && node.parent.defs_type?
return false unless node.parent
node = node.parent while node.parent.defs_type?

node.parent.children.compact.any? do |c|
c.class_type? && c.loc.name.is?(name.to_s)
node.parent.each_child_node(:class).any? do |c|
c.loc.name.is?(name.to_s)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/mixin/multiline_expression_indentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,12 @@ def not_for_this_cop?(node)
end

def grouped_expression?(node)
node.type == :begin && node.loc.respond_to?(:begin) && node.loc.begin
node.begin_type? && node.loc.respond_to?(:begin) && node.loc.begin
end

def inside_arg_list_parentheses?(node, ancestor)
a = ancestor.loc
return false unless ancestor.type == :send && a.begin &&
return false unless ancestor.send_type? && a.begin &&
a.begin.is?('(')
n = node.source_range
n.begin_pos > a.begin.begin_pos && n.end_pos < a.end.end_pos
Expand Down
12 changes: 6 additions & 6 deletions lib/rubocop/cop/mixin/negative_conditional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ module Cop
# Some common code shared between FavorUnlessOverNegatedIf and
# FavorUntilOverNegatedWhile.
module NegativeConditional
def self.included(mod)
mod.def_node_matcher :single_negative?, '(send !(send _ :!) :!)'
end
extend NodePattern::Macros
include IfNode

def_node_matcher :single_negative?, '(send !(send _ :!) :!)'

def check_negative_conditional(node)
condition, _body, _rest = *node

# Look at last expression of contents if there are parentheses
# around condition.
condition = condition.children.last while condition.type == :begin
condition = condition.children.last while condition.begin_type?

return unless single_negative?(condition) &&
!(node.loc.respond_to?(:else) && node.loc.else)
return unless single_negative?(condition) && !if_else?(node)

add_offense(node, :expression)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/trailing_comma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def avoid_comma(kind, comma_begin_pos, sb, extra_info)

def put_comma(node, items, kind, sb)
last_item = items.last
return if last_item.type == :block_pass
return if last_item.block_pass_type?

last_expr = last_item.source_range
ix = last_expr.source.rindex("\n") || 0
Expand Down
8 changes: 2 additions & 6 deletions lib/rubocop/cop/performance/fixed_size.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,13 @@ def on_send(node)
def contains_splat?(node)
return unless node.array_type?

node.children.any? do |child|
child.respond_to?(:splat_type?) && child.splat_type?
end
node.each_child_node(:splat).any?
end

def contains_double_splat?(node)
return unless node.hash_type?

node.children.any? do |child|
child.respond_to?(:kwsplat_type?) && child.kwsplat_type?
end
node.each_child_node(:kwsplat).any?
end

def string_argument?(node)
Expand Down
5 changes: 2 additions & 3 deletions lib/rubocop/cop/rails/delegate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ def trivial_delegate?(method_name, args, body)
end

def delegate?(body)
receiver, * = *body
return false unless receiver.is_a? Parser::AST::Node
return false unless receiver.type == :send
receiver, = *body
return false unless receiver.is_a?(Node) && receiver.send_type?
receiver.child_nodes.empty?
end

Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/cop/rails/scope_args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ def on_send(node)

second_arg = args[1]

add_offense(second_arg, :expression) if second_arg.type == :send
return unless second_arg.send_type?

add_offense(second_arg, :expression)
end
end
end
Expand Down
Loading

0 comments on commit 0d048ac

Please sign in to comment.