Fix binary encoding for parser translator #3390
Conversation
|
It seems like this is causing a bunch of new files to fail. I'm not sure if I understand fully what's going on there. Why are we adding a bunch of skips? |
|
Ah, yes. That is from the first commit, they were previously rejected by the parser gem as invalid syntax. I made that change since I noticed that the RuboCop accepts such code via monkeypatch here: https://github.com/rubocop/rubocop-ast/blob/02b8d0faeeebc3ba6b5284c9e66569bbae836941/lib/rubocop/ast/builder.rb#L112-L114 Edit: Let me just fix that minimal build failure... Doesn't really matter which encoding it is |
Earlopain
force-pushed
the
parser-translator-binary-encoding
branch
from
4892b05 to
69cb8b9
Compare
|
@Earlopain if you don't mind just rebasing here I can merge this |
Skipping detecting the encoding is almost always right, just for binary it should actually happen. A symbol containing escapes that are invalid in utf-8 would fail to parse since symbols must be valid in the script encoding. Additionally, the parser gem would raise an exception somewhere during string handling
Earlopain
force-pushed
the
parser-translator-binary-encoding
branch
from
69cb8b9 to
fa0154d
Compare
|
Merged the two commits since all newly running tests from that monkeypatch pass now. |
2 participants
Skipping detecting the encoding is almost always right, just for binary it should actually happen.
A symbol containing escapes that are invalid in utf-8 would fail to parse since symbols must be valid in the script encoding.
Additionally, the parser gem would raise an exception somewhere during string handling.
The parser gem reject code where a string is invalid in the script encoding. RuboCop however monkey-patches that behaviour out, I have mirrored that logic for the tests here.