fix(formatter): preserve required statement semicolons

[sean0407]

Summary

format statement
  if preserve_statement_semicolon is enabled
    keep source semicolon
  else if next statement starts with `(`
    keep required source semicolon
  otherwise
    omit optional semicolon

Fixes #1301.

Evidence

• Before: With preserve_statement_semicolon = false, formatting local req = f() followed by ;(req).style = 1 removed the semicolon; the regression test failed because (req).style = 1 became a continuation call.
• After: The formatter retains the separator as local req = f();. The focused regression tests and all 493 emmylua_formatter tests passed. cargo fmt --all -- --check passed.

Merge Danger

Door: Two-way

Blast Radius: Formatter

[github-actions]

Code Review

Summary

The change adds logic to preserve a statement semicolon when the next statement begins with a parenthesized expression (e.g., (req).style = 1), avoiding Lua's ambiguous-call parsing pitfall. The intent is correct and the test covers the primary case.

Issues & Recommendations

1. descendants_with_tokens() includes the node itself — potential false positive

return next_node
    .descendants_with_tokens()
    .filter_map(|child| child.into_token())
    .find(|token| { ... })

descendants_with_tokens() yields the node itself first. If next_node is itself a token (unlikely here since it's a Syntax node), or if the first token is a keyword like local/return/if, the check works. But the logic is fragile: it finds the first non-whitespace token anywhere in the subtree, not necessarily the first token of the statement. For a statement like local x = (f)(), the first token is local, so it correctly returns false. However, consider a statement whose leading token is a comment token that isn't TkWhitespace/TkEndOfLine — comments are filtered out only in the sibling loop, not in the descendant scan. This could produce a false negative if a comment token appears first inside the node.

Recommendation: Use next_node.first_token() (or descendants_with_tokens().find_map(...) restricted to the first token) rather than scanning the entire subtree. This is both more correct and more efficient.

2. Comment handling inconsistency
In the sibling loop, LuaKind::Syntax(LuaSyntaxKind::Comment) is skipped, but in the descendant scan only whitespace/EOL tokens are filtered. If the first token of the next statement is a comment (e.g., a leading --[[...]] block comment attached to the node), the check will return false incorrectly. Align the filtering in both places.

3. is_some_and on Option after find — semantics

.find(|token| { ... })
.is_some_and(|token| token.kind() == LuaKind::Token(LuaTokenKind::TkLeftParen))

This is fine, but combined with issue #1, the "first non-whitespace token" is not guaranteed to be the statement's leading token. Consider using first_token() directly:

next_node
    .first_token()
    .is_some_and(|t| t.kind() == LuaKind::Token(LuaTokenKind::TkLeftParen))

This is clearer and avoids the subtree scan entirely.

4. Naming / scope
The function name next_statement_starts_with_parenthesized_expression is accurate, but the function is only used to decide semicolon insertion. Consider a doc comment explaining why (Lua ambiguity: f()\n(g)() parses as f()(g)()), which will help future maintainers.

5. Test coverage gaps

• No test for the case where the next statement starts with a comment before (.
• No test for preserve_statement_semicolon: true combined with a parenthesized next statement (should still emit semicolon — verify no double semicolon).
• No negative test confirming that a non-parenthesized next statement does not get a semicolon when preserve_statement_semicolon: false.
• No test for a parenthesized expression that is not the first token, e.g. local x = (f)() — should not trigger.

Recommendation: Add at least the negative test and the comment-prefixed case.

6. Edge case: next_sibling_or_token traversal
The loop only walks siblings. If the statement is the last child of a block and the next statement is in a parent's sibling (e.g., after end), the function returns false — which is correct since a new block boundary resets ambiguity. This is fine, but worth a comment.

Minor

• The match arm LuaKind::Syntax(_) returns early; the _ => return false arm is unreachable for LuaKind::Token variants not listed. Consider making the token arm explicit for clarity, or add a comment.

Verdict

The core logic is sound and addresses a real formatting bug. The main concerns are the subtree scan in descendants_with_tokens() (correctness + performance) and the inconsistent comment filtering. Recommend simplifying to first_token() and adding the missing negative tests before merging.