From a5778b336237dc9dda6a712026f1373ec87f89b9 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 27 Oct 2022 12:34:42 +0100 Subject: [PATCH 1/5] Add test case --- tests/inputs/excess-parentheses-dont-remove.lua | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 tests/inputs/excess-parentheses-dont-remove.lua diff --git a/tests/inputs/excess-parentheses-dont-remove.lua b/tests/inputs/excess-parentheses-dont-remove.lua new file mode 100644 index 00000000..dbc831f9 --- /dev/null +++ b/tests/inputs/excess-parentheses-dont-remove.lua @@ -0,0 +1,4 @@ +-- https://github.com/JohnnyMorganz/StyLua/issues/609 +-- Indicate precedence +local _ = (not true) == true +local _ = (not true) and false From b152b00357ba3dd34768a5b5c2f5f5d4943487c7 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 27 Oct 2022 12:34:55 +0100 Subject: [PATCH 2/5] Don't remove parentheses when highlighting precedence --- src/formatters/expression.rs | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/formatters/expression.rs b/src/formatters/expression.rs index d0711992..658012d0 100644 --- a/src/formatters/expression.rs +++ b/src/formatters/expression.rs @@ -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, @@ -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? @@ -127,7 +143,12 @@ fn check_excess_parentheses(internal_expression: &Expression, context: Expressio // we should always keep parentheses // [e.g. #(value :: Array) 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; } @@ -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 { @@ -1123,7 +1144,7 @@ fn hang_binop_expression( format_expression_internal( ctx, &lhs, - ExpressionContext::UnaryOrBinary, + ExpressionContext::BinaryLHS, lhs_shape, ) }, @@ -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) { From 1354302dd46715442d6f6c6b2defa2192bcfd851 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 27 Oct 2022 12:35:10 +0100 Subject: [PATCH 3/5] Update snapshots --- tests/inputs/excess-parentheses.lua | 5 ++--- ...sts__standard@excess-parentheses-dont-remove.lua.snap | 9 +++++++++ .../tests__standard@excess-parentheses.lua.snap | 1 - 3 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 tests/snapshots/tests__standard@excess-parentheses-dont-remove.lua.snap diff --git a/tests/inputs/excess-parentheses.lua b/tests/inputs/excess-parentheses.lua index f2e6278f..8cef48c5 100644 --- a/tests/inputs/excess-parentheses.lua +++ b/tests/inputs/excess-parentheses.lua @@ -5,7 +5,6 @@ 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 cond = condition and (not object or object.Value == y) local baz = (-4 + 3) * 2 @@ -15,7 +14,7 @@ local baz = (-4 + 3) * 2 function x() return 1, 2 end - + print(x()) print((x())) print(((x()))) @@ -29,4 +28,4 @@ local x = ({}) local y = ("hello") local z = (function() return true -end) \ No newline at end of file +end) diff --git a/tests/snapshots/tests__standard@excess-parentheses-dont-remove.lua.snap b/tests/snapshots/tests__standard@excess-parentheses-dont-remove.lua.snap new file mode 100644 index 00000000..cbb2f504 --- /dev/null +++ b/tests/snapshots/tests__standard@excess-parentheses-dont-remove.lua.snap @@ -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 + diff --git a/tests/snapshots/tests__standard@excess-parentheses.lua.snap b/tests/snapshots/tests__standard@excess-parentheses.lua.snap index 1455725d..f0d68f4e 100644 --- a/tests/snapshots/tests__standard@excess-parentheses.lua.snap +++ b/tests/snapshots/tests__standard@excess-parentheses.lua.snap @@ -9,7 +9,6 @@ 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 cond = condition and (not object or object.Value == y) local baz = (-4 + 3) * 2; From 72174dc35869e8dcf8fa89e88e60036de52915ec Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 27 Oct 2022 12:35:47 +0100 Subject: [PATCH 4/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6416480..35f38562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix incorrect indentation level used for hanging expressions in if expression syntax ([#596](https://github.com/JohnnyMorganz/StyLua/issues/596)) +- 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 From 6d393616ec743ecd5373f500a2368c29fa5e0bf8 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 27 Oct 2022 13:00:00 +0100 Subject: [PATCH 5/5] Update snapshots --- tests/inputs/excess-parentheses.lua | 1 + tests/snapshots/tests__standard@excess-parentheses.lua.snap | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/inputs/excess-parentheses.lua b/tests/inputs/excess-parentheses.lua index 8cef48c5..6027a33f 100644 --- a/tests/inputs/excess-parentheses.lua +++ b/tests/inputs/excess-parentheses.lua @@ -5,6 +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 = (#bar) and baz local cond = condition and (not object or object.Value == y) local baz = (-4 + 3) * 2 diff --git a/tests/snapshots/tests__standard@excess-parentheses.lua.snap b/tests/snapshots/tests__standard@excess-parentheses.lua.snap index f0d68f4e..cb6e4d75 100644 --- a/tests/snapshots/tests__standard@excess-parentheses.lua.snap +++ b/tests/snapshots/tests__standard@excess-parentheses.lua.snap @@ -9,6 +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 = #bar and baz local cond = condition and (not object or object.Value == y) local baz = (-4 + 3) * 2;