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/DotPosition seems to get confused by heredocs #10110

Closed
ccutrer opened this issue Sep 22, 2021 · 4 comments · Fixed by #10113
Closed

Layout/DotPosition seems to get confused by heredocs #10110

ccutrer opened this issue Sep 22, 2021 · 4 comments · Fixed by #10113
Assignees
Labels

Comments

@ccutrer
Copy link
Contributor

ccutrer commented Sep 22, 2021

I have some chained method calls that the arguments are heredocs. Layout/DotPosition doesn't seem to know that it needs to move the dot.

Given:

my_method.
  something(<<-HERE).
something
HERE
  somethingelse

and running rubocop -a -- test.rb


Expected behavior

my_method
  .something(<<~HERE)
    something
  HERE
  .somethingelse

Actual behavior

my_method
  .something(<<~HERE).
    something
  HERE
  somethingelse

RuboCop version

1.21.0 (using Parser 3.0.2.0, rubocop-ast 1.11.0, running on ruby 2.7.2 x86_64-darwin20)
  - rubocop-rails 2.12.2
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.4.0
@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 22, 2021

Seems to also have a problem with comments:

original:

my_method.
  something.
  # comment
  somethingelse

expected:

my_method
  .something
  # comment
  .somethingelse

actual:

my_method
  .something.
  # comment
  somethingelse

ruby version:

ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20]

note that I did test actual behavior, and all of the above produce the same behavior. is it something with older rubies that leading dot position wouldn't work with comments between the method calls?

@koic koic added the bug label Sep 22, 2021
@dvandersluis
Copy link
Member

dvandersluis commented Sep 22, 2021

The cop actually purposely does not register an offense on dots when there is a blank line or comment line between methods in the chain in order to not break things (added in #2354):

# don't register an offense if there is a line comment between the
# dot and the selector otherwise, we might break the code while
# "correcting" it (even if there is just an extra blank line, treat
# it the same)
return true if line_between?(selector_line, dot_line)

This is causing the bug with heredocs as well (because the heredoc adds extra lines between the dot and the receiver); however, I think that's a bug that I'm looking at fixing now.

@dvandersluis dvandersluis self-assigned this Sep 22, 2021
dvandersluis added a commit to dvandersluis/rubocop that referenced this issue Sep 22, 2021
koic added a commit that referenced this issue Sep 23, 2021
[Fix #10110] Update `Layout/DotPosition` to be able to handle heredocs
@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 23, 2021

Comments between fluent method calls is allowed in Ruby 2.7: https://blog.saeloun.com/2020/01/20/ruby-2-7-allows-placing-of-comment-lines-between-fluent-dots.html. I'd think the cop's behavior should be dependent on the TargetRubyVersion parameter. Thanks for the quick response though!

@dvandersluis
Copy link
Member

Yes that is indeed true, and thanks for the reference because I was looking for it myself but misremembered that it was done in ruby 3.0 rather than 2.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants