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

Lint/UnneededSplatExpansion: wrong unnecessary splat expansion. #3662

Closed
victorfeijo opened this issue Oct 21, 2016 · 2 comments · Fixed by #3688
Closed

Lint/UnneededSplatExpansion: wrong unnecessary splat expansion. #3662

victorfeijo opened this issue Oct 21, 2016 · 2 comments · Fixed by #3688

Comments

@victorfeijo
Copy link

victorfeijo commented Oct 21, 2016


Expected behaviour

Hello! I'm having a issue with Rubocop warning this code:

[*'A'..'H', *%w(J K M N), *'P'..'Z', *'0'..'9'] Unnecessary splat expansion in *%w(J K M N)

Which gives me something like ['A', 'B', 'C', 'D' 'E' .. 'J', 'K', 'M', 'N', 'P' .. '0', .. '9'].

Actual behaviour

When i use rubocop -a filename , the cop just removes the * from *%w(J K M N) and the returned data appears to be something different like:

['A', 'B', 'C', ['J', K', 'M', 'N'], 'P' ... '0']

It creates a new array inside. I'm trying # disable instead but would be good if i could keep both running, rubocop and *%w(J K M N)

RuboCop version

$ rubocop -V
0.44.1
@victorfeijo victorfeijo changed the title Style/Warning: Unnecessary splat expansion. Lint/UnneededSplatExpansion: Unnecessary splat expansion. Oct 21, 2016
@victorfeijo victorfeijo changed the title Lint/UnneededSplatExpansion: Unnecessary splat expansion. Lint/UnneededSplatExpansion: wrong unnecessary splat expansion. Oct 21, 2016
@tejasbubane
Copy link
Contributor

I tried putting together a bugfix, but came across this:

[1] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.source
=> "[*[1, 2, 3, 4, 5]]"
[2] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node
=> s(:array,
  s(:splat,
    s(:array,
      s(:int, 1),
      s(:int, 2),
      s(:int, 3),
      s(:int, 4),
      s(:int, 5))))
[1] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.source
=> "*[1, 2, 3]"
[2] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node
=> s(:array,
  s(:splat,
    s(:array,
      s(:int, 1),
      s(:int, 2),
      s(:int, 3))))

This^ should be splat-array instead of array-splat-array. Is this some issue with the parser?

@rrosenblum
Copy link
Contributor

@tejasbubane, I am not seeing the same issue with the code parsing that you are with the given example. What are versions of parser and ruby that you are using? Mine are 0.44.1 (using Parser 2.3.1.4, running on ruby 2.3.1 x86_64-darwin14).

[9] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.source
=> "*[1, 2, 3]"
[10] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node
=> s(:splat,
  s(:array,
    s(:int, 1),
    s(:int, 2),
    s(:int, 3)))
[11] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent.source
=> "[*[1, 2, 3]]"
[12] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent
=> s(:array,
  s(:splat,
    s(:array,
      s(:int, 1),
      s(:int, 2),
      s(:int, 3))))

I was able to reproduce what you saw with the foo = *[1, 2, 3]. In this case, I assume that the array-splat-array has something to do with how Ruby will treat the array once it has been expanded.

[1] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.source
=> "*[1, 2, 3]"
[2] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node
=> s(:splat,
  s(:array,
    s(:int, 1),
    s(:int, 2),
    s(:int, 3)))
[3] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent.source
=> "*[1, 2, 3]"
[4] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent
=> s(:array,
  s(:splat,
    s(:array,
      s(:int, 1),
      s(:int, 2),
      s(:int, 3))))
[5] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent.parent.source
=> "foo = *[1, 2, 3]"
[6] pry(#<RuboCop::Cop::Lint::UnneededSplatExpansion>)> node.parent.parent
=> s(:lvasgn, :foo,
  s(:array,
    s(:splat,
      s(:array,
        s(:int, 1),
        s(:int, 2),
        s(:int, 3)))))

@victorfeijo, this doesn't look like it will be difficult to fix. We just need to change the correction if we are in an array.

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

Successfully merging a pull request may close this issue.

3 participants