-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add #start_with?
and #end_with?
mutations
#1201
Conversation
515e3de
to
0a5ade5
Compare
/\Astatic/
-> #start_with?
mutations#start_with?
and #end_with?
mutations
# | ||
# @return [Parser::AST::Node] | ||
def self.expand_regexp_ast(node) | ||
*body, _opts = node.children |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that destructuring instead of using #slice
avoids the mutation we chose to ignore before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a followup we could also promote interpolations.
a.match?(/\Afoo#{dynamic}/)
to:
a.end_with?(dynamic)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, something like that might be interesting but I am not sure I will go that deep right away. I have a couple of other small improvements for similar cases I am thinking of as well that are pending.
cfbe4f1
to
6533a8d
Compare
|
||
RSpec.describe Mutant::AST::Regexp, '.expand_regexp_ast' do | ||
it 'returns the expanded AST' do | ||
# /foo/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking: Instead of this comment: what about running the regexp parser / AST transform to get the parser_ast
? This way this test is more meaningful and would have canary properties for future changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that makes sense--I have updated this to match. When I was originally writing the spec it felt like it might be clearer to have the raw AST so I started with that but now that I have just swapped it out for Unparser.parse
I think it's a lot simpler to read and, as you say, it has a slight "canary" property.
dd5ab04
to
e0042ad
Compare
- Adds the following mutations: * `a.match(/\Atext/)` -> `b.start_with?('text')` * `a.match?(/\Atext/)` -> `b.start_with?('text')` * `a =~ /\Atext/` -> `b.start_with?('text')` - NOTE: Adds a `Mutant::AST::Regexp.expand_regexp_ast` to avoid repeating AST expansion logic in the `send` mutator. At that level, the node appears as a simple `parser` `s(:regexp ...)` node so I chose to fully parse the regexp rather than try to do a string pattern. - Closes #169
- Adds the following mutations: * `a.match(/text\z/)` -> `b.end_with?('text')` * `a.match?(/text\z/)` -> `b.end_with?('text')` * `a =~ /text\z/` -> `b.end_with?('text')`
c106234
to
635a529
Compare
a.match(/\Atext/)
->b.start_with?('text')
a.match?(/\Atext/)
->b.start_with?('text')
a =~ /\Atext/
->b.start_with?('text')
Mutant::AST::Regexp.expand_regexp_ast
to avoid repeating AST expansion logic in thesend
mutator. At that level, the node appears as a simpleparser
s(:regexp ...)
node so I chose to fully parse the regexp rather than try to do a string pattern.a.match(/text\z/)
->b.end_with?('text')
a.match?(/text\z/)
->b.end_with?('text')
a =~ /text\z/
->b.end_with?('text')