Layout/LineLength auto-correction produces invalid Ruby syntax in specific situations

[jimeh]

I have come across certain situations where the Layout/LineLength cop moves things down to a new line in such a way it produces invalid Ruby syntax. Specially for method calls which do not surround the arguments with parentheses.

---

Expected behavior

Reformat method calls arguments in a sensible way that does not yield a syntax error.

Actual behavior

Method arguments are all moved to a new line, without any indentation, and because the method call does not wrap its arguments in parentheses, it becomes invalid syntax.

$ bundle exec rubocop --cache false --debug -a file.rb
For /Users/jimeh/Temp/rubocop-line-lenth-bug: configuration from /Users/jimeh/Temp/rubocop-line-lenth-bug/.rubocop.yml
Default configuration from /Users/jimeh/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rubocop-1.13.0/config/default.yml
Inspecting 1 file
Scanning /Users/jimeh/Temp/rubocop-line-lenth-bug/file.rb
E

Offenses:

file.rb:5:121: C: [Corrected] Layout/LineLength: Line is too long. [127/120]
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
                                                                                                                        ^^^^^^^
file.rb:6:106: E: Lint/Syntax: unexpected token tCOMMA
(Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)
'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
                                                                                                         ^

1 file inspected, 2 offenses detected, 1 offense corrected
Finished in 0.19170700013637543 seconds

(the .rubocop.yml file referenced above is blank)

Steps to reproduce the problem

The key is to have a method call without parentheses, at least two arguments, and the , after the first argument is on column 120 or later (assuming default max line length of 120). If max line length is customized to something other than the default 120, the column position that triggers the error will be the set max value.

Create file.rb:

class Foo
  def foo
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
  end
end

Run rubocop against it:

$ bundle exec rubocop -a file.rb
Inspecting 1 file
E

Offenses:

file.rb:5:121: C: [Corrected] Layout/LineLength: Line is too long. [127/120]
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
                                                                                                                        ^^^^^^^
file.rb:6:106: E: Lint/Syntax: unexpected token tCOMMA
(Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)
'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
                                                                                                         ^

1 file inspected, 2 offenses detected, 1 offense corrected

file.rb now has invalid syntax:

class Foo
  def foo
    foobarbaz 
'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
  end
end

RuboCop version

I've tested this with latest stable release (1.13.0), and from latest commit in master (74377b0) at time of writing.

Stable version details

$ bundle exec rubocop -V
1.13.0 (using Parser 3.0.1.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-darwin20)

.rubocop.yml:

# empty to override ~/.rubocop.yml

Gemfile:

source 'https://rubygems.org/'

gem 'rubocop'

Gemfile.lock:

GEM
  remote: https://rubygems.org/
  specs:
    ast (2.4.2)
    parallel (1.20.1)
    parser (3.0.1.0)
      ast (~> 2.4.1)
    rainbow (3.0.0)
    regexp_parser (2.1.1)
    rexml (3.2.5)
    rubocop (1.13.0)
      parallel (~> 1.10)
      parser (>= 3.0.0.0)
      rainbow (>= 2.2.2, < 4.0)
      regexp_parser (>= 1.8, < 3.0)
      rexml
      rubocop-ast (>= 1.2.0, < 2.0)
      ruby-progressbar (~> 1.7)
      unicode-display_width (>= 1.4.0, < 3.0)
    rubocop-ast (1.4.1)
      parser (>= 2.7.1.5)
    ruby-progressbar (1.11.0)
    unicode-display_width (2.0.0)

PLATFORMS
  x86_64-darwin-20

DEPENDENCIES
  rubocop

BUNDLED WITH
   2.2.16

Latest commit details

$ bundle exec rubocop -V
1.13.0 (using Parser 3.0.1.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-darwin20)

.rubocop.yml:

# empty to override ~/.rubocop.yml

Gemfile:

source 'https://rubygems.org/'

gem 'rubocop', github: 'rubocop/rubocop'

Gemfile.lock:

