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

Raise void value expression in begin #2808

Merged

Conversation

eileencodes
Copy link
Member

@eileencodes eileencodes commented May 13, 2024

In some cases Prism was either not raising an appropriate void value expression error, or raising that error when the syntax is considered
valid.

To fix this Prism needs to check whether we have other clauses on the
begin rather than just returning cast->statements.

  • If the cast->statements are null and the cast->ensure_clause is
    not null, set the code to cast->ensure_clause
  • else
    • If there is a cast->rescue_clause
      • Check if cast->statements are null and cast->rescue_clause->statements
        are null, and return NULL
      • Check if there is an else_clause, and set the node to
        cast->else_clause.
      • Otherwise return cast->statements as the node
    • return cast->statements as the node

See tests for test cases. Note I took these directly from CRuby so if
desired I can delete them since the test will now pass. This only fixes
one test in the test_parse file, taking failures from 14 to 13.

This fixes TestParse#test_void_value_in_rhs and is related to
issue #2791.

@eileencodes eileencodes force-pushed the raise-void-value-expression-with-begin-ensure branch 3 times, most recently from 23e3cd0 to 4e8db5f Compare May 15, 2024 17:33
@eileencodes eileencodes force-pushed the raise-void-value-expression-with-begin-ensure branch 2 times, most recently from 82dad5c to 7eb9913 Compare May 23, 2024 19:18
In some cases Prism was either not raising an appropriate `void value
expression` error, or raising that error when the syntax is considered
valid.

To fix this Prism needs to check whether we have other clauses on the
`begin` rather than just returning `cast->statements`.

* If the `cast->statements` are null and the `cast->ensure_clause` is
not null, set the code to `cast->ensure_clause`
* else
  * If there is a `cast->rescue_clause`
    * Check if `cast->statements` are null and `cast->rescue_clause->statements`
    are null, and return `NULL`
    * Check if there is an `else_clause`, and set the node to
      `cast->else_clause`.
    * Otherwise return `cast->statements` as the node
  * return `cast->statements` as the node

See tests for test cases. Note I took these directly from CRuby so if
desired I can delete them since the test will now pass. This only fixes
one test in the `test_parse` file, taking failures from 14 to 13.

This fixes `TestParse#test_void_value_in_rhs` and is related to
issue ruby#2791.
@eileencodes eileencodes force-pushed the raise-void-value-expression-with-begin-ensure branch from 7eb9913 to 398152b Compare May 23, 2024 19:18
@eileencodes eileencodes changed the title Raise void value expression in begin/ensure Raise void value expression in begin May 23, 2024
@kddnewton kddnewton merged commit 695de7d into ruby:main May 23, 2024
54 of 56 checks passed
@eileencodes eileencodes deleted the raise-void-value-expression-with-begin-ensure branch May 23, 2024 19:30
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 this pull request may close these issues.

2 participants