forked from rubocop/rubocop
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Showing
5 changed files
with
371 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 @@ | ||
* [#12309](https://github.com/rubocop/rubocop/issues/12309): Add new `Style/RedundantSuperArguments` cop. ([@earlopain][]) |
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,144 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Style | ||
# Checks for redundant argument forwarding when calling super | ||
# with arguments identical to the method definition. | ||
# | ||
# @example | ||
# # bad | ||
# def method(*args, **kwargs) | ||
# super(*args, **kwargs) | ||
# end | ||
# | ||
# # good - implicitly passing all arguments | ||
# def method(*args, **kwargs) | ||
# super | ||
# end | ||
# | ||
# # good - forwarding a subset of the arguments | ||
# def method(*args, **kwargs) | ||
# super(*args) | ||
# end | ||
# | ||
# # good - forwarding no arguments | ||
# def method(*args, **kwargs) | ||
# super() | ||
# end | ||
class RedundantSuperArguments < Base | ||
extend AutoCorrector | ||
|
||
DEF_TYPES = %i[def defs].freeze | ||
|
||
MSG = 'Call `super` without arguments and parentheses when the signature is identical.' | ||
|
||
def on_super(super_node) | ||
def_node = super_node.ancestors.find do |node| | ||
# You can't implicitly call super when dynamically defining methods | ||
break if define_method?(node) | ||
|
||
break node if DEF_TYPES.include?(node.type) | ||
end | ||
return unless def_node | ||
return unless arguments_identical?(def_node.arguments.argument_list, super_node.arguments) | ||
|
||
add_offense(super_node) { |corrector| corrector.replace(super_node, 'super') } | ||
end | ||
|
||
private | ||
|
||
# rubocop:disable Metrics/CyclomaticComplexity | ||
# rubocop:disable Metrics/PerceivedComplexity | ||
def arguments_identical?(def_args, super_args) | ||
super_args = preprocess_super_args(super_args) | ||
return false if def_args.size != super_args.size | ||
|
||
def_args.zip(super_args).each do |def_arg, super_arg| | ||
next if positional_arg_same?(def_arg, super_arg) | ||
next if positional_rest_arg_same(def_arg, super_arg) | ||
next if keyword_arg_same?(def_arg, super_arg) | ||
next if keyword_rest_arg_same?(def_arg, super_arg) | ||
next if block_arg_same?(def_arg, super_arg) | ||
next if forward_arg_same?(def_arg, super_arg) | ||
|
||
return false | ||
end | ||
true | ||
end | ||
# rubocop:enable Metrics/CyclomaticComplexity | ||
# rubocop:enable Metrics/PerceivedComplexity | ||
|
||
def positional_arg_same?(def_arg, super_arg) | ||
return false unless def_arg.arg_type? || def_arg.optarg_type? | ||
return false unless super_arg.lvar_type? | ||
|
||
def_arg.name == super_arg.children.first | ||
end | ||
|
||
def positional_rest_arg_same(def_arg, super_arg) | ||
return false unless def_arg.restarg_type? | ||
# anon forwarding | ||
return true if def_arg.name.nil? && super_arg.forwarded_restarg_type? | ||
return false unless super_arg.splat_type? | ||
return false unless (lvar_node = super_arg.children.first).lvar_type? | ||
|
||
def_arg.name == lvar_node.children.first | ||
end | ||
|
||
def keyword_arg_same?(def_arg, super_arg) | ||
return false unless def_arg.kwarg_type? || def_arg.kwoptarg_type? | ||
return false unless (pair_node = super_arg).pair_type? | ||
return false unless (sym_node = pair_node.key).sym_type? | ||
return false unless (lvar_node = pair_node.value).lvar_type? | ||
return false unless sym_node.value == lvar_node.children.first | ||
|
||
def_arg.name == sym_node.value | ||
end | ||
|
||
def keyword_rest_arg_same?(def_arg, super_arg) | ||
return false unless def_arg.kwrestarg_type? | ||
# anon forwarding | ||
return true if def_arg.name.nil? && super_arg.forwarded_kwrestarg_type? | ||
return false unless super_arg.kwsplat_type? | ||
return false unless (lvar_node = super_arg.children.first).lvar_type? | ||
|
||
def_arg.name == lvar_node.children.first | ||
end | ||
|
||
def block_arg_same?(def_arg, super_arg) | ||
return false unless def_arg.blockarg_type? && super_arg.block_pass_type? | ||
# anon forwarding | ||
return true if (block_pass_child = super_arg.children.first).nil? && def_arg.name.nil? | ||
|
||
def_arg.name == block_pass_child.children.first | ||
end | ||
|
||
def forward_arg_same?(def_arg, super_arg) | ||
return false unless def_arg.forward_arg_type? && super_arg.forwarded_args_type? | ||
|
||
true | ||
end | ||
|
||
def define_method?(node) | ||
return false unless node.block_type? | ||
|
||
child = node.child_nodes.first | ||
return false unless child.send_type? | ||
|
||
child.method?(:define_method) || child.method?(:define_singleton_method) | ||
end | ||
|
||
def preprocess_super_args(super_args) | ||
super_args.map do |node| | ||
if node.hash_type? && !node.braces? | ||
node.children | ||
else | ||
node | ||
end | ||
end.flatten | ||
end | ||
end | ||
end | ||
end | ||
end |
220 changes: 220 additions & 0 deletions
220
spec/rubocop/cop/style/redundant_super_arguments_spec.rb
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,220 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Style::RedundantSuperArguments, :config do | ||
shared_examples 'offense' do |description, args, forwarded_args = args| | ||
it "registers and corrects an offense when using def`#{description} (#{args}) => (#{forwarded_args})`" do | ||
expect_offense(<<~RUBY, forwarded_args: forwarded_args) | ||
def method(#{args}) | ||
super(#{forwarded_args}) | ||
^^^^^^^{forwarded_args}^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
def method(#{args}) | ||
super | ||
end | ||
RUBY | ||
end | ||
|
||
it "registers and corrects an offense when using defs`#{description} (#{args}) => (#{forwarded_args})`" do | ||
expect_offense(<<~RUBY, forwarded_args: forwarded_args) | ||
def self.method(#{args}) | ||
super(#{forwarded_args}) | ||
^^^^^^^{forwarded_args}^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
def self.method(#{args}) | ||
super | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
shared_examples 'no offense' do |description, args, forwarded_args = args| | ||
it "registers no offense when using def `#{description} (#{args}) => (#{forwarded_args})`" do | ||
expect_no_offenses(<<~RUBY) | ||
def method(#{args}) | ||
super(#{forwarded_args}) | ||
end | ||
RUBY | ||
end | ||
|
||
it "registers no offense when using defs `#{description} (#{args}) => (#{forwarded_args})`" do | ||
expect_no_offenses(<<~RUBY) | ||
def self.method(#{args}) | ||
super(#{forwarded_args}) | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
it_behaves_like 'offense', 'no arguments', '' | ||
it_behaves_like 'offense', 'single positional argument', 'a' | ||
it_behaves_like 'offense', 'multiple positional arguments', 'a, b' | ||
it_behaves_like 'offense', 'multiple positional arguments with default', 'a, b, c = 1', 'a, b, c' | ||
it_behaves_like 'offense', 'positional/keyword argument', 'a, b:', 'a, b: b' | ||
it_behaves_like 'offense', 'positional/keyword argument with default', 'a, b: 1', 'a, b: b' | ||
it_behaves_like 'offense', 'positional/keyword argument both with default', 'a = 1, b: 2', 'a, b: b' | ||
it_behaves_like 'offense', 'named block argument', '&blk' | ||
it_behaves_like 'offense', 'positional splat arguments', '*args' | ||
it_behaves_like 'offense', 'keyword splat arguments', '**kwargs' | ||
it_behaves_like 'offense', 'positional/keyword splat arguments', '*args, **kwargs' | ||
it_behaves_like 'offense', 'positionalkeyword splat arguments with block', '*args, **kwargs, &blk' | ||
it_behaves_like 'offense', 'keyword arguments mixed with forwarding', 'a:, **kwargs', 'a: a, **kwargs' | ||
it_behaves_like 'offense', 'tripple dot forwarding', '...' | ||
it_behaves_like 'offense', 'tripple dot forwarding with extra arg', 'a, ...' | ||
|
||
it_behaves_like 'no offense', 'different amount of positional arguments', 'a, b', 'a' | ||
it_behaves_like 'no offense', 'positional arguments in different order', 'a, b', 'b, a' | ||
it_behaves_like 'no offense', 'keyword arguments in different order', 'a:, b:', 'b: b, a: a' | ||
it_behaves_like 'no offense', 'positional/keyword argument mixing', 'a, b', 'a, b: b' | ||
it_behaves_like 'no offense', 'positional/keyword argument mixing reversed', 'a, b:', 'a, b' | ||
it_behaves_like 'no offense', 'block argument with different name', '&blk', '&other_blk' | ||
it_behaves_like 'no offense', 'keyword arguments and hash', 'a:', '{ a: a }' | ||
it_behaves_like 'no offense', 'keyword arguments with send node', 'a:, b:', 'a: a, b: c' | ||
it_behaves_like 'no offense', 'tripple dot forwarding with extra param', '...', 'a, ...' | ||
it_behaves_like 'no offense', 'tripple dot forwarding with different param', 'a, ...', 'b, ...' | ||
it_behaves_like 'no offense', 'keyword forwarding with extra keyword', 'a, **kwargs', 'a: a, **kwargs' | ||
|
||
context 'Ruby >= 3.1', :ruby31 do | ||
it_behaves_like 'offense', 'hash value omission', 'a:' | ||
it_behaves_like 'offense', 'anon block forwarding', '&' | ||
end | ||
|
||
context 'Ruby >= 3.2', :ruby32 do | ||
it_behaves_like 'offense', 'anon positional forwarding', '*' | ||
it_behaves_like 'offense', 'anon keyword forwarding', '**' | ||
|
||
it_behaves_like 'no offense', 'mixed anon forwarding', '*, **', '*' | ||
it_behaves_like 'no offense', 'mixed anon forwarding', '*, **', '**' | ||
end | ||
|
||
it 'registers no offense when explicitly passing no arguments' do | ||
expect_no_offenses(<<~RUBY) | ||
def foo(a) | ||
super() | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when passign along no arguments' do | ||
expect_offense(<<~RUBY) | ||
def foo | ||
super() | ||
^^^^^^^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for nested declarations' do | ||
expect_offense(<<~RUBY) | ||
def foo(a) | ||
def bar(b:) | ||
super(b: b) | ||
^^^^^^^^^^^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
super(a) | ||
^^^^^^^^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers no offense when calling super in a dsl method' do | ||
expect_no_offenses(<<~RUBY) | ||
describe 'example' do | ||
subject { super() } | ||
end | ||
RUBY | ||
end | ||
|
||
context 'when calling super with an extra block argument' do | ||
it 'registers no offense when calling super with no arguments' do | ||
expect_no_offenses(<<~RUBY) | ||
def test | ||
super { x } | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers no offense when calling super with implicit positional arguments' do | ||
expect_no_offenses(<<~RUBY) | ||
def test(a) | ||
super { x } | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers no offense for a method with block when calling super with positional argument' do | ||
expect_no_offenses(<<~RUBY) | ||
def test(a, &blk) | ||
super(a) { x } | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
context 'scope changes' do | ||
it 'registers no offense when the scope changes because of a class definition with block' do | ||
expect_no_offenses(<<~RUBY) | ||
def foo(a) | ||
Class.new do | ||
def foo(a, b) | ||
super(a) | ||
end | ||
end | ||
end | ||
RUBY | ||
end | ||
end | ||
|
||
it 'registers an offense when the scope changes because of a block' do | ||
expect_offense(<<~RUBY) | ||
def foo(a) | ||
bar do | ||
super(a) | ||
^^^^^^^^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when the scope changes because of a numblock' do | ||
expect_offense(<<~RUBY) | ||
def foo(a) | ||
bar do | ||
baz(_1) | ||
super(a) | ||
^^^^^^^^ Call `super` without arguments and parentheses when the signature is identical. | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers no offense when the scope changes because of sclass' do | ||
expect_no_offenses(<<~RUBY) | ||
def foo(a) | ||
class << self | ||
def foo(b) | ||
super(a) | ||
end | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers no offense when calling super in define_singleton_method' do | ||
expect_no_offenses(<<~RUBY) | ||
def test(a) | ||
define_singleton_method(:test2) do |a| | ||
super(a) | ||
end | ||
b.define_singleton_method(:test2) do |a| | ||
super(a) | ||
end | ||
end | ||
RUBY | ||
end | ||
end |