From 21e40628e1279e3d5d647b6e13992b2e764ef7b6 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sun, 25 Oct 2020 21:55:17 +0200 Subject: [PATCH] Add new `Lint/ToEnumArguments` cop --- changelog/new_to_enum_arguments_cop.md | 1 + config/default.yml | 5 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_lint.adoc | 40 ++++++ lib/rubocop.rb | 1 + lib/rubocop/cop/lint/to_enum_arguments.rb | 94 ++++++++++++ .../cop/lint/to_enum_arguments_spec.rb | 134 ++++++++++++++++++ 7 files changed, 276 insertions(+) create mode 100644 changelog/new_to_enum_arguments_cop.md create mode 100644 lib/rubocop/cop/lint/to_enum_arguments.rb create mode 100644 spec/rubocop/cop/lint/to_enum_arguments_spec.rb diff --git a/changelog/new_to_enum_arguments_cop.md b/changelog/new_to_enum_arguments_cop.md new file mode 100644 index 00000000000..da5659d2191 --- /dev/null +++ b/changelog/new_to_enum_arguments_cop.md @@ -0,0 +1 @@ +* [#7753](https://github.com/rubocop-hq/rubocop/issues/7753): Add new `Lint/ToEnumArguments` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index 47ab11b7edd..4dc2688dd18 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1921,6 +1921,11 @@ Lint/Syntax: VersionAdded: '0.9' +Lint/ToEnumArguments: + Description: 'This cop ensures that `to_enum`/`enum_for`, called for the current method, has correct arguments.' + Enabled: pending + VersionAdded: '1.1' + Lint/ToJSON: Description: 'Ensure #to_json includes an optional argument.' Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index a761ca25e75..b36cc370fd6 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -270,6 +270,7 @@ In the following section you find all available cops: * xref:cops_lint.adoc#lintstructnewoverride[Lint/StructNewOverride] * xref:cops_lint.adoc#lintsuppressedexception[Lint/SuppressedException] * xref:cops_lint.adoc#lintsyntax[Lint/Syntax] +* xref:cops_lint.adoc#linttoenumarguments[Lint/ToEnumArguments] * xref:cops_lint.adoc#linttojson[Lint/ToJSON] * xref:cops_lint.adoc#linttoplevelreturnwithargument[Lint/TopLevelReturnWithArgument] * xref:cops_lint.adoc#linttrailingcommainattributedeclaration[Lint/TrailingCommaInAttributeDeclaration] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index ba2d3b56366..d252df494e1 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -4025,6 +4025,46 @@ end This cop repacks Parser's diagnostics/errors into RuboCop's offenses. +== Lint/ToEnumArguments + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 1.1 +| - +|=== + +This cop ensures that `to_enum`/`enum_for`, called for the current method, +has correct arguments. + +=== Examples + +[source,ruby] +---- +# bad +def method(x, y = 1) + return to_enum(__method__, x) # `y` is missing +end + +# good +def method(x, y = 1) + return to_enum(__method__, x, y) +end + +# bad +def method(required:) + return to_enum(:method, required: something) # `required` has incorrect value +end + +# good +def method(required:) + return to_enum(:method, required: required) +end +---- + == Lint/ToJSON |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index c41bb147219..bb3569ca409 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -333,6 +333,7 @@ require_relative 'rubocop/cop/lint/struct_new_override' require_relative 'rubocop/cop/lint/suppressed_exception' require_relative 'rubocop/cop/lint/syntax' +require_relative 'rubocop/cop/lint/to_enum_arguments' require_relative 'rubocop/cop/lint/to_json' require_relative 'rubocop/cop/lint/top_level_return_with_argument' require_relative 'rubocop/cop/lint/trailing_comma_in_attribute_declaration' diff --git a/lib/rubocop/cop/lint/to_enum_arguments.rb b/lib/rubocop/cop/lint/to_enum_arguments.rb new file mode 100644 index 00000000000..4fc8df8a735 --- /dev/null +++ b/lib/rubocop/cop/lint/to_enum_arguments.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop ensures that `to_enum`/`enum_for`, called for the current method, + # has correct arguments. + # + # @example + # # bad + # def method(x, y = 1) + # return to_enum(__method__, x) # `y` is missing + # end + # + # # good + # def method(x, y = 1) + # return to_enum(__method__, x, y) + # end + # + # # bad + # def method(required:) + # return to_enum(:method, required: something) # `required` has incorrect value + # end + # + # # good + # def method(required:) + # return to_enum(:method, required: required) + # end + # + class ToEnumArguments < Base + MSG = 'Ensure you correctly provided all the arguments.' + + RESTRICT_ON_SEND = %i[to_enum enum_for].freeze + + def_node_matcher :enum_conversion_call?, <<~PATTERN + (send {nil? self} {:to_enum :enum_for} $_ $...) + PATTERN + + def_node_matcher :method_name?, <<~PATTERN + {(send nil? :__method__) (sym %1)} + PATTERN + + def_node_matcher :passing_keyword_arg?, <<~PATTERN + (pair (sym %1) (lvar %1)) + PATTERN + + # TODO: add support for argument forwarding (`...`) when ruby 3.0 is released + def on_send(node) + def_node = node.each_ancestor(:def, :defs).first + return unless def_node + + enum_conversion_call?(node) do |method_node, arguments| + add_offense(node) unless method_name?(method_node, def_node.method_name) && + arguments_match?(arguments, def_node) + end + end + + private + + def arguments_match?(arguments, def_node) + index = 0 + + def_node.arguments.reject(&:blockarg_type?).all? do |def_arg| + send_arg = arguments[index] + case def_arg.type + when :arg, :restarg, :optarg + index += 1 + end + + send_arg && argument_match?(send_arg, def_arg) + end + end + + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity + def argument_match?(send_arg, def_arg) + def_arg_name = def_arg.children[0] + + case def_arg.type + when :arg, :restarg + send_arg.source == def_arg.source + when :optarg + send_arg.source == def_arg_name.to_s + when :kwoptarg, :kwarg + send_arg.hash_type? && + send_arg.pairs.any? { |pair| passing_keyword_arg?(pair, def_arg_name) } + when :kwrestarg + send_arg.each_child_node(:kwsplat).any? { |child| child.source == def_arg.source } + end + end + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity + end + end + end +end diff --git a/spec/rubocop/cop/lint/to_enum_arguments_spec.rb b/spec/rubocop/cop/lint/to_enum_arguments_spec.rb new file mode 100644 index 00000000000..39a3ac0b5cc --- /dev/null +++ b/spec/rubocop/cop/lint/to_enum_arguments_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::ToEnumArguments do + subject(:cop) { described_class.new } + + it 'registers an offense when required arg is missing' do + expect_offense(<<~RUBY) + def m(x) + return to_enum(:m) unless block_given? + ^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when optional arg is missing' do + expect_offense(<<~RUBY) + def m(x, y = 1) + return to_enum(:m, x) unless block_given? + ^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when splat arg is missing' do + expect_offense(<<~RUBY) + def m(x, y = 1, *args) + return to_enum(:m, x, y) unless block_given? + ^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when required keyword arg is missing' do + expect_offense(<<~RUBY) + def m(x, y = 1, *args, required:) + return to_enum(:m, x, y, *args) unless block_given? + ^^^^^^^^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when optional keyword arg is missing' do + expect_offense(<<~RUBY) + def m(x, y = 1, *args, required:, optional: true) + return to_enum(:m, x, y, *args, required: required) unless block_given? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when splat keyword arg is missing' do + expect_offense(<<~RUBY) + def m(x, y = 1, *args, required:, optional: true, **kwargs) + return to_enum(:m, x, y, *args, required: required, optional: optional) unless block_given? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when arguments are swapped' do + expect_offense(<<~RUBY) + def m(x, y = 1) + return to_enum(:m, y, x) unless block_given? + ^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when other values are passed for keyword arguments' do + expect_offense(<<~RUBY) + def m(required:, optional: true) + return to_enum(:m, required: something_else, optional: optional) unless block_given? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'does not register an offense when not inside method definition' do + expect_no_offenses(<<~RUBY) + to_enum(:m) + RUBY + end + + it 'does not register an offense when method call has a receiver other than `self`' do + expect_no_offenses(<<~RUBY) + def m(x) + return foo.to_enum(:m) unless block_given? + end + RUBY + end + + it 'registers an offense when method is called on `self`' do + expect_offense(<<~RUBY) + def m(x) + return self.to_enum(:m) unless block_given? + ^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'ignores the block argument' do + expect_no_offenses(<<~RUBY) + def m(x, &block) + return to_enum(:m, x) unless block_given? + end + RUBY + end + + it 'registers an offense when enumerator is created for another method' do + expect_offense(<<~RUBY) + def m(x) + return to_enum(:not_m) unless block_given? + ^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'registers an offense when enumerator is created for `__method__`' do + expect_offense(<<~RUBY) + def m(x) + return to_enum(__method__) unless block_given? + ^^^^^^^^^^^^^^^^^^^ Ensure you correctly provided all the arguments. + end + RUBY + end + + it 'does not register an offense when enumerator is created with the correct arguments' do + expect_no_offenses(<<~RUBY) + def m(x, y = 1, *args, required:, optional: true, **kwargs, &block) + return to_enum(:m, x, y, *args, required: required, optional: optional, **kwargs) unless block_given? + end + RUBY + end +end