Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

RuboCop::Cop::Performance::Casecmp broke my code #101

Closed
hqm42 opened this issue Apr 6, 2020 · 1 comment · Fixed by #104
Closed

RuboCop::Cop::Performance::Casecmp broke my code #101

hqm42 opened this issue Apr 6, 2020 · 1 comment · Fixed by #104

Comments

@hqm42
Copy link

hqm42 commented Apr 6, 2020

a.casecmp(b).zero? is not an universal replacement for a.downcase == b.downcase

casecmp only supports characters A-Za-z. downcase supports special characters as well "ÄÖÜ".downcase == "äöü"

Expected behavior

Rubocop does not change the semantics of my program.

Actual behavior

Rubocop breaks my code.

Steps to reproduce the problem

foo.rb

# frozen_string_literal: true

if 'Grün'.downcase == 'GRÜN'.downcase
  puts 'this should be the same'
else
  puts 'rubocop broke my code'
end

rubocop -a foo.rb

foo.rb

# frozen_string_literal: true

if 'Grün'.casecmp('GRÜN').zero?
  puts 'this should be the same'
else
  puts 'rubocop broke my code'
end

RuboCop version

> bundle exec rubocop -V
0.80.0 (using Parser 2.7.0.3, running on ruby 2.5.3 x86_64-linux)
koic added a commit to koic/rubocop-performance that referenced this issue May 4, 2020
@koic
[Fix rubocop#101] Mark unsafe for Performance/Casecmp cop
aed1098 
Fixes rubocop#101.

This PR marks unsafe for `Performance/Casecmp` cop.

`String#casecmp` and `String#casecmp?` behave differently
when using Non-ASCII characters.

```console
'äöü'.casecmp('ÄÖÜ').zero? #=> false
'äöü'.casecmp?('ÄÖÜ')      #=> true
```

I thought about using a String literal encoding to
prevent false positives. But, it is difficult to
static analyze the encoding of strings passed from outside.
As a result, I decided to mark it as unsafe.
koic added a commit to koic/rubocop-performance that referenced this issue May 4, 2020
@koic
[Fix rubocop#101] Mark unsafe for Performance/Casecmp cop
7899e80 
Fixes rubocop#101.

This PR marks unsafe for `Performance/Casecmp` cop.

`String#casecmp` and `String#casecmp?` behave differently
when using Non-ASCII characters.

```console
'äöü'.casecmp('ÄÖÜ').zero? #=> false
'äöü'.casecmp?('ÄÖÜ')      #=> true
```

I thought about using a String literal encoding to
prevent false positives. But, it is difficult to
static analyze the encoding of strings passed from outside.
As a result, this PR marks unsafe for `Performance/Casecmp` cop.
koic added a commit to koic/rubocop-performance that referenced this issue May 4, 2020
@koic
[Fix rubocop#101] Mark unsafe for Performance/Casecmp cop
e2c1647 
Fixes rubocop#101.

`String#casecmp` and `String#casecmp?` behave differently
when using Non-ASCII characters.

```console
'äöü'.casecmp('ÄÖÜ').zero? #=> false
'äöü'.casecmp?('ÄÖÜ')      #=> true
```

I thought about using a String literal encoding to
prevent false positives. But, it is difficult to
static analyze the encoding of strings passed from outside.
As a result, this PR marks unsafe for `Performance/Casecmp` cop.
@koic koic mentioned this issue May 4, 2020
Merged
8 tasks
@koic
Copy link
Member

koic commented May 4, 2020

Thanks for the feedback. I opened a PR #104.

@koic koic closed this as completed in #104 May 7, 2020
koic added a commit that referenced this issue May 7, 2020
@koic
Merge pull request #104 from koic/mark_unsafe_for_performance_casecmp
c4d4496 
[Fix #101] Mark unsafe for `Performance/Casecmp` cop
@parkerfinch parkerfinch mentioned this issue Apr 29, 2021
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #101] Mark unsafe for Performance/Casecmp cop koic/rubocop-performance
2 participants
@koic @hqm42