Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #7737] Add new Style/UnnecessaryArguments cop #7761

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -279,6 +279,7 @@
* [#7805](https://github.com/rubocop-hq/rubocop/pull/7805): Change `AllowComments` option of `Lint/SuppressedException` to true by default. ([@koic][])
* [#7320](https://github.com/rubocop-hq/rubocop/issues/7320): `Naming/MethodName` now flags `attr_reader/attr_writer/attr_accessor/attr`. ([@denys281][])
* [#7813](https://github.com/rubocop-hq/rubocop/issues/7813): **(Breaking)** Remove `Lint/EndInMethod` cop. ([@tejasbubane][])
* [#7737](https://github.com/rubocop-hq/rubocop/issues/7737): Add new `Style/RedundantArguments` cop. ([@tejasbubane][])

## 0.80.1 (2020-02-29)

Expand Down
9 changes: 9 additions & 0 deletions config/default.yml
Expand Up @@ -3660,6 +3660,15 @@ Style/RandomWithOffset:
Enabled: true
VersionAdded: '0.52'

Style/RedundantArguments:
Description: 'Check for redundant arguments to methods.'
Enabled: pending
Safe: false
VersionAdded: '0.88'
RedundantArguments:
join: ''
split: ' '

Style/RedundantAssignment:
Description: 'Checks for redundant assignment before returning.'
Enabled: 'pending'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -437,6 +437,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleproc[Style/Proc]
* xref:cops_style.adoc#styleraiseargs[Style/RaiseArgs]
* xref:cops_style.adoc#stylerandomwithoffset[Style/RandomWithOffset]
* xref:cops_style.adoc#styleredundantarguments[Style/RedundantArguments]
* xref:cops_style.adoc#styleredundantassignment[Style/RedundantAssignment]
* xref:cops_style.adoc#styleredundantbegin[Style/RedundantBegin]
* xref:cops_style.adoc#styleredundantcapitalw[Style/RedundantCapitalW]
Expand Down
55 changes: 55 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -7097,6 +7097,61 @@ rand(1...7)

* https://rubystyle.guide#random-numbers

== Style/RedundantArguments

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

| Pending
| No
| No
| 0.88
| -
|===

This cop checks for redundant arguments to methods.
Method names & arguments can be configured like:

RedundantArguments:
join: ''
split: ' '
foo: 2

Limitations:
1. This cop matches for method names only and hence cannot tell apart
methods with same name in different classes.
2. This cop is limited to methods with single parameter.

=== Examples

[source,ruby]
----
# bad

array.join('')
[1, 2, 3].join("")
string.split(" ")
"first\nsecond".split(" ")
A.foo(2)

# good
array.join
[1, 2, 3].join
string.split
"first second".split
A.foo
----

=== Configurable attributes

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

| RedundantArguments
| `{"join"=>"", "split"=>" "}`
|
|===

== Style/RedundantAssignment

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -486,6 +486,7 @@
require_relative 'rubocop/cop/style/proc'
require_relative 'rubocop/cop/style/raise_args'
require_relative 'rubocop/cop/style/random_with_offset'
require_relative 'rubocop/cop/style/redundant_arguments'
require_relative 'rubocop/cop/style/redundant_begin'
require_relative 'rubocop/cop/style/redundant_capital_w'
require_relative 'rubocop/cop/style/redundant_condition'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/correctors/percent_literal_corrector.rb
Expand Up @@ -53,7 +53,7 @@ def new_contents(node, escape, delimiters)
def autocorrect_multiline_words(node, escape, delimiters)
contents = process_multiline_words(node, escape, delimiters)
contents << end_content(node.source)
contents.join('')
contents.join
end

def autocorrect_words(node, escape, delimiters)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/lint/literal_in_interpolation.rb
Expand Up @@ -83,7 +83,7 @@ def autocorrected_value_for_symbol(node)
def autocorrected_value_for_array(node)
return node.source.gsub('"', '\"') unless node.percent_literal?

contents_range(node).source.split(' ').to_s.gsub('"', '\"')
contents_range(node).source.split.to_s.gsub('"', '\"')
end

# Does node print its own source when converted to a string?
Expand Down
69 changes: 69 additions & 0 deletions lib/rubocop/cop/style/redundant_arguments.rb
@@ -0,0 +1,69 @@
# frozen_string_literal: true

require 'parser/current'

module RuboCop
module Cop
module Style
# This cop checks for redundant arguments to methods.
# Method names & arguments can be configured like:
#
# RedundantArguments:
# join: ''
# split: ' '
# foo: 2
#
# Limitations:
# 1. This cop matches for method names only and hence cannot tell apart
# methods with same name in different classes.
# 2. This cop is limited to methods with single parameter.
#
# @example
# # bad
#
# array.join('')
# [1, 2, 3].join("")
# string.split(" ")
# "first\nsecond".split(" ")
# A.foo(2)
#
# # good
# array.join
# [1, 2, 3].join
# string.split
# "first second".split
# A.foo
class RedundantArguments < Cop
MSG = 'Argument is redundant.'

def on_send(node)
return unless redundant_argument?(node)

add_offense(node)
end

private

def redundant_argument?(node)
redundant_argument = redundant_arg_for_method(node.method_name.to_s)
return false if redundant_argument.nil?

node.arguments.first == redundant_argument
end

def redundant_arg_for_method(method_name)
return nil unless cop_config['RedundantArguments'].key?(method_name)

@mem ||= {}
@mem[method_name] ||= begin
arg = cop_config['RedundantArguments'].fetch(method_name)
buffer = Parser::Source::Buffer.new('(string)', 1)
buffer.source = arg.inspect
builder = RuboCop::AST::Builder.new
Parser::CurrentRuby.new(builder).parse(buffer)
end
end
end
end
end
end
120 changes: 120 additions & 0 deletions spec/rubocop/cop/style/redundant_arguments_spec.rb
@@ -0,0 +1,120 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::RedundantArguments, :config do
subject(:cop) { described_class.new(config) }

context 'join' do
let(:cop_config) do
{ 'RedundantArguments' => { 'join' => '' } }
end

it 'registers an offense when using `#join` with empty string argument' do
expect_offense(<<~RUBY)
foo.join('')
^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'registers an offense when using `#join` with double quoted string' do
expect_offense(<<~RUBY)
foo.join("")
^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'registers an offense when using `#join` on literal arrays' do
expect_offense(<<~RUBY)
[1, 2, 3].join("")
^^^^^^^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'does not register an offense when using `#join` with no arguments' do
expect_no_offenses(<<~RUBY)
foo.join
RUBY
end

it 'does not register an offense when using `#join` with array literals' do
expect_no_offenses(<<~RUBY)
[1, 2, 3].join
RUBY
end

it 'does not register an offense when using `#join` with separator' do
expect_no_offenses(<<~RUBY)
foo.join(',')
RUBY
end
end

context 'split' do
let(:cop_config) do
{ 'RedundantArguments' => { 'split' => ' ' } }
end

it 'registers an offense when using `#split` with space' do
expect_offense(<<~RUBY)
foo.split(' ')
^^^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'registers an offense when using `#split` on string literal' do
expect_offense(<<~RUBY)
"first second".split(' ')
^^^^^^^^^^^^^^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'registers an offense when using `#join` with double quoted string' do
expect_offense(<<~RUBY)
foo.split(" ")
^^^^^^^^^^^^^^ Argument is redundant.
RUBY
end

it 'does not register an offense when using `#split` with no arguments' do
expect_no_offenses(<<~RUBY)
foo.split
RUBY
end

it 'does not register an offense when using `#split` with string literal' do
expect_no_offenses(<<~RUBY)
"first second".split
RUBY
end

it 'does not register an offense when using `#split` with separator' do
expect_no_offenses(<<~RUBY)
foo.split(',')
RUBY
end

it 'does not register an offense when using `#split` with empty string' do
expect_no_offenses(<<~RUBY)
foo.split('')
RUBY
end
end

context 'non-builtin method' do
let(:cop_config) do
{ 'RedundantArguments' => { 'foo' => 2 } }
end

it 'registers an offense with configured argument' do
expect_offense(<<~RUBY)
A.foo(2)
^^^^^^^^ Argument is redundant.
RUBY
end

it 'does not register an offense with other argument' do
expect_no_offenses(<<~RUBY)
A.foo(5)
RUBY
end
end
end