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

Style/RedundantArgument report for cases like foo.split(' ', 2) #9083

Closed
ShockwaveNN opened this issue Nov 23, 2020 · 7 comments · Fixed by #9085
Closed

Style/RedundantArgument report for cases like foo.split(' ', 2) #9083

ShockwaveNN opened this issue Nov 23, 2020 · 7 comments · Fixed by #9085
Labels
bug good first issue Easy task, suitable for newcomers to the project

Comments

@ShockwaveNN
Copy link
Contributor

Seems Style/RedundantArgument didn't know about second argument limit for split method


Steps to reproduce the problem

  1. Create simple test.rb script with content:
foo = 'a b c d e'
bar = foo.split(' ', 2)
puts(bar[0] == 'a')
puts(bar[1] == 'b c d e')
  1. Run rubocop on it and get:
rubocop -A test.rb 
Inspecting 1 file
C

Offenses:

test.rb:2:7: C: Style/RedundantArgument: Argument ' ' is redundant because it is implied by default.
bar = foo.split(' ', 2)
      ^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

Seems in that case this cop should just ignore it, if limit is set

RuboCop version

rubocop -V
1.4.0 (using Parser 2.7.2.0, rubocop-ast 1.1.1, running on ruby 2.7.2 x86_64-linux)
  - rubocop-performance 1.9.0
  - rubocop-rspec 2.0.0

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 23, 2020

The cop was supposed ignore method calls with more than 1 argument, but I guess the check and the test case is missing. //cc @tejasbubane

@tejasbubane
Copy link
Contributor

The cop was supposed ignore method calls with more than 1 argument, but I guess the check and the test case is missing. //cc @tejasbubane

Right I missed that check.

tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Nov 23, 2020
…for more than one argument

Also
* Add check for explicit receiver must be present.
* Refactor tests for the cop.

Closes rubocop#9083
bbatsov pushed a commit that referenced this issue Nov 23, 2020
…in `Style/RedundantArgument` (#9085)

Also:

* Add a check that an explicit receiver must be present.
* Refactor tests for the cop.
@avdv
Copy link

avdv commented Nov 24, 2020

Hi.

It seems like the fix did not make it into the 1.4.1 release, although it is listed in the CHANGELOG:
https://github.com/rubocop-hq/rubocop/blob/master/CHANGELOG.md#141-2020-11-23

I just hit this problem with 1.4.1:

Resolving dependencies...
...
Using rake 13.0.1
Fetching rubocop 1.4.1
...
lib/colorls/git.rb:19:24: C: Style/RedundantArgument: Argument ' ' is redundant because it is implied by default.

        mode, file = status_line.chomp("\x0").split(' ', 2)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@tejasbubane
Copy link
Contributor

@avdv Right. This bug came in just after 1.4.1 was released. The fix has been merged but will be released in the next version.

@avdv
Copy link

avdv commented Nov 24, 2020

@avdv Right. This bug came in just after 1.4.1 was released. The fix has been merged but will be released in the next version.

It's just that I am using depend-a-bot which sources the changelog from the changelog.md file of a gem's repository and hitting the same bug it says is fixed on the tin is ... surprising. 😄

Waiting for the next release then.

@tejasbubane
Copy link
Contributor

tejasbubane commented Nov 24, 2020

@avdv Ok something seems wrong with the changelog. Commits here show Cut 1.4.1 before merging this PR.

cc @bbatsov

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 24, 2020

Yeah, it seems I didn't notice that the changelog entry was misplaced. That's why these days it's discouraged to edit the changelog directly and we're mostly generating it from the files under changelog/. I've fixed it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Easy task, suitable for newcomers to the project
Projects
None yet
4 participants