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

Clobbering error from Lint/EmptyConditionalBody #11132

Closed
r7kamura opened this issue Oct 28, 2022 · 2 comments · Fixed by #11133
Closed

Clobbering error from Lint/EmptyConditionalBody #11132

r7kamura opened this issue Oct 28, 2022 · 2 comments · Fixed by #11133

Comments

@r7kamura
Copy link
Contributor

r7kamura commented Oct 28, 2022

I ran rubocop to the following code and encountered an error.

# frozen_string_literal: true

def foo
  if condition
  else
  end
end

Since the above example is the most minimal example, it may seem that it is an edge case that almost never happens in real life. In a real application, this kind of problem would be possible with code like the following:

def foo
  if condition1
  elsif condition2
    bar
  elsif condition3
    baz
  else
  end
end

Expected behavior

Expect that no internal error is raised.

Actual behavior

$ bundle exec rubocop example.rb -d
For /tmp/example: configuration from /home/r7kamura/.rubocop.yml
Default configuration from /home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/config/default.yml
Use parallel by default.
Skipping parallel inspection: only a single file needs inspection
Inspecting 1 file
Scanning /tmp/example/example.rb
An error occurred while Lint/EmptyConditionalBody cop was inspecting /tmp/example/example.rb:4:2.
Parser::Source::TreeRewriter detected clobbering
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter.rb:427:in `trigger_policy'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter.rb:414:in `enforce_policy'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:233:in `call'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:233:in `swallow'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:97:in `with'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:124:in `place_in_hierarchy'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:106:in `do_combine'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:117:in `place_in_hierarchy'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:106:in `do_combine'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter/action.rb:30:in `combine'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter.rb:400:in `combine'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/parser-3.1.2.1/lib/parser/source/tree_rewriter.rb:194:in `replace'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/lint/empty_conditional_body.rb:110:in `correct_other_branches'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/lint/empty_conditional_body.rb:84:in `autocorrect'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/lint/empty_conditional_body.rb:75:in `block in on_if'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/base.rb:346:in `correct'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/base.rb:127:in `add_offense'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/lint/empty_conditional_body.rb:74:in `on_if'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:100:in `public_send'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:100:in `block (2 levels) in trigger_responding_cops'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:160:in `with_cop_error_handling'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:99:in `block in trigger_responding_cops'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:98:in `each'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:98:in `trigger_responding_cops'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:69:in `on_if'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-ast-1.23.0/lib/rubocop/ast/traversal.rb:153:in `on_def'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:71:in `on_def'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-ast-1.23.0/lib/rubocop/ast/traversal.rb:20:in `walk'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/commissioner.rb:86:in `investigate'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/team.rb:155:in `investigate_partial'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cop/team.rb:83:in `investigate'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:315:in `inspect_file'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:259:in `block in do_inspection_loop'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:293:in `block in iterate_until_no_changes'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:286:in `loop'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:286:in `iterate_until_no_changes'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:255:in `do_inspection_loop'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:138:in `block in file_offenses'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:163:in `file_offense_cache'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:137:in `file_offenses'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:128:in `process_file'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:109:in `block in each_inspected_file'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:108:in `each'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:108:in `reduce'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:108:in `each_inspected_file'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:94:in `inspect_files'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/runner.rb:47:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/command/execute_runner.rb:26:in `block in execute_runner'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/command/execute_runner.rb:52:in `with_redirect'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/command/execute_runner.rb:25:in `execute_runner'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/command/execute_runner.rb:17:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/command.rb:11:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli/environment.rb:18:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli.rb:72:in `run_command'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli.rb:79:in `execute_runners'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/lib/rubocop/cli.rb:48:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/exe/rubocop:19:in `block in <top (required)>'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/3.1.0/benchmark.rb:311:in `realtime'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.37.1/exe/rubocop:19:in `<top (required)>'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/bin/rubocop:25:in `load'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/bin/rubocop:25:in `<top (required)>'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli/exec.rb:58:in `load'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli/exec.rb:58:in `kernel_load'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli/exec.rb:23:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli.rb:486:in `exec'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli.rb:31:in `dispatch'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/cli.rb:25:in `start'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/exe/bundle:48:in `block in <top (required)>'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/lib/bundler/friendly_errors.rb:120:in `with_friendly_errors'
/home/r7kamura/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/bundler-2.3.23/exe/bundle:36:in `<top (required)>'
/home/r7kamura/.rbenv/versions/3.1.2/bin/bundle:25:in `load'
/home/r7kamura/.rbenv/versions/3.1.2/bin/bundle:25:in `<main>'
C

Offenses:

example.rb:5:3: C: [Correctable] Style/EmptyElse: Redundant else-clause.
  else
  ^^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable

1 error occurred:
An error occurred while Lint/EmptyConditionalBody cop was inspecting /tmp/example/example.rb:4:2.
Errors are usually caused by RuboCop bugs.
Please, report your problems to RuboCop's issue tracker.
https://github.com/rubocop/rubocop/issues

Mention the following information in the issue report:
1.37.1 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 3.1.2) [x86_64-linux]
Finished in 0.10786570000345819 seconds

Steps to reproduce the problem

Create an empty project with the following Gemfile:

# frozen_string_literal: true

source 'https://rubygems.org'

gem 'rubocop', '1.37.1'

then run bundle install and create example.rb like this:

# frozen_string_literal: true

def foo
  if condition
  else
  end
end

then run bundle exec rubocop.

The another option is to add a test case to rubocop and run it like this:
(Note: -RUBY is used instead of ~RUBY for adding indentation)

diff --git a/spec/rubocop/cop/lint/empty_conditional_body_spec.rb b/spec/rubocop/cop/lint/empty_conditional_body_spec.rb
index 3595ddef0..33d9a47d9 100644
--- a/spec/rubocop/cop/lint/empty_conditional_body_spec.rb
+++ b/spec/rubocop/cop/lint/empty_conditional_body_spec.rb
@@ -14,7 +14,7 @@ RSpec.describe RuboCop::Cop::Lint::EmptyConditionalBody, :config do
   end
 
   it 'registers an offense for missing `if` and `else` body' do
-    expect_offense(<<~RUBY)
+    expect_offense(<<-RUBY)
       if condition
       ^^^^^^^^^^^^ Avoid `if` branches without a body.
       else
$ bundle exec rspec spec/rubocop/cop/lint/empty_conditional_body_spec.rb 

Randomized with seed 21344
..................F...

Failures:

  1) RuboCop::Cop::Lint::EmptyConditionalBody registers an offense for missing `if` and `else` body
     Failure/Error: corrector.replace(node.loc.else, "#{node.inverse_keyword} #{node.condition.source}")
     
     Parser::ClobberingError:
       Parser::Source::TreeRewriter detected clobbering
     # ./lib/rubocop/cop/lint/empty_conditional_body.rb:110:in `correct_other_branches'
     # ./lib/rubocop/cop/lint/empty_conditional_body.rb:84:in `autocorrect'
     # ./lib/rubocop/cop/lint/empty_conditional_body.rb:75:in `block in on_if'
     # ./lib/rubocop/cop/base.rb:346:in `correct'
     # ./lib/rubocop/cop/base.rb:127:in `add_offense'
     # ./lib/rubocop/cop/lint/empty_conditional_body.rb:74:in `on_if'
     # ./lib/rubocop/cop/commissioner.rb:100:in `public_send'
     # ./lib/rubocop/cop/commissioner.rb:100:in `block (2 levels) in trigger_responding_cops'
     # ./lib/rubocop/cop/commissioner.rb:160:in `with_cop_error_handling'
     # ./lib/rubocop/cop/commissioner.rb:99:in `block in trigger_responding_cops'
     # ./lib/rubocop/cop/commissioner.rb:98:in `each'
     # ./lib/rubocop/cop/commissioner.rb:98:in `trigger_responding_cops'
     # ./lib/rubocop/cop/commissioner.rb:69:in `on_if'
     # /home/r7kamura/ghq/github.com/rubocop/rubocop-ast/lib/rubocop/ast/traversal.rb:20:in `walk'
     # ./lib/rubocop/cop/commissioner.rb:86:in `investigate'
     # ./lib/rubocop/cop/team.rb:154:in `investigate_partial'
     # ./lib/rubocop/cop/team.rb:76:in `investigate'
     # ./lib/rubocop/rspec/cop_helper.rb:70:in `_investigate'
     # ./lib/rubocop/rspec/expect_offense.rb:120:in `expect_offense'
     # ./spec/rubocop/cop/lint/empty_conditional_body_spec.rb:17:in `block (2 levels) in <top (required)>'

Finished in 0.08826 seconds (files took 0.59821 seconds to load)
22 examples, 1 failure

Failed examples:

rspec ./spec/rubocop/cop/lint/empty_conditional_body_spec.rb:16 # RuboCop::Cop::Lint::EmptyConditionalBody registers an offense for missing `if` and `else` body

Randomized with seed 21344

RuboCop version

$ bundle exec rubocop -V
1.37.1 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 3.1.2) [x86_64-linux]
@r7kamura
Copy link
Contributor Author

r7kamura commented Oct 28, 2022

@ydah Do you know of any good solutions for this? I ask you because previously you had made changes about this Cop at #11017 and I thought maybe you would be familiar with it.

I think it's probably caused by the overlapping range of #remove_empty_branches and #correct_other_branches. To be more specific, I am guessing that there is something wrong with the way the range is calculated when the if statement is indented.

@ydah
Copy link
Member

ydah commented Oct 29, 2022

@r7kamura I see that you have already sent a pull request, but I think your guess is correct. will the fixes be similar to #11003 .

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