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/HeredocArgumentClosingParenthesis false positive when chaining methods that pass heredocs #9161

Closed
pdobb opened this issue Dec 3, 2020 · 1 comment · Fixed by #9165
Labels

Comments

@pdobb
Copy link
Contributor

pdobb commented Dec 3, 2020

When an outer method contains a chained method call with heredocs present, the Layout/HeredocArgumentClosingParenthesis cop appears to produce a false positive offense.


Expected behavior

No offenses should be found.

Actual behavior

# Baseline case (works fine)
my_outer_method(
  MyObject
    .select(Arel.sql("..."))
    .where(Arel.sql("...")))

# Raises an offense when Strings are changed to heredocs
my_outer_method(
  MyObject
    .select(Arel.sql(<<~SQL.squish))
      ...
    SQL
    .where(Arel.sql(<<~SQL.squish)))
      ...
    SQL

###

temp.rb:15:36: C: Layout/HeredocArgumentClosingParenthesis: Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening. (https://rubystyle.guide#heredoc-argument-closing-parentheses)
    .where(Arel.sql(<<~SQL.squish)))
                                   ^

Also:

# Also raises the same offense:
my_outer_method(-> {
  my_inner_method(<<~SQL.squish)
    ...
  SQL
})

####

rcs/temp.rb:7:2: C: Layout/HeredocArgumentClosingParenthesis: Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening. (https://rubystyle.guide#heredoc-argument-closing-parentheses)
})
 ^

Steps to reproduce the problem

There is no special configuration for this cop. So just run

rubocop --only Layout/HeredocArgumentClosingParenthesis temp.rb

on

my_outer_method(
  MyObject
    .select(Arel.sql(<<~SQL.squish))
      ...
    SQL
    .where(Arel.sql(<<~SQL.squish)))
      ...
    SQL

NOTE: In my testing, the code must contain an outer method as well (which adds in an additional closing paren) to get the offense. E.g. the following does not produce any offenses:

# Works fine due to no outer method (I presume).
MyObject
  .select(Arel.sql(<<~SQL.squish))
    ...
  SQL
  .where(Arel.sql(<<~SQL.squish))
    ...
  SQL

UPDATE: This fails as well:

my_outer_method(-> {
  my_inner_method(<<~SQL.squish)
    ...
  SQL
})

RuboCop version

I've tested this on both 1.4.2 and 1.5.1. Both exhibit the above issue.

$ rubocop -V
1.4.2 (using Parser 2.7.2.0, rubocop-ast 1.2.0, running on ruby 2.6.6 x86_64-darwin19)
  - rubocop-performance 1.9.0
  - rubocop-rails 2.8.1
  - rubocop-rake 0.5.1

# AND

$ rubocop -V
1.5.1 (using Parser 2.7.2.0, rubocop-ast 1.3.0, running on ruby 2.6.6 x86_64-darwin19)
  - rubocop-performance 1.9.0
  - rubocop-rails 2.8.1
  - rubocop-rake 0.5.1
@koic koic added the bug label Dec 3, 2020
@pdobb
Copy link
Contributor Author

pdobb commented Dec 3, 2020

Note, I edited and added a 2nd false positive example to the description, but adding here as well to be extra clear:

my_outer_method(-> {
  my_inner_method(<<~SQL.squish)
    ...
  SQL
})
$ rubocop --only Layout/HeredocArgumentClosingParenthesis temp.rb

temp.rb:7:2: C: Layout/HeredocArgumentClosingParenthesis: Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening. (https://rubystyle.guide#heredoc-argument-closing-parentheses)
})
 ^

koic added a commit to koic/rubocop that referenced this issue Dec 4, 2020
…osingParenthesis`

Fixes rubocop#9161.

This PR fixes a false positive for `Layout/HeredocArgumentClosingParenthesis`
when using subsequence closing parentheses in the same line.
bbatsov pushed a commit that referenced this issue Dec 4, 2020
…renthesis`

Fixes #9161.

This PR fixes a false positive for `Layout/HeredocArgumentClosingParenthesis`
when using subsequence closing parentheses in the same line.
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.

2 participants