Skip to content

Commit

Permalink
Add new InternalAffairs/MethodNameEqual cop
Browse files Browse the repository at this point in the history
This PR adds new `InternalAffairs/MethodNameEqual` cop.

```ruby
# bad
node.method_name == :do_something

# good
node.method?(:do_something)
```

This cop is an internal cop and will not be added to a changelog entry.
  • Loading branch information
koic authored and bbatsov committed Nov 12, 2019
1 parent 35ea8a5 commit a2b67db
Show file tree
Hide file tree
Showing 17 changed files with 113 additions and 16 deletions.
2 changes: 1 addition & 1 deletion lib/rubocop/ast/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ def parent_module_name_for_sclass(sclass_node)
end

def parent_module_name_for_block(ancestor)
if ancestor.method_name == :class_eval
if ancestor.method?(:class_eval)
# `class_eval` with no receiver applies to whatever module or class
# we are currently in
return unless (receiver = ancestor.receiver)
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/ast/node/block_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ module AST
# A `block` node is essentially a method send with a block. Parser nests
# the `send` node inside the `block` node.
class BlockNode < Node
include MethodIdentifierPredicates

VOID_CONTEXT_METHODS = %i[each tap].freeze

# The `send` node associated with this block.
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require_relative 'internal_affairs/method_name_equal'
require_relative 'internal_affairs/node_destructuring'
require_relative 'internal_affairs/node_type_predicate'
require_relative 'internal_affairs/offense_location_keyword'
Expand Down
59 changes: 59 additions & 0 deletions lib/rubocop/cop/internal_affairs/method_name_equal.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

module RuboCop
module Cop
module InternalAffairs
# Checks that method names are checked using `method?` method.
#
# @example
# # bad
# node.method_name == :do_something
#
# # good
# node.method?(:do_something)
#
class MethodNameEqual < Cop
include RangeHelp

MSG = 'Use `method?(%<method_name>s)` instead of ' \
'`method_name == %<method_name>s`.'

def_node_matcher :method_name?, <<~PATTERN
(send
$(send
(...) :method_name) :==
$...)
PATTERN

def on_send(node)
method_name?(node) do |method_name_node, method_name_arg|
message = format(MSG, method_name: method_name_arg.first.source)

range = range(method_name_node, node)

add_offense(node, location: range, message: message)
end
end

def autocorrect(node)
lambda do |corrector|
method_name?(node) do |method_name_node, method_name_arg|
corrector.replace(
range(method_name_node, node),
"method?(#{method_name_arg.first.source})"
)
end
end
end

private

def range(method_name_node, node)
range_between(
method_name_node.loc.selector.begin_pos, node.source_range.end_pos
)
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class MultilineMethodArgumentLineBreaks < Cop
'on a separate line.'

def on_send(node)
return if node.method_name == :[]=
return if node.method?(:[]=)

args = node.arguments

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def on_def(node)

# @param [DefNode] node a constructor definition
def check(node)
return unless node.method_name == :initialize
return unless node.method?(:initialize)

check_body(node.body)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/ineffective_access_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def access_modifier?(node)
end

def correct_visibility?(node, modifier, ignored_methods)
return true if modifier.nil? || modifier.method_name == :public
return true if modifier.nil? || modifier.method?(:public)

ignored_methods.include?(node.method_name)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/redundant_with_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def on_block(node)
def autocorrect(node)
lambda do |corrector|
redundant_with_index?(node) do |send|
if send.method_name == :each_with_index
if send.method?(:each_with_index)
corrector.replace(send.loc.selector, 'each')
else
corrector.remove(with_index_range(send))
Expand All @@ -63,7 +63,7 @@ def autocorrect(node)
private

def message(node)
if node.method_name == :each_with_index
if node.method?(:each_with_index)
MSG_EACH_WITH_INDEX
else
MSG_WITH_INDEX
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/redundant_with_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def on_block(node)
def autocorrect(node)
lambda do |corrector|
redundant_with_object?(node) do |send|
if send.method_name == :each_with_object
if send.method?(:each_with_object)
corrector.replace(with_object_range(send), 'each')
else
corrector.remove(with_object_range(send))
Expand All @@ -64,7 +64,7 @@ def autocorrect(node)
private

def message(node)
if node.method_name == :each_with_object
if node.method?(:each_with_object)
MSG_EACH_WITH_OBJECT
else
MSG_WITH_OBJECT
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/useless_access_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def check_node(node)

def access_modifier?(node)
node.bare_access_modifier? ||
node.method_name == :private_class_method
node.method?(:private_class_method)
end

def check_scope(node)
Expand Down Expand Up @@ -203,7 +203,7 @@ def check_child_nodes(node, unused, cur_vis)
def check_send_node(node, cur_vis, unused)
if node.bare_access_modifier?
check_new_visibility(node, unused, node.method_name, cur_vis)
elsif node.method_name == :private_class_method && !node.arguments?
elsif node.method?(:private_class_method) && !node.arguments?
add_offense(node, message: format(MSG, current: node.method_name))
[cur_vis, unused]
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/useless_setter_call.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def constructor?(node)
return true if node.literal?
return false unless node.send_type?

node.method_name == :new
node.method?(:new)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/metrics/method_length.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def on_def(node)
alias on_defs on_def

def on_block(node)
return unless node.send_node.method_name == :define_method
return unless node.send_node.method?(:define_method)

check_code_length(node)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/alias.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def scope_type(node)
when :def, :defs
return :dynamic
when :block
return :instance_eval if parent.method_name == :instance_eval
return :instance_eval if parent.method?(:instance_eval)

return :dynamic
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/eval_with_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def special_line_keyword?(node)
# FIXME: It's a Style/ConditionalAssignment's false positive.
# rubocop:disable Style/ConditionalAssignment
def with_lineno?(node)
if node.method_name == :eval
if node.method?(:eval)
node.arguments.size == 4
else
node.arguments.size == 3
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/mixin_grouping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def sibling_mixins(send_node)
.select(&:macro?)

siblings.select do |sibling_node|
sibling_node.method_name == send_node.method_name
sibling_node.method?(send_node.method_name)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/trivial_accessors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def in_module_or_instance_eval?(node)
when :module
return true
else
return true if pnode.method_name == :instance_eval
return true if pnode.method?(:instance_eval)
end
end
false
Expand Down
35 changes: 35 additions & 0 deletions spec/rubocop/cop/internal_affairs/method_name_equal_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::MethodNameEqual do
subject(:cop) { described_class.new(config) }

let(:config) { RuboCop::Config.new }

it 'registers an offense when using `#method == :do_something`' do
expect_offense(<<~RUBY)
node.method_name == :do_something
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `method?(:do_something)` instead of `method_name == :do_something`.
RUBY

expect_correction(<<~RUBY)
node.method?(:do_something)
RUBY
end

it 'registers an offense when using `#method == other_node.do_something`' do
expect_offense(<<~RUBY)
node.method_name == other_node.do_something
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `method?(other_node.do_something)` instead of `method_name == other_node.do_something`.
RUBY

expect_correction(<<~RUBY)
node.method?(other_node.do_something)
RUBY
end

it 'does not register an offense when using `#method?`' do
expect_no_offenses(<<~RUBY)
node.method?(:do_something)
RUBY
end
end

0 comments on commit a2b67db

Please sign in to comment.