From f7d93e55a74d9b99e4dfca0a90d250b5d5873ba4 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Wed, 22 May 2024 18:15:46 +0200 Subject: [PATCH] [Fix #12309] Add new `Style/ZeroAritySuper` cop --- .../new_add_new_style_super_arguments_cop.md | 1 + config/default.yml | 5 + lib/rubocop.rb | 1 + lib/rubocop/cop/style/super_arguments.rb | 144 ++++++++++++ .../rubocop/cop/style/super_arguments_spec.rb | 220 ++++++++++++++++++ 5 files changed, 371 insertions(+) create mode 100644 changelog/new_add_new_style_super_arguments_cop.md create mode 100644 lib/rubocop/cop/style/super_arguments.rb create mode 100644 spec/rubocop/cop/style/super_arguments_spec.rb diff --git a/changelog/new_add_new_style_super_arguments_cop.md b/changelog/new_add_new_style_super_arguments_cop.md new file mode 100644 index 000000000000..4c3c28c0d732 --- /dev/null +++ b/changelog/new_add_new_style_super_arguments_cop.md @@ -0,0 +1 @@ +* [#12309](https://github.com/rubocop/rubocop/issues/12309): Add new `Style/SuperArguments` cop. ([@earlopain][]) diff --git a/config/default.yml b/config/default.yml index 8080a3a77dad..db2afd366a26 100644 --- a/config/default.yml +++ b/config/default.yml @@ -5424,6 +5424,11 @@ Style/StructInheritance: VersionAdded: '0.29' VersionChanged: '1.20' +Style/SuperArguments: + Description: 'Call `super` without arguments and parentheses when the signature is identical.' + Enabled: pending + VersionAdded: '<>' + Style/SuperWithArgsParentheses: Description: 'Use parentheses for `super` with arguments.' StyleGuide: '#super-with-args' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 5c9f67275bd0..4a14cfc5e9eb 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -683,6 +683,7 @@ require_relative 'rubocop/cop/style/string_methods' require_relative 'rubocop/cop/style/strip' require_relative 'rubocop/cop/style/struct_inheritance' +require_relative 'rubocop/cop/style/super_arguments' require_relative 'rubocop/cop/style/super_with_args_parentheses' require_relative 'rubocop/cop/style/swap_values' require_relative 'rubocop/cop/style/symbol_array' diff --git a/lib/rubocop/cop/style/super_arguments.rb b/lib/rubocop/cop/style/super_arguments.rb new file mode 100644 index 000000000000..3925e8a270b5 --- /dev/null +++ b/lib/rubocop/cop/style/super_arguments.rb @@ -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 SuperArguments < 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 diff --git a/spec/rubocop/cop/style/super_arguments_spec.rb b/spec/rubocop/cop/style/super_arguments_spec.rb new file mode 100644 index 000000000000..e86ce80764f8 --- /dev/null +++ b/spec/rubocop/cop/style/super_arguments_spec.rb @@ -0,0 +1,220 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::SuperArguments, :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