Auto-correction for Style/IdenticalConditionalBranches cop breaks behavior

[DmitryBarskov]

Description: suggested auto-correction for Style/IdenticalConditionalBranches offence breaks an if statement if it is the last statement in the method with no explicit return keyword.

---

Expected behavior

Rubocop does not suggest auto-correction for Style/IdenticalConditionalBranches rule.

Actual behavior

Rubocop suggests an auto-correction which breaks the last if statement in a method.

Running with options rubocop test.rb --debug --cache false

For $PWD: Default configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-1.32.0/config/default.yml
Inspecting 1 file
Scanning $PWD/test.rb
For $PWD: configuration from $PWD/.rubocop.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rails-2.13.2/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rails-2.13.2/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rspec-2.10.0/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-rspec-2.10.0/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-performance-1.13.2/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-performance-1.13.2/config/default.yml
configuration from $HOME/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rubocop-thread_safety-0.4.4/config/default.yml
configuration from #{path}
C

Offenses:

xpense-tracker-api/test.rb:12:7: C: [Correctable] Style/IdenticalConditionalBranches: Move statement_a out of the conditional.
      statement_a
      ^^^^^^^^^^^
xpense-tracker-api/test.rb:14:7: C: [Correctable] Style/IdenticalConditionalBranches: Move statement_a out of the conditional.
      statement_a
      ^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable
Finished in 0.399621999822557 seconds

Steps to reproduce the problem

Given a plain Ruby file test.rb:

# frozen_string_literal: true

# Example of false-positive offence of
# Style/IdenticalConditionalBranches cop
class Test
  def initialize(cond)
    @cond = cond
  end

  def test
    if @cond
      statement_a
    else
      statement_a
      statement_b
    end
  end

  def statement_a
    :a
  end

  def statement_b
    :b
  end
end

raise 'should return :a' unless Test.new(true).test == :a
raise 'should return :b' unless Test.new(false).test == :b

Run it: ruby test.rb. It exits with code 0.
Run rubocop on it: rubocop test.rb -A.
It changes the #test method to:

  def test
    statement_a
    if @cond
    else
      statement_b
    end
  end

Run the script again: ruby test.rb.

I still expect the script to exit with code 0.
Actual exit code of auto-corrected script is 1.

RuboCop version

Rubocop 1.32.0 (fresh version) with no extensions.

$ [bundle exec] rubocop -V
1.32.0 (using Parser 3.1.2.0, rubocop-ast 1.19.1, running on ruby 3.1.2 arm64-darwin21)

[dvandersluis]

You're seeing offenses after autocorrection because the cop is leaving the if branch empty, which is triggering Lint/EmptyConditionalBody.

However, the autocorrection is faulty too, because as you pointed out it changes the implicit return value:

def test
  statement_a
  if @cond
  else
    statement_b
  end
end

will never return the statement_a value, but rather nil when if @cond is true.

[Rylan12]

Now that autocorrection has been added, I'm running into an edge case. If an empty if statement also has an empty else¹, the autocorrection totally breaks:

# frozen_string_literal: true

if foo
else
end

Running rubocop test.rb --debug --cache false --autocorrect-all changes the file to the following:

# frozen_string_literal: true

end

It seems that the combination of the Lint/EmptyConditionalBody and Style/EmptyElse autocorrectors doesn't work as expected. I know that the new Lint/EmptyConditionalBody is marked as unsafe, but this is creating a syntax error rather than simply risking different behavior.

Happy to open a separate issue if that's better.

Footnotes

1. For context: this comes up when the if and else branches are completely identical since Style/IdenticalConditionalBranches extracts the common branch and leaves a totally empty if + else. This behavior was reported to me in this discussion. ↩

[fatkodima]

Can confirm, than in rubocop 1.33.0 there was an error as mentioned in the previous comment. Now everything is working.
@koic You can close this issue.

[DmitryBarskov]

Hey @fatkodima, @koic! The error described in original issue still occurs in 1.37.1 and in 1.41.1. Do you think it should be closed?

[fatkodima]

Ah, I see. I assumed it is working now (from the mentioned comment) and only the problem with correction exists.
I will look into this.

[koic]

@DmitryBarskov You're right. I've reopen this issue.