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

heredoc as hash value raises Style/RedundantParentheses #2874

Closed
SaltwaterC opened this issue Feb 22, 2016 · 8 comments
Closed

heredoc as hash value raises Style/RedundantParentheses #2874

SaltwaterC opened this issue Feb 22, 2016 · 8 comments

Comments

@SaltwaterC
Copy link

I'll let the command line do the talking:

$ rubocop -V # supplied by ChefDK 0.11.0
0.37.2 (using Parser 2.3.0.5, running on ruby 2.1.6 x86_64-darwin12.0)
$ ruby code.rb
{:foo=>"heredoc\nmore heredoc\n", :bar=>"baz"}
$ cat code.rb
foo = {
  foo: (
  <<-EOF.gsub(/^\s*\|/, '')
    |heredoc
    |more heredoc
  EOF
  ),
  bar: 'baz'
}

p foo
$ rubocop
Inspecting 1 file
C

Offenses:

code.rb:2:8: C: Don't use parentheses around a method call.
  foo: (
       ^

1 file inspected, 1 offense detected
$ rubocop -a
Inspecting 1 file
E

Offenses:

code.rb:2:8: C: [Corrected] Don't use parentheses around a method call.
  foo: (
       ^
code.rb:7:3: E: unexpected token tCOMMA
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)
  ,
  ^

1 file inspected, 2 offenses detected, 1 offense corrected
$ ruby code.rb
code.rb:7: syntax error, unexpected ',', expecting '}'
code.rb:9: syntax error, unexpected '}', expecting end-of-input
$ cat code.rb
foo = {
  foo:
  <<-EOF.gsub(/^\s*\|/, '')
    |heredoc
    |more heredoc
  EOF
  ,
  bar: 'baz'
}

p foo

Don't know whether this issue is related to #2654 and/or #2845. This is the closest I got before raising a new issue.

@alexdowad
Copy link
Contributor

This doesn't specifically have anything to do with heredocs.

$ cat test.rb
hash = {
  foo: 1
  ,
  bar: 2
}
$ ruby-parse test.rb
test4.rb:3:3: error: unexpected token tCOMMA
test4.rb:3:   ,
test4.rb:3:   ^

@alexdowad
Copy link
Contributor

From parser's Ruby grammar:

          assocs: assoc
                    {
                      result = [ val[0] ]
                    }
                | assocs tCOMMA assoc
                    {
                      result = val[0] << val[2]
                    }

@alexdowad
Copy link
Contributor

I consider this to be a bug in Ruby's parser. Wonder if they would accept a patch...

@alexdowad
Copy link
Contributor

I guess I can't say that it has nothing to do with heredocs: the issue is that a heredoc must always be terminated with a newline (you can't put anything else on the terminating line), so a bare heredoc can't be used anywhere where the parser can't swallow a newline. If you surround it with parens, it's OK.

@alexdowad
Copy link
Contributor

Maybe we shouldn't consider parens redundant if the right paren comes directly after the terminating line of a heredoc?

That would probably require us to walk the entire subtree looking for a heredoc, which is a bit of a pain.

@alexdowad
Copy link
Contributor

OR, rather than looking at the AST, we look at the tokens. Find the token for the rparen, see if it is preceded by tNL and tSTRING_END. If so, back up looking for the corresponding tSTRING_BEG and see if it begins a heredoc...

@SaltwaterC
Copy link
Author

Maybe we shouldn't consider parens redundant if the right paren comes directly after the terminating line of a heredoc?

^ this

Also, this issue appeared between:

$ rubocop -V
0.34.2 (using Parser 2.2.3.0, running on ruby 2.1.6 x86_64-linux)

and the specified version in OP.

@alexdowad
Copy link
Contributor

That's probably when the RedundantParentheses cop was added.

lumeet added a commit to lumeet/rubocop that referenced this issue Apr 24, 2016
`Style/RedundantParentheses` doesn't register an offense for
parentheses in array and hash literals when the removal would cause
invalid syntax. For example, the following is accepted:

    array = [(
             <<-EOF
               something
             EOF
             ),
             'something']
Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Oct 15, 2016
`Style/RedundantParentheses` doesn't register an offense for
parentheses in array and hash literals when the removal would cause
invalid syntax. For example, the following is accepted:

    array = [(
             <<-EOF
               something
             EOF
             ),
             'something']
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants