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

Performance/ArraySemiInfiniteRangeSlice breaks code on Strings #197

Closed
brushbox opened this issue Nov 23, 2020 · 2 comments · Fixed by #199
Closed

Performance/ArraySemiInfiniteRangeSlice breaks code on Strings #197

brushbox opened this issue Nov 23, 2020 · 2 comments · Fixed by #199

Comments

@brushbox
Copy link

When running rubocop-performance over our code we noticed some autocorrections broke code.
The ArraySemiInfiniteRangeSlice rule was being applied to Strings but there is no String#drop method, leading to broken code.

See the minimal .rubocop.yml and sample.rb below for details.

Note: this also applies to non-obvious strings (returned from a method) and not just string literals.

The only workaround was to disable the rule.


Expected behavior

$ rubocop -a sample.rb 
Inspecting 1 file
.

1 file inspected, no offenses detected

And running sample.rb produces:

$ ruby sample.rb
ous string

Actual behavior

$ rubocop -a sample.rb
Inspecting 1 file
C

Offenses:

sample.rb:3:6: C: [Corrected] Performance/ArraySemiInfiniteRangeSlice: Use drop instead of [] with semi-infinite range.
puts 'an obvious string'[7..]
     ^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

And running sample.rb produces:

$ ruby sample.rb
Traceback (most recent call last):
sample.rb:3:in `<main>': undefined method `drop' for "an obvious string":String (NoMethodError)

Steps to reproduce the problem

  1. Given the .rubocop.yml file:

    require:
      - rubocop-performance
    
    AllCops:
      NewCops: enable
      TargetRubyVersion: 2.7
  2. And the sample.rb file:

    # frozen_string_literal: true
    
    puts 'an obvious string'[7...]
  3. Run rubocop -a

  • the "violations" will be auto-corrected.
  1. Run ruby sample.rb
  • the code will break as there is no String#drop method.

RuboCop version

$ rubocop -V
1.3.1 (using Parser 2.7.2.0, rubocop-ast 1.1.1, running on ruby 2.7.1 x86_64-darwin19)
  - rubocop-performance 1.9.0
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 23, 2020
…as unsafe

This cop is unsafe for string slices because string does not have `#take` and `#drop` methods

Closes rubocop#197
@tejasbubane
Copy link
Contributor

Although we could change the cop to skip literal strings, it still remains unsafe for variables which we cannot determine statically if they are arrays or strings.

I have raised a PR to mark this cop as unsafe.

tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 23, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 23, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 23, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 24, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 24, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
tejasbubane added a commit to tejasbubane/rubocop-performance that referenced this issue Nov 24, 2020
This cop was created due to a mistake in microbenchmark
Refer rubocop#175 (comment)

Closes rubocop#197, rubocop#198
@koic koic closed this as completed in #199 Nov 25, 2020
koic added a commit that referenced this issue Nov 25, 2020
[Fix #197] Disable `Performance/ArraySemiInfiniteRangeSlice` cop
@brushbox
Copy link
Author

brushbox commented Nov 25, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants