Skip to content
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

Rspec tests fail on views with erb comments with single quotes #429

Closed
djfpaagman opened this issue Mar 25, 2022 · 7 comments · Fixed by #430
Closed

Rspec tests fail on views with erb comments with single quotes #429

djfpaagman opened this issue Mar 25, 2022 · 7 comments · Fixed by #430
Assignees

Comments

@djfpaagman
Copy link
Contributor

I found that 1.0.x fails for me, and narrowed it down to erb comments that have single quotes in them, for example when used as a apostrophe in a sentence.

This is the simpelest test case I can make that reproduces the error:

<%# this will fail: ' %>
  1) I18n does not have missing keys
     Failure/Error: raise Parser::SyntaxError, diagnostic

     I18n::Tasks::CommandError:
       Error scanning app/views/test.html.erb: unterminated string meets end of file
     # (eval):3:in `_racc_do_parse_c'
     # (eval):3:in `do_parse'
     # ------------------
     # --- Caused by: ---
     # Parser::SyntaxError:
     #   unterminated string meets end of file
     #   (eval):3:in `_racc_do_parse_c'

  2) I18n does not have unused keys
     Failure/Error: raise Parser::SyntaxError, diagnostic

     I18n::Tasks::CommandError:
       Error scanning app/views/test.html.erb: unterminated string meets end of file
     # (eval):3:in `_racc_do_parse_c'
     # (eval):3:in `do_parse'
     # ------------------
     # --- Caused by: ---
     # Parser::SyntaxError:
     #   unterminated string meets end of file
     #   (eval):3:in `_racc_do_parse_c'

The rake tasks do work properly.

@djfpaagman djfpaagman changed the title Rspec tests fail on views with erb comments with single quote in it Rspec tests fail on views with erb comments with single quotes Mar 25, 2022
@davidwessman
Copy link
Collaborator

Might have time to look at it today.

@glebm would it make sense to rescue in the parser and skip that node if it fails? E.g. if there is "wrong syntax" it will miss translations on that row as well? Should we print an error message?

@glebm
Copy link
Owner

glebm commented Mar 25, 2022

@davidwessman But why is it parsing a comment as code?

@glebm
Copy link
Owner

glebm commented Mar 25, 2022

Putting a space between a % and # makes it work:

<% # this should not fail: ' %>

Looks like a bug in the parser somewhere.

Without a space we get this in handler missing for node.inspect:

s(:erb,
  s(:indicator, "#"), nil,
  s(:code, " this should not fail: ' "), nil)

With a space we get:

s(:erb, nil, nil,
  s(:code, " # this should not fail: ' "), nil)

@davidwessman
Copy link
Collaborator

Ah, that seems to be a limit in better-html :/ I guess we could handle that AST-node for now

glebm added a commit that referenced this issue Mar 25, 2022
The ERB parser we're using misparses comments of the form:

    <%# ... #>

(no space between % and #)

With a space the AST is:

    s(:erb, nil, nil,
      s(:code, " # this should not fail: ' "), nil)

Without a space the AST is:

    s(:erb,
      s(:indicator, "#"), nil,
      s(:code, " this should not fail: ' "), nil)

The latter AST causes a crash when parsing the `:code` node as Ruby.

Works around the issue by transforming the latter AST to the former.

Fixes #429
@glebm
Copy link
Owner

glebm commented Mar 25, 2022

@davidwessman I've given it a go at #430

glebm added a commit that referenced this issue Mar 25, 2022
The ERB parser we're using misparses comments of the form:

    <%# ... #>

(no space between % and #)

With a space the AST is:

    s(:erb, nil, nil,
      s(:code, " # this should not fail: ' "), nil)

Without a space the AST is:

    s(:erb,
      s(:indicator, "#"), nil,
      s(:code, " this should not fail: ' "), nil)

The latter AST causes a crash when parsing the `:code` node as Ruby.

Works around the issue by transforming the latter AST to the former.

Fixes #429
@glebm glebm self-assigned this Mar 25, 2022
glebm added a commit that referenced this issue Mar 25, 2022
The ERB parser we're using misparses comments of the form:

    <%# ... #>

(no space between % and #)

With a space the AST is:

    s(:erb, nil, nil,
      s(:code, " # this should not fail: ' "), nil)

Without a space the AST is:

    s(:erb,
      s(:indicator, "#"), nil,
      s(:code, " this should not fail: ' "), nil)

The latter AST causes a crash when parsing the `:code` node as Ruby.

Works around the issue by transforming the latter AST to the former.

Fixes #429
@glebm
Copy link
Owner

glebm commented Mar 25, 2022

Released v1.0.4

@djfpaagman
Copy link
Contributor Author

Thanks @davidwessman and @glebm for the super fast follow up 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants