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

Don't remove excess parentheses which are highlighting precedence #610

Merged
merged 6 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fix incorrect indentation level used for hanging expressions in if expression syntax ([#596](https://github.com/JohnnyMorganz/StyLua/issues/596))
- Fixed Luau return type in parentheses containing a comment on the last item being collapsed causing a syntax error ([#608](https://github.com/JohnnyMorganz/StyLua/issues/608))
- Fix parentheses removed which highlight precedence in `(not X) == Y` causing linting errors ([#609](https://github.com/JohnnyMorganz/StyLua/issues/609))

## [0.15.1] - 2022-09-22

Expand Down
36 changes: 26 additions & 10 deletions src/formatters/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ enum ExpressionContext {
/// as for cases like `(expr) :: any) :: type`
#[cfg(feature = "luau")]
TypeAssertion,

/// The internal expression is on the RHS of a binary operation
/// e.g. `(not X) and Y` or `(not X) == Y`, where internal_expression = `not X`
/// We should keep parentheses in this case to highlight precedence
BinaryLHS,

/// The internal expression is having a unary operation applied to it: the `expr` part of #expr.
/// If this occurs, and `expr` is a type assertion, then we need to keep the parentheses
UnaryOrBinary,
Expand Down Expand Up @@ -113,7 +119,17 @@ fn check_excess_parentheses(internal_expression: &Expression, context: Expressio
// Parentheses inside parentheses, not necessary
Expression::Parentheses { .. } => true,
// Check whether the expression relating to the UnOp is safe
Expression::UnaryOperator { expression, .. } => {
Expression::UnaryOperator {
expression, unop, ..
} => {
// If the expression is of the format `(not X) and Y` or `(not X) == Y` etc.
// Where internal_expression = not X, we should keep the parentheses
if let ExpressionContext::BinaryLHS = context {
if let UnOp::Not(_) = unop {
return false;
}
}

check_excess_parentheses(expression, context)
}
// Don't bother removing them if there is a binop, as they may be needed. TODO: can we be more intelligent here?
Expand All @@ -127,7 +143,12 @@ fn check_excess_parentheses(internal_expression: &Expression, context: Expressio
// we should always keep parentheses
// [e.g. #(value :: Array<string>) or -(value :: number)]
#[cfg(feature = "luau")]
if type_assertion.is_some() && matches!(context, ExpressionContext::UnaryOrBinary) {
if type_assertion.is_some()
&& matches!(
context,
ExpressionContext::UnaryOrBinary | ExpressionContext::BinaryLHS
)
{
return false;
}

Expand Down Expand Up @@ -293,7 +314,7 @@ fn format_expression_internal(
}
}
Expression::BinaryOperator { lhs, binop, rhs } => {
let lhs = format_expression_internal(ctx, lhs, ExpressionContext::UnaryOrBinary, shape);
let lhs = format_expression_internal(ctx, lhs, ExpressionContext::BinaryLHS, shape);
let binop = format_binop(ctx, binop, shape);
let shape = shape.take_last_line(&lhs) + binop.to_string().len();
Expression::BinaryOperator {
Expand Down Expand Up @@ -1123,7 +1144,7 @@ fn hang_binop_expression(
format_expression_internal(
ctx,
&lhs,
ExpressionContext::UnaryOrBinary,
ExpressionContext::BinaryLHS,
lhs_shape,
)
},
Expand All @@ -1147,12 +1168,7 @@ fn hang_binop_expression(
let lhs = if contains_comments(&*lhs) {
hang_binop_expression(ctx, *lhs, binop.to_owned(), shape, lhs_range)
} else {
format_expression_internal(
ctx,
&lhs,
ExpressionContext::UnaryOrBinary,
shape,
)
format_expression_internal(ctx, &lhs, ExpressionContext::BinaryLHS, shape)
};

let rhs = if contains_comments(&*rhs) {
Expand Down
4 changes: 4 additions & 0 deletions tests/inputs/excess-parentheses-dont-remove.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- https://github.com/JohnnyMorganz/StyLua/issues/609
-- Indicate precedence
local _ = (not true) == true
local _ = (not true) and false
6 changes: 3 additions & 3 deletions tests/inputs/excess-parentheses.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ local x = (1 + 2) * 3
local y = ((1) * 3)
local z = (...) == nil and foo or bar
local foo = not (bar and baz)
local bar = (not bar) and baz
local bar = (#bar) and baz
local cond = condition and (not object or object.Value == y)
local baz = (-4 + 3) * 2

Expand All @@ -15,7 +15,7 @@ local baz = (-4 + 3) * 2
function x()
return 1, 2
end

print(x())
print((x()))
print(((x())))
Expand All @@ -29,4 +29,4 @@ local x = ({})
local y = ("hello")
local z = (function()
return true
end)
end)
9 changes: 9 additions & 0 deletions tests/snapshots/[email protected]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: tests/tests.rs
expression: format(&contents)
---
-- https://github.com/JohnnyMorganz/StyLua/issues/609
-- Indicate precedence
local _ = (not true) == true
local _ = (not true) and false

2 changes: 1 addition & 1 deletion tests/snapshots/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ local x = (1 + 2) * 3
local y = (1 * 3)
local z = (...) == nil and foo or bar
local foo = not (bar and baz)
local bar = not bar and baz
local bar = #bar and baz
local cond = condition and (not object or object.Value == y)
local baz = (-4 + 3) * 2;

Expand Down