GIT
  remote: https://github.com/rubocop/rubocop.git
  revision: 74377b0502a211036713afca3c69a1198b0a6dc7
  specs:
    rubocop (1.13.0)
      parallel (~> 1.10)
      parser (>= 3.0.0.0)
      rainbow (>= 2.2.2, < 4.0)
      regexp_parser (>= 1.8, < 3.0)
      rexml
      rubocop-ast (>= 1.2.0, < 2.0)
      ruby-progressbar (~> 1.7)
      unicode-display_width (>= 1.4.0, < 3.0)

GEM
  remote: https://rubygems.org/
  specs:
    ast (2.4.2)
    parallel (1.20.1)
    parser (3.0.1.0)
      ast (~> 2.4.1)
    rainbow (3.0.0)
    regexp_parser (2.1.1)
    rexml (3.2.5)
    rubocop-ast (1.4.1)
      parser (>= 2.7.1.5)
    ruby-progressbar (1.11.0)
    unicode-display_width (2.0.0)

PLATFORMS
  x86_64-darwin-20

DEPENDENCIES
  rubocop!

BUNDLED WITH
   2.2.16

Extra Examples

This seems to happen whenever the , between the first and second argument is in colum 120 or higher with default Layout/LineLengh max value of 120. if the max value is set to 80, the issue happens when the , is on column 80 or higher.

Example 1 (from above)

Yields invalid syntax when , between arguments is on column 120 or later, with no parentheses around arguments.

Before:

class Foo
  def foo
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
  end
end

After:

class Foo
  def foo
    foobarbaz 
'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
  end
end

Expected:

class Foo
  def foo
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA',
              fozbaz
  end
end

Example 2

Formats the code properly when the , between arguments falls on column 119 or less, with no parentheses around arguments.

Before:

class Foo
  def foo
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxI', fozbaz
  end
end

After:

class Foo
  def foo
    foobarbaz 'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxI',
              fozbaz
  end
end

Example 3

Formats the code properly when arguments are wrapped in parentheses

Before:

class Foo
  def foo
    foobarbaz('sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz)
  end
end

After:

class Foo
  def foo
    foobarbaz(
      'sllVjITDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhFjxxIA', fozbaz
    )
  end
end

Example 4

Does not quite format/fix line length complaints when arguments are wrapped in parentheses, combined the arguments do not fit on a single line, but separated onto their own lines there would be no lines too long.

In the example below, the fozbaz argument gets placed so it ends beyond the 120th column triggering a Layout/LineLength complaint. The argument can very reasonably be moved down to the next line as illustrated.

Before:

class Foo
  def foo
    foobarbaz('sllVjITasdDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhaksljdf;l', fozbaz)
  end
end

After:

class Foo
  def foo
    foobarbaz(
      'sllVjITasdDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhaksljdf;l', fozbaz
    )
  end
end

Expected:

class Foo
  def foo
    foobarbaz(
      'sllVjITasdDpqP4fZC4HGqliNc3ODWhFjIA3e7zvMVSYAtUsrXCJHyoesllPSVUjIU6MsiTDpqP4fZC4HjldfkGqliNc3ODjfdWhaksljdf;l',
      fozbaz
    )
  end
end

[gregnavis]

I'd like to report a similar problem with method calls and heredocs.

In my case, the following method definition:

  def test_unindexed_foreign_keys
    assert_equal(<<~OUTPUT, unindexed_foreign_keys({ "users" => ["profile_id", "account_id"], "account" => ["group_id"] }))
      account group_id
      users account_id profile_id
    OUTPUT
  end

gets transformed into

  def test_unindexed_foreign_keys
    assert_equal(<<~OUTPUT, 
unindexed_foreign_keys({ "users" => ["profile_id", "account_id"], "account" => ["group_id"] }))
      account group_id
      users account_id profile_id
    OUTPUT
  end

You can see the original method invoked assert_equal with two arguments, the first one defined as a heredoc. The transformed method is syntactically invalid as the second argument and the following closing parenthesis were moved into the heredoc.

[dvandersluis]

@gregnavis I extracted your bug to #9799.

[dvandersluis]

@jimeh I opened #9801 to fix the buggy behaviour leading to the syntax error (ie. example 1).

For your other examples, the cop should flag the lines but it does not currently know how to autocorrect them -- the autocorrection for Layout/LineLength is definitely not all encompasing.