Layout/LineEndStringConcatenationIndentation not following EnforcedStyle in hashes

[fuegas]

Given this source file test.rb:

{
  field: 'some_field',
  error: :invalid,
  reason: 'some_field has'\
    'an invalid value',
}

puts 'String on'\
  'multiple rows'

and .rubocop config:

Layout/LineEndStringConcatenationIndentation:
  EnforcedStyle: indented

running rubocop --only Layout/LineEndStringConcatenationIndentation -- test.rb gives the following output:

⋗ rubocop --only Layout/LineEndStringConcatenationIndentation -- test.rb
Inspecting 1 file
C

Offenses:

test.rb:5:5: C: [Correctable] Layout/LineEndStringConcatenationIndentation: Align parts of a string concatenated with backslash.
    'an invalid value',
    ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable

I would expect no offenses in that file.

When I change the source to

{
  field: 'some_field',
  error: :invalid,
  reason: 'some_field has'\
          'an invalid value',
}

puts 'String on'\
  'multiple rows'

no offense is shown.

As I've set EnforcedStyle: indented, I expected the first example to work.

RuboCop version

⋗ bundle exec rubocop -V
1.18.3 (using Parser 3.0.2.0, rubocop-ast 1.7.0, running on ruby 2.7.4 x86_64-darwin20)
  - rubocop-performance 1.11.4
  - rubocop-rails 2.11.3

[koic]

Unaligned strings in hash literal values is designed to always be aligned with EnforcedStyle: aligned style. I would like to hear @jonas054 opinion.
https://github.com/rubocop/rubocop/blob/v1.18.3/spec/rubocop/cop/layout/line_end_string_concatenation_indentation_spec.rb#L51-L72

[jonas054]

Yes, the cop behaves as documented. https://docs.rubocop.org/rubocop/1.18/cops_layout.html#layoutlineendstringconcatenationindentation

The exception for hash values is something I added because in my opinion indentation within a hash literal was just too ugly. This is of course very subjective, and other people way find it surprising. It's a small detail though, and perhaps not worth changing?

[fuegas]

In my opinion it makes the "Enforced" style not enforced as there is a (subjective) deviation from it. However, I do have to acknowledge that the deviation is written down in the documentation. It might be a bit clearer if the examples would reflect this deviation as well.

I've never written or altered a cop myself, so I can't judge how much work it would be to have an EnforcedHashStyle option (which defaults to the existing behaviour). But I do think that the term EnforcedStyle is inconsistent with the behaviour of the Cop.

[jonas054]

It might be a bit clearer if the examples would reflect this deviation as well.

I think they already do. Look:

# bad
[...]
my_hash = {
  first: 'a message' \
    'in two parts'
}

# good
[...]
my_hash = {
  first: 'a message' \
         'in two parts'
}

There might be a better way to express it. I'm open to suggestions.

I've never written or altered a cop myself, so I can't judge how much work it would be to have an EnforcedHashStyle option (which defaults to the existing behaviour).

To me, it seems like a clear case of "not worth the effort". There are cops with more than one style parameter today, but it's a bit complicated if you ask me.

[fuegas]

Sorry for my unclear answer @jonas054, I mean under the example of EnforcedStyle: indented, as that only shows the split string and not the exception that exists for hashes.

For example, this would clear it up right away I think.

# bad
result = 'x' \
         'y'

# good
result = 'x' \
  'y'

# indented has no effect on hashes
my_hash = {
  first: 'a message' \
         'in two parts'
}

But you could say we should carefully read the paragraph as well 😉

To me, it seems like a clear case of "not worth the effort". There are cops with more than one style parameter today, but it's a bit complicated if you ask me.

Noted, I'll see that I'm going to inform the team regarding the current use of the cop and the style in our hashes.

[jonas054]

Yes, I think the normal style for the cop documentation is to have very terse comments in the examples ("good" and "bad"), and to put any explanations in the text above them. I'll leave the cop as it is without changes for now, and close the issue.

[shanecav84]

The exception for hash values is something I added because in my opinion indentation within a hash literal was just too ugly. This is of course very subjective, and other people way find it surprising.

I started updating my codebase to conform to the 'indented' style and found this surprising. I think the EnforcedStyle should be applied universally. If not, then another enforced style should be introduced (maybe "indentedExceptHashValues").

[jonas054]

Looking at this again, I must admit that your expectations are correct, @fuegas and @shanecav84. It's much more reasonable for Layout/LineEndStringConcatenationIndentation to behave like Layout/MultilineOperationIndentation, and in the case of hash literals it doesn't.

With the configuration

AllCops:
  NewCops: enable

Layout/LineEndStringConcatenationIndentation:
  EnforcedStyle: indented

Layout/MultilineOperationIndentation:
  EnforcedStyle: indented

Style/LineEndConcatenation:
  Enabled: false

Style/StringConcatenation:
  Enabled: false

RuboCop corrects to

# frozen_string_literal: true

puts 'Lorem ipsum dolor sit amet, consectetur ' +
  'adipiscing elit. Aenean sollicitudin maximus ' +
  'lacus eu maximus.'

h = {
  abc: 'Lorem ipsum dolor sit amet, consectetur ' +
    'adipiscing elit. Aenean sollicitudin maximus ' +
    'lacus eu maximus.',
  defghi: 'Lorem ipsum dolor sit amet, consectetur ' +
    'adipiscing elit.'
}

p h

so with backslashes it should be like this:

# frozen_string_literal: true

puts 'Lorem ipsum dolor sit amet, consectetur ' \
  'adipiscing elit. Aenean sollicitudin maximus ' \
  'lacus eu maximus.'

h = {
  abc: 'Lorem ipsum dolor sit amet, consectetur ' \
    'adipiscing elit. Aenean sollicitudin maximus ' \
    'lacus eu maximus.',
  defghi: 'Lorem ipsum dolor sit amet, consectetur ' \
    'adipiscing elit.'
}

p h

Rather than adding another style, I will consider the current behavior a bug, and submit a fix for it.

[fuegas]

Thank you for revisiting this @jonas054 ! I'm looking forward to the fix being merged 👍

[shanecav84]

Thanks for considering our feedback @jonas054!