Skip to content

Commit

Permalink
Merge pull request #7646 from koic/add_style_arguments_forwarding
Browse files Browse the repository at this point in the history
[Fix #7549] Add `Style/ArgumentsForwarding` cop
  • Loading branch information
koic committed Oct 22, 2020
2 parents 437216b + 5f75bac commit 26dfdf8
Show file tree
Hide file tree
Showing 7 changed files with 458 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### New features

* [#8895](https://github.com/rubocop-hq/rubocop/pull/8895): Add new `Lint/EmptyBlock` cop. ([@fatkodima][])
* [#7549](https://github.com/rubocop-hq/rubocop/issues/7549): Add new `Style/ArgumentsForwarding` cop. ([@koic][])

## 1.0.0 (2020-10-21)

Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -2484,6 +2484,13 @@ Style/AndOr:
- always
- conditionals

Style/ArgumentsForwarding:
Description: 'Use arguments forwarding.'
StyleGuide: '#arguments-forwarding'
Enabled: pending
AllowOnlyRestArgument: true
VersionAdded: '1.1'

Style/ArrayCoercion:
Description: >-
Use Array() instead of explicit Array check or [*var], when dealing
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -337,6 +337,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleaccessorgrouping[Style/AccessorGrouping]
* xref:cops_style.adoc#stylealias[Style/Alias]
* xref:cops_style.adoc#styleandor[Style/AndOr]
* xref:cops_style.adoc#styleargumentsforwarding[Style/ArgumentsForwarding]
* xref:cops_style.adoc#stylearraycoercion[Style/ArrayCoercion]
* xref:cops_style.adoc#stylearrayjoin[Style/ArrayJoin]
* xref:cops_style.adoc#styleasciicomments[Style/AsciiComments]
Expand Down
75 changes: 75 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -293,6 +293,81 @@ end

* https://rubystyle.guide#no-and-or-or

== Style/ArgumentsForwarding

NOTE: Required Ruby version: 2.7

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 1.1
| -
|===

In Ruby 2.7, arguments forwarding has been added.

This cop identifies places where `do_something(*args, &block)`
can be replaced by `do_something(...)`.

=== Examples

[source,ruby]
----
# bad
def foo(*args, &block)
bar(*args, &block)
end
# bad
def foo(*args, **kwargs, &block)
bar(*args, **kwargs, &block)
end
# good
def foo(...)
bar(...)
end
----

==== AllowOnlyRestArgument: true (default)

[source,ruby]
----
# good
def foo(*args)
bar(*args)
end
----

==== AllowOnlyRestArgument: false

[source,ruby]
----
# bad
# The following code can replace the arguments with `...`,
# but it will change the behavior. Because `...` forwards block also.
def foo(*args)
bar(*args)
end
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| AllowOnlyRestArgument
| `true`
| Boolean
|===

=== References

* https://rubystyle.guide#arguments-forwarding

== Style/ArrayCoercion

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -385,6 +385,7 @@
require_relative 'rubocop/cop/style/accessor_grouping'
require_relative 'rubocop/cop/style/alias'
require_relative 'rubocop/cop/style/and_or'
require_relative 'rubocop/cop/style/arguments_forwarding'
require_relative 'rubocop/cop/style/array_coercion'
require_relative 'rubocop/cop/style/array_join'
require_relative 'rubocop/cop/style/ascii_comments'
Expand Down
142 changes: 142 additions & 0 deletions lib/rubocop/cop/style/arguments_forwarding.rb
@@ -0,0 +1,142 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# In Ruby 2.7, arguments forwarding has been added.
#
# This cop identifies places where `do_something(*args, &block)`
# can be replaced by `do_something(...)`.
#
# @example
# # bad
# def foo(*args, &block)
# bar(*args, &block)
# end
#
# # bad
# def foo(*args, **kwargs, &block)
# bar(*args, **kwargs, &block)
# end
#
# # good
# def foo(...)
# bar(...)
# end
#
# @example AllowOnlyRestArgument: true (default)
# # good
# def foo(*args)
# bar(*args)
# end
#
# @example AllowOnlyRestArgument: false
# # bad
# # The following code can replace the arguments with `...`,
# # but it will change the behavior. Because `...` forwards block also.
# def foo(*args)
# bar(*args)
# end
#
class ArgumentsForwarding < Base
include RangeHelp
extend AutoCorrector
extend TargetRubyVersion

minimum_target_ruby_version 2.7

MSG = 'Use arguments forwarding.'

def_node_matcher :use_rest_arguments?, <<~PATTERN
(args (restarg $_) $...)
PATTERN

def_node_matcher :only_rest_arguments?, <<~PATTERN
(send _ _ (splat (lvar %1)))
PATTERN

def_node_matcher :forwarding_method_arguments?, <<~PATTERN
{
(send _ _
(splat (lvar %1))
(block-pass (lvar %2)))
(send _ _
(splat (lvar %1))
(hash (kwsplat (lvar %3)))
(block-pass (lvar %2)))
}
PATTERN

def on_def(node)
return unless node.body
return unless (rest_args_name, args = use_rest_arguments?(node.arguments))

node.each_descendant(:send) do |send_node|
kwargs_name, block_name = extract_argument_names_from(args)

next unless forwarding_method?(send_node, rest_args_name, kwargs_name, block_name) &&
all_lvars_as_forwarding_method_arguments?(node, send_node)

register_offense_to_forwarding_method_arguments(send_node)
register_offense_to_method_definition_arguments(node)
end
end
alias on_defs on_def

private

def extract_argument_names_from(args)
kwargs_name = args.first.source.delete('**') if args.first&.kwrestarg_type?
block_arg_name = args.last.source.delete('&') if args.last&.blockarg_type?

[kwargs_name, block_arg_name].map { |name| name&.to_sym }
end

def forwarding_method?(node, rest_arg, kwargs, block_arg)
return only_rest_arguments?(node, rest_arg) unless allow_only_rest_arguments?

forwarding_method_arguments?(node, rest_arg, block_arg, kwargs)
end

def all_lvars_as_forwarding_method_arguments?(def_node, forwarding_method)
lvars = def_node.body.each_descendant(:lvar, :lvasgn)

begin_pos = forwarding_method.source_range.begin_pos
end_pos = forwarding_method.source_range.end_pos

lvars.all? do |lvar|
lvar.source_range.begin_pos.between?(begin_pos, end_pos)
end
end

def register_offense_to_forwarding_method_arguments(forwarding_method)
add_offense(arguments_range(forwarding_method)) do |corrector|
range = range_between(
forwarding_method.loc.selector.end_pos, forwarding_method.source_range.end_pos
)
corrector.replace(range, '(...)')
end
end

def register_offense_to_method_definition_arguments(method_definition)
add_offense(arguments_range(method_definition)) do |corrector|
arguments_range = range_with_surrounding_space(
range: method_definition.arguments.source_range, side: :left
)
corrector.replace(arguments_range, '(...)')
end
end

def arguments_range(node)
arguments = node.arguments

range_between(arguments.first.source_range.begin_pos, arguments.last.source_range.end_pos)
end

def allow_only_rest_arguments?
cop_config.fetch('AllowOnlyRestArgument', true)
end
end
end
end
end

0 comments on commit 26dfdf8

Please sign in to comment.