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

Autocorrect for Style/SafeNavigation or/with Style/Next breaks code #6738

Closed
kke opened this issue Feb 6, 2019 · 1 comment · Fixed by #7006
Closed

Autocorrect for Style/SafeNavigation or/with Style/Next breaks code #6738

kke opened this issue Feb 6, 2019 · 1 comment · Fixed by #7006
Labels

Comments

@kke
Copy link

kke commented Feb 6, 2019

Steps to reproduce the problem

Create a file containing:

until [stdout, stderr].all?(&:eof?)
  readable = IO.select([stdout, stderr])
  if readable
    readable.first.each do |stream|
      data = +''
      stream.read_nonblock(1024, data)
    end
  end
end

Run rubocop -a on it

Expected behavior

Expected the result to be error free

Actual behavior

Rubocop broke the file between cops:

Inspecting 1 file
W

Offenses:

rubocop.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
until [stdout, stderr].all?(&:eof?)
^
rubocop.rb:3:3: C: [Corrected] Style/Next: Use next to skip iteration.
  if readable
  ^^^^^^^^^^^
rubocop.rb:3:3: C: [Corrected] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.
  if readable ...
  ^^^^^^^^^^^
rubocop.rb:4:3: W: Lint/UselessAssignment: Useless assignment to variable - readable. Did you mean readablereadable?
  readable = IO.select([stdout, stderr])
  ^^^^^^^^

1 file inspected, 4 offenses detected, 3 offenses corrected

The autocorrected file now looks like:

# frozen_string_literal: true

until [stdout, stderr].all?(&:eof?)
  readable = IO.select([stdout, stderr])
  next unless readablereadable&.first&.each do |stream| # <--- duplicated variable name
    data = +''
    stream.read_nonblock(1024, data)
  end
end

RuboCop version

0.63.1 (using Parser 2.6.0.0, running on ruby 2.6.1 x86_64-darwin18)

@Drenmi Drenmi added the bug label Feb 6, 2019
@hoshinotsuyoshi
Copy link
Contributor

smaller example:

until x
  if foo
    foo.some_method do
      y
    end
  end
end

auto-corrects badly;

# frozen_string_literal: true

until x
  next unless foofoo&.some_method do
    y
  end
end

( in rubocop.yml, TargetRubyVersion: 2.3 needed(2.3 or higher version). )


when --only Style/SafeNavigation

until x
  foo&.some_method do
      y
    end
end

when --only Style/Next

until x
  next unless foo
  foo.some_method do
    y
  end
end

$ rubocop -V

0.68.0 (using Parser 2.6.3.0, running on ruby 2.5.1 x86_64-darwin17)

hoshinotsuyoshi added a commit to hoshinotsuyoshi/rubocop that referenced this issue Apr 30, 2019
…`Style/SafeNavigation`

Fixes rubocop#6738

Solves problem like below:

  ```ruby
  until x
    if foo
      foo.some_method do
        y
      end
    end
  end
  ```

  auto-corrects badly;

  ```ruby
  # frozen_string_literal: true

  until x
    next unless foofoo&.some_method do
      y
    end
  end
  ```
bbatsov pushed a commit that referenced this issue Apr 30, 2019
…SafeNavigation` (#7006)

Fixes #6738

Solves problem like below:

  ```ruby
  until x
    if foo
      foo.some_method do
        y
      end
    end
  end
  ```

  auto-corrects badly;

  ```ruby
  # frozen_string_literal: true

  until x
    next unless foofoo&.some_method do
      y
    end
  end
  ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants