Performance/Casecmp cop fails when autocorrectting #2664

tobypinder · 2016-01-18T14:17:04Z

The Performance/Casecmp cop seems to consistently fail while autocorrecting.

It works fine without --auto-correct

➜  rubocop app/models/dummy.rb --only Performance/Casecmp                              

1  Performance/Casecmp
--
1  Total

But with, it throws exception (shown with -d set for stacktrace).

➜  rubocop app/models/dummy.rb --auto-correct --only Performance/Casecmp -d
For {app}: configuration from {app}/.rubocop.yml
configuration from {rvm}/gems/ruby-2.3.0@global/gems/rubocop-rspec-1.3.1/config/default.yml
Default configuration from {rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/config/default.yml
Inheriting configuration from {rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/config/enabled.yml
Inheriting configuration from {rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/config/disabled.yml
Scanning {app}/app/models/dummy.rb
An error occurred while Performance/Casecmp cop was inspecting ../app/models/dummy.rb.
undefined method `begin' for #<Parser::Source::Map::Variable:0x00000002d994d0>
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/performance/casecmp.rb:35:in `autocorrect'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/cop.rb:179:in `correct'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/cop.rb:169:in `add_offense'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/performance/casecmp.rb:27:in `block in on_send'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/performance/casecmp.rb:21:in `downcase_eq'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/performance/casecmp.rb:25:in `on_send'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:38:in `block (2 levels) in on_send'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:88:in `with_cop_error_handling'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:37:in `block in on_send'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:35:in `each'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:35:in `on_send'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:259:in `process'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:276:in `block in process_all'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:275:in `map'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:275:in `process_all'
{rvm}/gems/ruby-2.3.0@global/gems/parser-2.3.0.1/lib/parser/ast/processor.rb:9:in `process_regular_node'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:42:in `on_begin'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:259:in `process'
{rvm}/gems/ruby-2.3.0@global/gems/parser-2.3.0.1/lib/parser/ast/processor.rb:125:in `on_def'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:42:in `on_def'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:259:in `process'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:276:in `block in process_all'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:275:in `map'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:275:in `process_all'
{rvm}/gems/ruby-2.3.0@global/gems/parser-2.3.0.1/lib/parser/ast/processor.rb:9:in `process_regular_node'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:42:in `on_class'
{rvm}/gems/ruby-2.3.0@cerebro/gems/ast-2.2.0/lib/ast/processor/mixin.rb:259:in `process'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/commissioner.rb:54:in `investigate'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cop/team.rb:37:in `inspect_file'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:194:in `inspect_file'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:164:in `block in do_inspection_loop'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:158:in `loop'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:158:in `do_inspection_loop'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:87:in `process_file'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:59:in `block in inspect_files'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:57:in `each'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:57:in `inspect_files'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/runner.rb:35:in `run'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/lib/rubocop/cli.rb:30:in `run'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/bin/rubocop:14:in `block in <top (required)>'
{rvm}/rubies/ruby-2.3.0/lib/ruby/2.3.0/benchmark.rb:308:in `realtime'
{rvm}/gems/ruby-2.3.0@cerebro/bundler/gems/rubocop-ac236f6fa81a/bin/rubocop:13:in `<top (required)>'
{rvm}/gems/ruby-2.3.0@cerebro/bin/rubocop:23:in `load'
{rvm}/gems/ruby-2.3.0@cerebro/bin/rubocop:23:in `<main>'
{rvm}/gems/ruby-2.3.0@cerebro/bin/ruby_executable_hooks:15:in `eval'
{rvm}/gems/ruby-2.3.0@cerebro/bin/ruby_executable_hooks:15:in `<main>'

--
0  Total


1 error occurred:
An error occurred while Performance/Casecmp cop was inspecting /home/toby/Documents/Code/Ruby/cerebro/app/models/dummy.rb.
Errors are usually caused by RuboCop bugs.
Please, report your problems to RuboCop's issue tracker.
Mention the following information in the issue report:
0.36.0 (using Parser 2.3.0.1, running on ruby 2.3.0 x86_64-linux)
Finished in 0.0628273000002082 seconds

Reproduce with

class Dummy
  def check?(foo, bar)
    foo.downcase == bar
  end
end

System

➜  rubocop -V
0.36.0 (using Parser 2.3.0.1, running on ruby 2.3.0 x86_64-linux)
➜  ruby -v   
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux]

Discovered in 0.36.0, Tested on master branch.

The text was updated successfully, but these errors were encountered:

jonas054 · 2016-01-22T15:20:26Z

I've found that changing foo.downcase == bar to foo.casecmp(bar) in the above example is not correct, since that's semantically equivalent only if bar is all lower case. And we don't know that.

Looks like there are a few bugs in the current implementation (apart from the crash). I'll try to sort them out.

alexdowad · 2016-01-22T15:32:18Z

I've found that changing foo.downcase == bar to foo.casecmp(bar) in the above example is not correct, since that's semantically equivalent only if bar is all lower case. And we don't know that.

If it's not all lower case, then the comparison will always be false and will be useless. Which is a strange thing to do.

You are right, though.

madwort · 2016-01-27T14:05:04Z

some more examples that produce slightly different crashes for me in my duplicate thread #2727

myothervar if myvar.downcase == myothervar

and

MY_CONSTANT if myvar.downcase == MY_CONSTANT

dbxfb · 2016-01-28T10:24:47Z

Getting the same problem, reduced the crash to str.upcase == hash[:key]

stamhankar999 · 2016-01-29T02:15:57Z

crashes aside, this just seems wrong: x.upcase == 'FOO' evaluates to true or false depending on whether or not x is equivalent to FOO regardless of case. So one would be tempted to use casecmp, but it's wrong because when the strings are not equal, casecmp returns -1 or 1 (it's a comparator after all, not a boolean method), and these are not false in Ruby. The cop should convert the above to

x.casecmp('FOO') == 0

not simply

x.casecmp('FOO')

segiddins · 2016-01-29T02:19:07Z

@stamhankar999 that's been fixed on master

stamhankar999 · 2016-01-29T02:22:30Z

Ok, thanks!

alexdowad · 2016-01-29T03:40:45Z

@bbatsov We have already received several reports on this same issue. New minor version?

bbatsov · 2016-01-29T06:28:16Z

Soon. There are a few outstanding PRs that I'd like to get merged before 0.37. I'll likely ship it over the weekend or on Monday.

alexdowad · 2016-02-05T11:53:55Z

@tobypinder Please confirm if you still have a problem with the new release.

jonas054 · 2016-02-06T10:22:39Z

For me, RuboCop no longer crahses on the given example after

f81fc06 Merge pull request #2692 from rrosenblum/casecmp_with_method

which was included in v0.37.0, so I feel confident the issue can be closed.

jonas054 added the bug label Jan 21, 2016

jonas054 self-assigned this Jan 22, 2016

jonas054 removed their assignment Jan 22, 2016

jonas054 mentioned this issue Jan 22, 2016

Casecmp order #2699

Merged

madwort mentioned this issue Jan 27, 2016

Performance/Casecmp auto-correct crashes #2727

Closed

jonas054 closed this as completed Feb 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance/Casecmp cop fails when autocorrectting #2664

Performance/Casecmp cop fails when autocorrectting #2664

tobypinder commented Jan 18, 2016

jonas054 commented Jan 22, 2016

alexdowad commented Jan 22, 2016

madwort commented Jan 27, 2016

dbxfb commented Jan 28, 2016

stamhankar999 commented Jan 29, 2016

segiddins commented Jan 29, 2016

stamhankar999 commented Jan 29, 2016

alexdowad commented Jan 29, 2016

bbatsov commented Jan 29, 2016

alexdowad commented Feb 5, 2016

jonas054 commented Feb 6, 2016

Performance/Casecmp cop fails when autocorrectting #2664

Performance/Casecmp cop fails when autocorrectting #2664

Comments

tobypinder commented Jan 18, 2016

Reproduce with

System

jonas054 commented Jan 22, 2016

alexdowad commented Jan 22, 2016

madwort commented Jan 27, 2016

dbxfb commented Jan 28, 2016

stamhankar999 commented Jan 29, 2016

segiddins commented Jan 29, 2016

stamhankar999 commented Jan 29, 2016

alexdowad commented Jan 29, 2016

bbatsov commented Jan 29, 2016

alexdowad commented Feb 5, 2016

jonas054 commented Feb 6, 2016