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

[Bug Report] Syntax error: Invalid yield in a brand new Rails application #400

Closed
fxn opened this issue Apr 12, 2024 · 5 comments · Fixed by #404
Closed

[Bug Report] Syntax error: Invalid yield in a brand new Rails application #400

fxn opened this issue Apr 12, 2024 · 5 comments · Fixed by #404
Labels
bug Something isn't working

Comments

@fxn
Copy link

fxn commented Apr 12, 2024

Description

bin/packwerk check does not pass in a just generated Rails application, the command complains about the ERB templates.

To Reproduce

  1. Create a new Rails application.
  2. Add packwerk to its Gemfile
  3. Run bundle install
  4. Run bundle binstub packwerk
  5. Run bin/packwerk init
  6. Run bin/packwerk check

Expected Behaviour

bin/packwerk check passes.

Actual Behaviour

% bin/packwerk check
📦 Packwerk is inspecting 28 files
E.EE........................
📦 Finished in 1.4 seconds

app/views/layouts/application.html.erb
Syntax error: Invalid yield

app/views/layouts/mailer.html.erb
Syntax error: Invalid yield

app/views/layouts/mailer.text.erb
Syntax error: Invalid yield

3 offenses detected

No stale violations detected

Version Information

  • Packwerk: 3.2.0
  • Ruby: 3.3.0
  • Rails: 7.1
@fxn fxn added the bug Something isn't working label Apr 12, 2024
@tqhung04
Copy link

I got the same issue here. Could anyone fix that? ><

@irphilli
Copy link

This a due to a bug in prism: ruby/prism#2688

If you downgrade prism to 0.24.0, the error goes away.

@a-abdellatif98
Copy link

a-abdellatif98 commented Apr 29, 2024

This a due to a bug in prism: ruby/prism#2688

If you downgrade prism to 0.24.0, the error goes away.

@irphilli fixed the issue to me, Thanks,

@exterm
Copy link
Contributor

exterm commented Apr 30, 2024

Should be resolved in prism 0.27.0.

@rafaelfranca
Copy link
Member

Closing since it is fixed on prism.

@rafaelfranca rafaelfranca reopened this May 2, 2024
rafaelfranca added a commit that referenced this issue May 2, 2024
Top level yields are invalid Ruby on files, but are valid on `eval`.

They exist on ERB files, and since those files are evaluated no written
to disk they are fine.

We can tell Prism to parse the code using the eval context by passing
`scopes: [[]]` to `parse`, but since are are using the
`Prism::Translation::Parser` class we have no way to do that.

A [PR][1] was proposed to Prism to always consider the code as being
evaluated in the `Parser` translation.

For now, we can safely ignore that class of error by overriding the
`valid_error?` method on our own parser.

Fixes #400.

[1]: ruby/prism#2741
Earlopain added a commit to Earlopain/rubocop that referenced this issue Jun 10, 2024
Most of these got disabled in rubocop#12822

It is however rather important that these cases parse sinc
some extensions rely on parsing code line by line.
This is the case when extracting code from a templating language like erb or haml

Prism was changed to allow these cases in ruby/prism#2741

`packwerk`ran into issue: Shopify/packwerk#400
I myself observed the same in `rubocop-erb`
Earlopain added a commit to Earlopain/rubocop that referenced this issue Jun 10, 2024
Most of these got disabled in rubocop#12822

It is however rather important that these cases parse sinc
some extensions rely on parsing code line by line.
This is the case when extracting code from a templating language like erb or haml

Prism was changed to allow these cases in ruby/prism#2741

`packwerk`ran into issue: Shopify/packwerk#400
I myself observed the same in `rubocop-erb`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants