Improve Ripper compatibiliy layer and emit :on_embexpr_end for unmatched closing brace#4164
Closed
andrykonchin wants to merge 1 commit into
Closed
Conversation
…hed closing brace
a62dfdb to
4f6c3b4
Compare
Collaborator
|
I'm not 100% convinded we should do this. More generally, relying on the shape of syntax invalid tokens seems like a horrible idea and subject to change without notice. Haml transforms syntax invalid code into slightly different syntax invalid code and when this got reported in #4127 I suggested to simply stop doing that. Would haml/haml#1210 work for you (I suppose you may have already seen this)? |
Member
Author
|
Ah, I completely overlooked the mentioned PR haml/haml#1210. At first glance the changes look good enough. Thank you! Will restore the PR if haml/haml#1210 is rejected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
:on_embexpr_endfor the first unmatched}Rationale
Ripper contains a scanner quirk that tokenizes the first unmatched closing curly brace (
}) as:on_embexpr_endrather than:on_rbrace. Because Prism did not replicate this behavior, Haml attribute parsing (which relies on this quirk to balance braces) failed with anHaml::SyntaxError: Unbalanced bracketserror.Right now the Haml and Sinatra tests fail on TruffleRuby on CI. This patch fixes the specs (checked locally).
cc @eregon
Examples (before the fix)
Ripper:
Prism:
The Haml use case
When rendering a Haml tag with attributes (e.g.,
%h1{:class => :header} Hello World), Haml needs to parse and extract the attributes hash. It does this by temporarily replacing the leading{with a dummy method call prefixa(, transforming it intoa(:class => :header}.It then tokenizes this string using
Ripper.lexto balance the brackets and extract the attributes hash. Since the opening{was replaced with(, the closing}is unmatched.https://github.com/haml/haml/blob/08808371860b7289af6ef23f328ae42f81a19afc/lib/haml/parser.rb#L686-L692