Skip to content

fix: error on statement after return#1191

Open
PennyJim wants to merge 2 commits into
EmmyLuaLs:mainfrom
PennyJim:return_error
Open

fix: error on statement after return#1191
PennyJim wants to merge 2 commits into
EmmyLuaLs:mainfrom
PennyJim:return_error

Conversation

@PennyJim

Copy link
Copy Markdown

While it would have been better to specify the expected keyword to ends that particular block like actual Lua does, I do not see that being possible without refactoring.

I barely know rust, let alone your project structure, so forgive any cardinal sins I may have committed. I navigated by looking at how other statements were parsed and copied what I understood. I also do not know any language besides English, so I did not feel comfortable adding in a translation I would not be able to verify the quality of.

fix #1163

While it would have been better to specify the expected keyword to ends that particular block like actual Lua does, I do not see that being possible in the given structure.

fix EmmyLuaLs#1163

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Results

Issues Found:

  1. Missing translations for new error message

    • File: crates/emmylua_parser/locales/app.yml
    • Problem: The new error key expected end of block after return only has an English translation. Chinese translations (zh_CN, zh_HK) are missing, which may cause display issues for Chinese-speaking users.
    • Recommendation: Add Chinese translations for consistency with other error messages in the file.
  2. Potential duplicate error reporting

    • File: crates/emmylua_parser/src/grammar/lua/stat.rs (line 765-770)
    • Problem: The new error check is placed after semicolon consumption but before the return statement is completed. If the parser already reports a similar error elsewhere (e.g., during expression parsing or block closure), this could result in duplicate error messages for the same issue.
    • Recommendation: Verify that no other error reporting mechanism covers this case to avoid duplicate errors.
  3. Error range may be misleading

    • File: crates/emmylua_parser/src/grammar/lua/stat.rs (line 768)
    • Problem: The error range uses return_start_range (the position of the return keyword), but the actual error is about code after the return statement. This may confuse developers trying to locate the problematic code.
    • Recommendation: Consider using the range of the first offending token after the return statement instead, or extend the range to cover the entire problematic area.
  4. Test coverage is insufficient

    • File: crates/emmylua_parser/src/grammar/doc/test.rs
    • Problem: Tests only cover two specific cases (return 1, 2 x = 1 and return; x = 1). Missing edge cases such as:
      • Multiple statements after return
      • Return with no expressions followed by statements
      • Nested blocks where return is inside a block
      • Return at the end of a block (should not produce error)
    • Recommendation: Add more comprehensive test cases to ensure the error is correctly triggered only when appropriate.
  5. Logic may incorrectly flag valid code

    • File: crates/emmylua_parser/src/grammar/lua/stat.rs (line 765)
    • Problem: The condition !block_follow(p) checks if the next token is not a block follower. However, in Lua, return can be followed by end (closing a block), else, elseif, until, or EOF. If block_follow() doesn't account for all these cases, valid code may be incorrectly flagged.
    • Recommendation: Review the block_follow() function to ensure it correctly identifies all valid tokens that can follow a return statement.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a syntax error check when statements are placed after a return statement in Lua code, along with corresponding unit tests and localization keys. The review feedback suggests improving the user experience by highlighting the first invalid token after the return statement instead of the return keyword itself, which also simplifies the code and removes an unnecessary semicolon. Additionally, the feedback recommends asserting exact error ranges in the unit tests and adding Chinese translations for the new localization key.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/emmylua_parser/src/grammar/lua/stat.rs
Comment thread crates/emmylua_parser/src/grammar/lua/stat.rs Outdated
Comment thread crates/emmylua_parser/src/grammar/doc/test.rs
Comment on lines +258 to +259
expected end of block after return:
en: expected end of block after return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To maintain complete localization support for Chinese users (which is a major user base for EmmyLua), let's add the zh_CN and zh_HK translations for the new error message.

expected end of block after return:
  en: expected end of block after return
  zh_CN: 期望在 'return' 后结束块
  zh_HK: 期望在 'return' 後結束塊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me realize I probably want to change it to expected end of block after return statement
Because 'return' feels misleading as if it's after the return keyword, when an expression or multiple can and will be there.

I need to go to bed, so I'll do that after someone chimes in with feedback on these translations.

I was putting the error on the `return` to match how it's on the `if` when missing the `end`. But the code review made me realize that where the `end` can be is arbitrary with an if statement, but not with a return statement.
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.

Uncaught syntax error involving return

1 participant