seattlerb / debride

Analyze code for potentially uncalled / dead methods, now with auto-removal.
https://www.zenspider.com/projects/debride.html
720 stars 19 forks source link

Parse failure involving single-method private and do/end block #11

Closed dschweisguth closed 9 years ago

dschweisguth commented 9 years ago
$ ruby --version
ruby 2.1.6p336 (2015-04-13 revision 50298) [x86_64-darwin14.0]
$ gem list | grep debride
debride (1.3.0)
$ cat x.rb 
class X
  private def traverse(list)
    list.each do |item| end
  end
end
[05:12:01 ~/data/projects/IndieGoGo] debride x.rb
/Users/me/.rbenv/versions/2.1.6/lib/ruby/2.1.0/racc/parser.rb:529:in `on_error': x.rb:3 :: parse error on value ["do", 3] (kDO_BLOCK) (Racc::ParseError)
    from /Users/me/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/ruby_parser-3.6.6/lib/ruby_parser_extras.rb:1124:in `on_error'
# ...

Removing private, removing the contents of the method and replacing do |item| end with { |item| } all avoid the error.

Hopefully this is the same error as the one I really care about, which is the same method in the context of a larger class which I can't share. In that case the symptoms are the same, but the error is skipping lib/larger_class.rb: Odd number (2) list for Hash. s(:array, s(:call, nil, :url))

zenspider commented 9 years ago

I assume it works if you put private on its own line?

I doubt the hash thing is related, can you extract, abstract, and still repro?

On Apr 23, 2015, at 05:20, Dave Schweisguth notifications@github.com wrote:

$ ruby --version ruby 2.1.6p336 (2015-04-13 revision 50298) [x86_64-darwin14.0] $ gem list | grep debride debride (1.3.0) $ cat x.rb class X private def traverse(list) list.each do |item| end end end [05:12:01 ~/data/projects/IndieGoGo] debride x.rb /Users/me/.rbenv/versions/2.1.6/lib/ruby/2.1.0/racc/parser.rb:529:in on_error': x.rb:3 :: parse error on value ["do", 3] (kDO_BLOCK) (Racc::ParseError) from /Users/me/.rbenv/versions/2.1.6/lib/ruby/gems/2.1.0/gems/ruby_parser-3.6.6/lib/ruby_parser_extras.rb:1124:inon_error'

...

Removing private, removing the contents of the method and replacing do |item| end with { |item| } all avoid the error.

Hopefully this is the same error as the one I really care about, which is the same method in the context of a larger class which I can't share. In that case the symptoms are the same, but the error is skipping lib/larger_class.rb: Odd number (2) list for Hash. s(:array, s(:call, nil, :url))

— Reply to this email directly or view it on GitHub.

dschweisguth commented 9 years ago

Yes, private on its own line works too.

> cat x.rb 
class X
  def foo
    { key: :value }
  end

  private def traverse(list)
    list.each do |item| end
  end

end
> debride x.rb
  skipping x.rb: Odd number (2) list for Hash. s(:array, s(:call, nil, :key))
These methods MIGHT not be called:
# end of output

If I delete the method foo or only its contents, debride crashes as above.

dschweisguth commented 9 years ago

https://github.com/seattlerb/ruby_parser/issues/187

dschweisguth commented 9 years ago

This makes me wonder whether the bug is entirely in ruby_parser: debride behaves like RubyParser.new on this code (foo matters). RubyParser.for_current_ruby on this code isn't affected by foo.

zenspider commented 9 years ago

Iiinteresting... Are you at railsconf? If so, grab me and we'll pair. I'm extremely sleep-depped right now and could use a functioning pair-brain.

dschweisguth commented 9 years ago

I also have half a brain. In sessions 'til 11:30, then flexible 'til 4ish. @daveschweisguth works too.

zenspider commented 9 years ago

On Apr 23, 2015, at 09:13, Dave Schweisguth notifications@github.com wrote:

I also have half a brain. In sessions 'til 11:30, then flexible 'til 4ish. @daveschweisguth works too.

hopefully pairing is additive, and not multiplicative.

come find me.

dschweisguth commented 9 years ago

Looked around a bit; didn't see you. I'm on 4 with an Indiegogo shirt.

zenspider commented 9 years ago

On Apr 23, 2015, at 11:24, Dave Schweisguth notifications@github.com wrote:

Looked around a bit; didn't see you. I'm on 4 with an Indiegogo shirt.

I just pushed a fix

dschweisguth commented 9 years ago

Confirmed that installing ruby_parser from current source fixes this issue. Thanks!

zenspider commented 9 years ago

Closing. Release of ruby_parser coming soon(ish).