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

* Add test case

* Don't remove parentheses when highlighting precedence

* Update snapshots

* Update changelog

* Update snapshots

CHANGELOG.md

@@ -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
 

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<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;
             }
 
@@ -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) {

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

tests/inputs/excess-parentheses.lua

@@ -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
 
@@ -15,7 +15,7 @@ local baz = (-4 + 3) * 2
 function x()
 	return 1, 2
 end
-  
+
 print(x())
 print((x()))
 print(((x())))
@@ -29,4 +29,4 @@ local x = ({})
 local y = ("hello")
 local z = (function()
 	return true
-end)
+end)

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
+

tests/snapshots/tests__standard@excess-parentheses.lua.snap

@@ -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;