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

Metrics/LineLength auto-correct breaks line in the wrong place #7477

Closed
jonekdahl opened this issue Nov 2, 2019 · 2 comments · Fixed by #7497
Closed

Metrics/LineLength auto-correct breaks line in the wrong place #7477

jonekdahl opened this issue Nov 2, 2019 · 2 comments · Fixed by #7497

Comments

@jonekdahl
Copy link

Hi,

I have enabled auto-correction of Metrics/LineLength that was introduced in 0.68. I have also enabled the other recommended cops that were not enabled by default.

Expected behavior

When I run auto-correct, the code should still be valid Ruby code.

Actual behavior

In one instance, a line break was injected in the middle of an integer literal.

I have re-created the issue in a smaller example and this is the diff:

diff --git a/rubocop_line_length.rb b/rubocop_line_length.rb
index c505caa..01762f0 100644
--- a/rubocop_line_length.rb
+++ b/rubocop_line_length.rb
@@ -1,7 +1,9 @@
 class RubocopLineLength
   def breaks_this_incorrectly
     inventory_rows_to_fix = [
-      InventoryRow.new(year: 2019, week: 30, path: 'publisher_group:123;publisher:456;slot_type:midroll', to_book: 21906),
+      InventoryRow.new(year: 2019, week: 3
+0, path: 'publisher_group:123;
+publisher:456;slot_type:midroll', to_book: 21906),
       InventoryRow.new(year: 2019, week: 31, path: 'publisher_group:123;slot_type:preroll', to_book: 1558)
     ]
   end

Steps to reproduce the problem

Create .rubocop.yml:

Metrics/LineLength:
  AutoCorrect: true

Layout/MultilineArrayLineBreaks:
  Enabled: true

Layout/MultilineHashKeyLineBreaks:
  Enabled: true

Layout/MultilineMethodArgumentLineBreaks:
  Enabled: true

Create rubocop_line_length.rb:

class RubocopLineLength
  def breaks_this_incorrectly
    inventory_rows_to_fix = [
      InventoryRow.new(year: 2019, week: 30, path: 'publisher_group:123;publisher:456;slot_type:midroll', to_book: 21906),
      InventoryRow.new(year: 2019, week: 31, path: 'publisher_group:123;slot_type:preroll', to_book: 1558)
    ]
  end
end

Run rubocop --only Layout,Metrics/LineLength --auto-correct rubocop_line_length.rb

RuboCop version

$ rubocop -V
0.76.0 (using Parser 2.6.5.0, running on ruby 2.6.5 x86_64-linux)
@buehmann
Copy link
Contributor

buehmann commented Nov 4, 2019

That's another off-by-one error in Metrics/LineLength (I recently fixed another one): master...buehmann:line-length/7477

The example reveals another bug. With the fix above, the cop autocorrects the lines to

      InventoryRow.new(year: 2019, week: 30, path: 'publisher_group:123;
publisher:456;slot_type:midroll', to_book: 21906),
      InventoryRow.new(year: 2019, week: 31, path: 'publisher_group:123;
slot_type:preroll', to_book: 1558)

i.e. it inserts linebreaks into string literal (after the semicolons).

@maxh
Copy link
Contributor

maxh commented Nov 11, 2019

inserts linebreaks into string literal

Here's a fix for that: #7497

I believe that fixes the off-by-one error as well, by removing the buggy code.

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.

3 participants