Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
f1e7209
commit 56373fb
Showing
5 changed files
with
291 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#10202](https://github.com/rubocop/rubocop/issues/10202): Add new `Lint/UselessRuby2Keywords` cop. ([@dvandersluis][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Lint | ||
# This cop looks for `ruby2_keywords` calls for methods that do not need it. | ||
# | ||
# `ruby2_keywords` should only be called on methods that accept an argument splat | ||
# (`*args`) but do not explicit keyword arguments (`k:` or `k: true`) or | ||
# a keyword splat (`**kwargs`). | ||
# | ||
# @example | ||
# # good (splat argument without keyword arguments) | ||
# ruby2_keywords def foo(*args); end | ||
# | ||
# # bad (no arguments) | ||
# ruby2_keywords def foo; end | ||
# | ||
# # good | ||
# def foo; end | ||
# | ||
# # bad (positional argument) | ||
# ruby2_keywords def foo(arg); end | ||
# | ||
# # good | ||
# def foo(arg); end | ||
# | ||
# # bad (double splatted argument) | ||
# ruby2_keywords def foo(**args); end | ||
# | ||
# # good | ||
# def foo(**args); end | ||
# | ||
# # bad (keyword arguments) | ||
# ruby2_keywords def foo(i:, j:); end | ||
# | ||
# # good | ||
# def foo(i:, j:); end | ||
# | ||
# # bad (splat argument with keyword arguments) | ||
# ruby2_keywords def foo(*args, i:, j:); end | ||
# | ||
# # good | ||
# def foo(*args, i:, j:); end | ||
# | ||
# # bad (splat argument with double splat) | ||
# ruby2_keywords def foo(*args, **kwargs); end | ||
# | ||
# # good | ||
# def foo(*args, **kwargs); end | ||
# | ||
# # bad (ruby2_keywords given a symbol) | ||
# def foo; end | ||
# ruby2_keywords :foo | ||
# | ||
# # good | ||
# def foo; end | ||
# | ||
# # bad (ruby2_keywords with dynamic method) | ||
# define_method(:foo) { |arg| } | ||
# ruby2_keywords :foo | ||
# | ||
# # good | ||
# define_method(:foo) { |arg| } | ||
# | ||
class UselessRuby2Keywords < Base | ||
MSG = '`ruby2_keywords` is unnecessary for method `%<method_name>s`.' | ||
RESTRICT_ON_SEND = %i[ruby2_keywords].freeze | ||
|
||
# Looks for statically or dynamically defined methods with a given name | ||
# @!method method_definition(node, method_name) | ||
def_node_matcher :method_definition, <<~PATTERN | ||
{ | ||
(def %1 ...) | ||
({block numblock} (send _ :define_method (sym %1)) ...) | ||
} | ||
PATTERN | ||
|
||
def on_send(node) | ||
if node.first_argument.def_type? | ||
inspect_def(node, node.first_argument) | ||
elsif node.first_argument.sym_type? | ||
inspect_sym(node, node.first_argument) | ||
end | ||
end | ||
|
||
private | ||
|
||
def inspect_def(node, def_node) | ||
return if allowed_arguments(def_node.arguments) | ||
|
||
add_offense(node.loc.selector, message: format(MSG, method_name: def_node.method_name)) | ||
end | ||
|
||
def inspect_sym(node, sym_node) | ||
return unless node.parent | ||
|
||
method_name = sym_node.value | ||
definition = node.parent.each_child_node.detect { |n| method_definition(n, method_name) } | ||
|
||
return unless definition | ||
return if allowed_arguments(definition.arguments) | ||
|
||
add_offense(node, message: format(MSG, method_name: method_name)) | ||
end | ||
|
||
# `ruby2_keywords` is only allowed if there's a `restarg` and no keyword arguments | ||
def allowed_arguments(arguments) | ||
return false if arguments.empty? | ||
|
||
arguments.each_child_node(:restarg).any? && | ||
arguments.each_child_node(:kwarg, :kwoptarg, :kwrestarg).none? | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,167 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Lint::UselessRuby2Keywords, :config do | ||
context 'when `ruby2_keywords` is given a `def` node' do | ||
it 'registers an offense for a method without arguments' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with only positional args' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(arg) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with only `kwrestarg`' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(**kwargs) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with only keyword args' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(i:, j:) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with a `restarg` and keyword args' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(*args, i:, j:) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with a `restarg` and `kwoptarg`' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(*args, i: 1) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a method with a `restarg` and `kwrestarg`' do | ||
expect_offense(<<~RUBY) | ||
ruby2_keywords def foo(*args, **kwargs) | ||
^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a method with a `restarg` and no `kwrestarg`' do | ||
expect_no_offenses(<<~RUBY) | ||
ruby2_keywords def foo(*args) | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a method with a `restarg` other positional args' do | ||
expect_no_offenses(<<~RUBY) | ||
ruby2_keywords def foo(arg1, arg2, *rest) | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a method with a `restarg` other optional args' do | ||
expect_no_offenses(<<~RUBY) | ||
ruby2_keywords def foo(arg1 = 5, *rest) | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a method with a `restarg` and `blockarg`' do | ||
expect_no_offenses(<<~RUBY) | ||
ruby2_keywords def foo(*rest, &block) | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when `ruby2_keywords` is given a symbol' do | ||
it 'registers an offense for an unnecessary `ruby2_keywords`' do | ||
expect_offense(<<~RUBY) | ||
def foo(**kwargs) | ||
end | ||
ruby2_keywords :foo | ||
^^^^^^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for an allowed def' do | ||
expect_no_offenses(<<~RUBY) | ||
def foo(*args) | ||
end | ||
ruby2_keywords :foo | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when there is no `def`' do | ||
expect_no_offenses(<<~RUBY) | ||
ruby2_keywords :foo | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when the `def` is at a different depth' do | ||
expect_no_offenses(<<~RUBY) | ||
class C | ||
class D | ||
def foo(**kwargs) | ||
end | ||
end | ||
ruby2_keywords :foo | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'with a dynamically defined method' do | ||
it 'registers an offense for an unnecessary `ruby2_keywords`' do | ||
expect_offense(<<~RUBY) | ||
define_method(:foo) { |**kwargs| } | ||
ruby2_keywords :foo | ||
^^^^^^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for an allowed `ruby2_keywords`' do | ||
expect_no_offenses(<<~RUBY) | ||
define_method(:foo) { |*args| } | ||
ruby2_keywords :foo | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when the method has a `shadowarg`' do | ||
expect_offense(<<~RUBY) | ||
define_method(:foo) { |x; y| } | ||
ruby2_keywords :foo | ||
^^^^^^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when the method has a `restarg` and a `shadowarg`' do | ||
expect_no_offenses(<<~RUBY) | ||
define_method(:foo) { |*args; y| } | ||
ruby2_keywords :foo | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a numblock', :ruby27 do | ||
expect_offense(<<~RUBY) | ||
define_method(:foo) { _1 } | ||
ruby2_keywords :foo | ||
^^^^^^^^^^^^^^^^^^^ `ruby2_keywords` is unnecessary for method `foo`. | ||
RUBY | ||
end | ||
end | ||
end |