ammar / regexp_parser

A regular expression parser library for Ruby
MIT License
144 stars 23 forks source link

Make traverse/each_expression to return an Enumerator if they're called without a block #62

Closed pocke closed 4 years ago

pocke commented 4 years ago

Problem

Currently, Subexpression#traverse and Subexpression#each_expression require a block. So It is difficult to combine other Enumerator methods, such as map, select and so on.

require 'regexp_parser'

root = Regexp::Parser.parse(/foo|bar/)

# To get all literal nodes

# It works, but not smart
result = []
root.each_expression do |expr|
  result << expr if expr.is_a?(Regexp::Expression::Literal)
end
p result

# I'd like to write the following, but it doesn't works well without this pull request.
result = root.each_expression.select do |expr, _idx|
  expr.is_a?(Regexp::Expression::Literal)
end
p result

And, in the real world, I'd like to use it for RuboCop. https://github.com/pocke/rubocop-regular_expression/blob/b3f243b01973705cf43e8a3594c0854729e1ab7d/lib/rubocop/cop/regular_expression/mixed_capture_types.rb#L31-L41

Solution

Make them accept calling without a block.

Note

I found a bug of MatchLength#each method. It wasn't aware of opts argument if the method is called without a block. So this pull request also fixes the problem.

jaynetics commented 4 years ago

@pocke Great, thank you for the contribution! I will release a new version with your fix later today.

I also think the cop is a good idea.

Perhaps mixing both types should even cause a warning in Onigmo because the results are really counter-intuitive:

/(a)(?<name>b)/ =~ 'ab'; $1 # => 'b'
pocke commented 4 years ago

Thanks for merging and the advice!

Perhaps mixing both types should even cause a warning in Onigmo because the results are really counter-intuitive:

Good idea! I'll open an issue to Onigmo.

jaynetics commented 4 years ago

@pocke v1.7.0 is now released with your fix.