turboladen / tailor

A RubyGem that allows for checking standard styling of Ruby files.
146 stars 18 forks source link

Ruby 2.3.0 uses Struct for Ripple parse elements #163

Open biinari opened 8 years ago

biinari commented 8 years ago

https://github.com/ruby/ruby/commit/9a28a29b870b5f45d370bc8f16c431b435f0bbb3 commited for Ruby feature 9098 changes the elements passed in parser events from an Array to Ripper::Lexer::Elem (a Struct).

It seems that Ripper::Lexer is documented as being for internal use only, so backwards compatibility is evidently not being worried about there.

$ tailor
Error while parsing file /home/bill/code/busser-serverspec/lib/busser/runner_plugin/serverspec.rb
undefined method `last' for #<Ripper::Lexer::Elem:0x00000002966368> (NoMethodError)
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexed_line.rb:161:in `block in last_non_line_feed_event'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexed_line.rb:158:in `each'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexed_line.rb:158:in `find_all'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexed_line.rb:158:in `last_non_line_feed_event'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/rulers/allow_trailing_line_spaces_ruler.rb:12:in `ignored_nl_update'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/rulers/allow_trailing_line_spaces_ruler.rb:19:in `nl_update'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/composite_observable.rb:36:in `block (2 levels) in define_observer'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/composite_observable.rb:35:in `each'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/composite_observable.rb:35:in `block in define_observer'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexer.rb:313:in `on_nl'
/usr/lib/ruby/2.3.0/ripper/lexer.rb:61:in `parse'
/usr/lib/ruby/2.3.0/ripper/lexer.rb:61:in `parse'
/usr/lib/ruby/2.3.0/ripper/lexer.rb:55:in `lex'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/lexer.rb:42:in `lex'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:72:in `check_file'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:33:in `block (2 levels) in critique'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:29:in `each'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:29:in `block in critique'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:26:in `each'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/critic.rb:26:in `critique'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/cli.rb:46:in `execute!'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/lib/tailor/cli.rb:22:in `run'
/home/bill/code/busser-serverspec/vendor/bundle/ruby/2.3.0/gems/tailor-1.4.1/bin/tailor:10:in `<top (required)>'
/home/bill/code/busser-serverspec/.bundle/bin/tailor:16:in `load'
/home/bill/code/busser-serverspec/.bundle/bin/tailor:16:in `<main>'

I'm thinking that the simplest solution could be to use [0] and [2] in place of .first and .last respectively.

turboladen commented 8 years ago

Ouch--I didn't realize Ripper::Lexer was intended for internal use only. When I did the 1.0.0 release I really wanted to rely on stdlib ruby for tailor (as opposed to relying on a gem), and ripper was 100% new to me, without much documentation to go by at the time. In any case... @biinari would you be interested in making a PR? Admittedly I haven't been using tailor much since my team has adopted rubocop in recent years. I'd be happy to make a release if you'd be open to making the changes.

biinari commented 8 years ago

It is odd that the part that provides event based parsing has been changed and it seems poorly documented. I don't use tailor either but came across this when looking at an issue with busser-serverspec. We also use rubocop for our own projects.

I'm not going to spend the time on this now but if there is anybody who does use tailor and wants it to be made compatible with Ruby 2.3, this should be a useful starting point.

turboladen commented 8 years ago

That sounds totally reasonable. Thanks a ton for taking the time to log this, @biinari !