Keep necessary parentheses on the LHS of a hung binary operation#1136
Open
sarathfrancis90 wants to merge 1 commit into
Open
Keep necessary parentheses on the LHS of a hung binary operation#1136sarathfrancis90 wants to merge 1 commit into
sarathfrancis90 wants to merge 1 commit into
Conversation
check_excess_parentheses already knows that parentheses on the LHS of a binop must be kept for `(-X) ^ Y` and `(expr :: T) < Y`, but the hanging code paths never told it they were formatting an LHS: they passed down either the context the whole expression was hung in, or UnaryOrBinary. As soon as the expression was multilined the parentheses were dropped again, changing the meaning of `(-X) ^ Y` and producing a syntax error for the Luau type assertion. Derive the LHS context from the binop at every LHS call site instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two parentheses bugs that were fixed once already come back as soon as the expression is long enough to be hung over multiple lines:
and, with
syntax = "Luau":These are the same cases as #623 and #940.
check_excess_parentheseshandles both correctly, but only if it is told the expression is the LHS of a binop — and the hanging paths never do.hang_binop_expressionforwards whatever context the expression as a whole was hung in, andformat_hanging_expression_passesUnaryOrBinary, so theBinaryLHS/BinaryLHSExponentguards never fire once hanging kicks in.I pulled the context choice into a small helper and used it at each LHS call site. The direction is one-way — these contexts only ever keep more parentheses, never remove them — so the blast radius is small: across the whole test corpus one existing snapshot changed,
multiline-expression-comments-1, where(not strictFiltering)now keeps its parentheses like the non-hung(not true) and falsecase above it already does.Found this while checking that formatting is a fixed point (
format(format(x)) == format(x)) over the test inputs with various configs; the Luau one showed up as StyLua failing to re-read its own output.Tested with
cargo teston each feature in the CI matrix, plus clippy and rustfmt.