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

Layout/MultilineOperationIndentation is missing some cases #7592

Closed
pirj opened this issue Dec 24, 2019 · 6 comments
Closed

Layout/MultilineOperationIndentation is missing some cases #7592

pirj opened this issue Dec 24, 2019 · 6 comments
Labels
bug stale Issues that haven't been active in a while

Comments

@pirj
Copy link
Member

pirj commented Dec 24, 2019

Layout/MultilineOperationIndentation is missing cases when \ is used as string concatenation.

# 1.rb
puts 'a' +
'b'

puts 'a' \
'b'

puts "#{1}" \
'b'

puts 'a' \
"#{2}"

puts "#{1}" \
"#{2}"

Expected behavior

All the offences are detected.

Actual behavior

Only the first statement is detected as offensive.

1.rb:2:1: C: Layout/MultilineOperationIndentation: Align the operands of an expression spanning multiple lines.
'b'
^^^

Steps to reproduce the problem

rubocop --only Layout/MultilineOperationIndentation 1.rb

RuboCop version

$ [bundle exec] rubocop -V
0.78.0 (using Parser 2.6.5.0, running on ruby 2.5.5 x86_64-darwin18)
@bbatsov bbatsov added the bug label Jan 5, 2020
@stale
Copy link

stale bot commented Jul 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Jul 3, 2020
@marcandre
Copy link
Contributor

Is this really a bug though? / is not an operator, it just escape the newline. I'd say this falls outside of the cop's jurisdiction:

This cop checks the indentation of the right hand side operand in binary operations that span more than one line.

@stale stale bot removed the stale Issues that haven't been active in a while label Jul 3, 2020
@pirj
Copy link
Member Author

pirj commented Jul 3, 2020

It might not be its business to take care of \, but the indentation looks broken. This may be a more generic issue than just this cop, might equally affect several cops.

To be honest, I agree with you that \ still makes the same line, but it's on different rows.
To my understanding, the indentation should consider rows, not lines (I hope I'm not mixing those two terms up), e.g.

class A
  def m
    puts "a" \
"b"
  end
end

from my point of view should raise at least some offence. I might be just me, but aestetically this code is broken.

@marcandre
Copy link
Contributor

Oh, I agree it's not a great alignment. I just meant to ask: is it this cop's responsibility? I'm still pretty ignorant of all the different cops we have 😀

@pirj
Copy link
Member Author

pirj commented Jul 3, 2020

Got it.
Answering this question directly: yes, I believe it's this cop's responsibility to raise an offence for those code examples.
But since it might be a common problem in indentation cops not being aware of \.

Still, I think it's a very minor issue, I've only seen this problem once in the wild, fixed it by hand, and no cop complained about the correct indentation. No objections if this is closed. It's just my OCD that pushed me to file this ticket.

@stale
Copy link

stale bot commented Dec 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Dec 31, 2020
jonas054 added a commit to jonas054/rubocop that referenced this issue Jun 20, 2021
The cop Layout/MultilineOperationIndentation doesn't view the
backslash as an operator, which is correct. Therefore, a new cop
is needed to inspect character literals broken up into multiple
lines with backslashes at the ends of the lines.

The new cop deals exclusively with string literals, not any other
constructs where backslashes might be used to "glue" lines
together.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug stale Issues that haven't been active in a while
Projects
None yet
Development

No branches or pull requests

3 participants