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/DeleteSuffix is not safe #245

Closed
jmkoni opened this issue May 7, 2021 · 1 comment · Fixed by #246
Closed

Performance/DeleteSuffix is not safe #245

jmkoni opened this issue May 7, 2021 · 1 comment · Fixed by #246

Comments

@jmkoni
Copy link

jmkoni commented May 7, 2021

We had a user of StandardRB open up standardrb/standard#292.

Expected behavior

Currently, if you are using sub to remove a suffix on a Pathname object, it replaces it with delete_suffix, which is not a valid method on a Pathname object. I would expect the following not to change:

path = Pathname.new("path/to/some/ruby/file.rb").sub(/\.rb\z/, "")

This should either not change or the cop should be marked as not safe.

Actual behavior

It changes the above code to:

path = Pathname.new("path/to/some/ruby/file.rb").delete_suffix(".rb")

which results in the following error:

Traceback (most recent call last):
standardrb_bug.rb:3:in `<main>': undefined method `delete_suffix' for #<Pathname:path/to/some/ruby/file.rb> (NoMethodError)

Steps to reproduce the problem

Put the above code in a ruby file, fix the file with Performance/DeleteSuffix enabled. Then try to run the file.

RuboCop version

rubocop 1.14.0
rubocop-performance 1.11.2
koic added a commit to koic/rubocop-performance that referenced this issue May 9, 2021
…leteSuffix` as unsafe

Fixes rubocop#245.

This PR marks `Performance/DeletePrefix` and `Performance/DeleteSuffix` as unsafe.
@koic
Copy link
Member

koic commented May 9, 2021

@jmkoni Thank you for your feedback! JFYI, Performance/DeletePrefix cop has the same false positive.

koic added a commit to koic/rubocop-performance that referenced this issue May 9, 2021
…leteSuffix` as unsafe

Fixes rubocop#245.

This PR marks `Performance/DeletePrefix` and `Performance/DeleteSuffix` as unsafe.
koic added a commit to koic/rubocop-performance that referenced this issue May 9, 2021
…leteSuffix` as unsafe

Fixes rubocop#245.

This PR marks `Performance/DeletePrefix` and `Performance/DeleteSuffix` as unsafe.
@koic koic closed this as completed in #246 May 16, 2021
koic added a commit that referenced this issue May 16, 2021
…x_as_unsafe

[Fix #245] Mark `Performance/DeletePrefix` and `Performance/DeleteSuffix` as unsafe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants