Conversation
6b83aaf to
4a0ac3e
Compare
… would fail on CI.
63421ca to
c90147d
Compare
lib/unparser/emitter/primitive.rb
Outdated
|
|
||
| # mutant:disable | ||
| def dispatch | ||
| if RUBY_VERSION < '3.2' && value[-1] == '=' |
There was a problem hiding this comment.
@prikha if you had mutation tested this on 3.1, it should have forced you into value[-1].eql?('=') which is better code as it uses the non coercing #== (even if coercion does not trigger here in the first place, its just less powerful primitive).
But in addition, I do not think we should pull a string out to than compare with a literal. We can AFAIK just directly uses regexps so value.match?(/=\z/). This creates no intermediary objects and avoids also to access out of bounds on empty symbols.
lib/unparser/emitter/primitive.rb
Outdated
|
|
||
| # mutant:disable | ||
| def dispatch | ||
| if RUBY_VERSION < '3.2' && value.match?(/=\z/) |
There was a problem hiding this comment.
@prikha I'm well aware I probably did this wrong within unparser at other places, but I think we should use: RUBY_VERSION < '3.2.' note the extra . If ruby ever where to get a minor version of like: 3.11 it would despite logically being bigger, be lexicographical sorted wrong.
There was a problem hiding this comment.
Next question, how do we know that only = suffix symbols are affected?
I wounder if we should instead do something "stupid" and on RUBY < '3.2.' do a round trip on #inspect and if that fails add the quotes, and raise if that also fails.
There was a problem hiding this comment.
Let's go with a single roundtrip. Second one seems to be an overkill to me.
e3ae40e to
d421720
Compare
d421720 to
03c5722
Compare
Original bug:
https://bugs.ruby-lang.org/issues/18905
Resolves: #359