From 58db9ef32216fa09bd75a0b29429a0060edfe83d Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:31:52 +0000 Subject: [PATCH] refactor(codegen): do not print unnecessary parentheses if both sides use the same logical operator (#7325) As shown by the changing tests, we don't need to print parentheses for them. ### Comparison In [esbuild](https://esbuild.github.io/try/#dAAwLjI0LjAAAGEgPz8gKGIgPz8gKGMgPz8gZCkpOwooYSA/PyAoYiA/PyAoYyA/PyBkKSkpOwooYSB8fCAoYiB8fCBjKSkgfHwgZDsKYSB8fCAoYiB8fCAoYyB8fCBkKSk7CmEgJiYgKChiICYmIGMpICYmIGQp), it will print parentheses as-is, in [SWC](https://play.swc.rs/?version=1.9.2&code=H4sIAAAAAAAAA0tUsLdX0EgCk8kgMkVT05pLIxGLMES8pgYkDiSTNTVBVIo1F5IgUDFIDKQ2UUFNTUEDKAykkjVBZIomAGEbiHtuAAAA&config=H4sIAAAAAAAAA1VQzW7DIAy%2B9ykin6tlyrHXTb3ttCdA1GmpACPbSIuqvPuAJml6w9%2Bv8ePQdXAXC6fuUZ5lSIYFeZsLIlNU81cQ0CmhWHZJ4biyKpVSztiQ%2BUmAGr6iVhPK8DkMsOJsoozEYd%2BQBb9xdBHPxF%2FeiJwd%2BossuVsVo7G68xXIhUSsv5TZYi27qSY59T1K%2BJBbn56W48vAOaoLTWuyUjDqLCz0%2FPYDTyRVNxovyw4QXHTjtF%2FdUiglIu%2FCKjXx6jf%2FYc1v6RDokhu5HL0etq5U0gLFu8BLuTZu6eDkZ7W3s8%2F%2FYy0r4MUBAAA%3D), we have the same output now --- crates/oxc_codegen/src/binary_expr_visitor.rs | 16 ++++++++++++++-- crates/oxc_codegen/tests/integration/unit.rs | 10 +++++----- tasks/minsize/minsize.snap | 8 ++++---- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/crates/oxc_codegen/src/binary_expr_visitor.rs b/crates/oxc_codegen/src/binary_expr_visitor.rs index ca4b22527a37a..f5417ad4883c3 100644 --- a/crates/oxc_codegen/src/binary_expr_visitor.rs +++ b/crates/oxc_codegen/src/binary_expr_visitor.rs @@ -46,6 +46,12 @@ pub enum BinaryishOperator { Logical(LogicalOperator), } +impl BinaryishOperator { + fn is_binary(self) -> bool { + matches!(self, Self::Binary(_)) + } +} + fn print_binary_operator(op: BinaryOperator, p: &mut Codegen) { let operator = op.as_str(); if op.is_keyword() { @@ -148,9 +154,15 @@ impl<'a> BinaryExpressionVisitor<'a> { pub fn check_and_prepare(&mut self, p: &mut Codegen) -> bool { let e = self.e; - self.operator = e.operator(); - self.wrap = self.precedence >= self.operator.precedence() + // We don't need to print parentheses if both sides use the same logical operator + // For example: `(a && b) && c` should be printed as `a && b && c` + // ^^ e.operator() ^^ self.operator + let precedence_check = self.precedence >= e.operator().precedence() + && (self.operator.is_binary() || self.precedence != self.operator.precedence()); + + self.operator = e.operator(); + self.wrap = precedence_check || (self.operator == BinaryishOperator::Binary(BinaryOperator::In) && self.ctx.intersects(Context::FORBID_IN)); diff --git a/crates/oxc_codegen/tests/integration/unit.rs b/crates/oxc_codegen/tests/integration/unit.rs index 2dc4bd1e7cd9d..cbd077a4e2549 100644 --- a/crates/oxc_codegen/tests/integration/unit.rs +++ b/crates/oxc_codegen/tests/integration/unit.rs @@ -174,8 +174,8 @@ fn conditional() { fn coalesce() { test_minify("a ?? b", "a??b;"); test_minify("a ?? b ?? c ?? d", "a??b??c??d;"); - test_minify("a ?? (b ?? (c ?? d))", "a??(b??(c??d));"); - test_minify("(a ?? (b ?? (c ?? d)))", "a??(b??(c??d));"); + test_minify("a ?? (b ?? (c ?? d))", "a??b??c??d;"); + test_minify("(a ?? (b ?? (c ?? d)))", "a??b??c??d;"); test_minify("a, b ?? c", "a,b??c;"); test_minify("(a, b) ?? c", "(a,b)??c;"); test_minify("a, b ?? c, d", "a,b??c,d;"); @@ -188,8 +188,8 @@ fn coalesce() { #[test] fn logical_or() { test_minify("a || b || c", "a||b||c;"); - test_minify("(a || (b || c)) || d", "a||(b||c)||d;"); - test_minify("a || (b || (c || d))", "a||(b||(c||d));"); + test_minify("(a || (b || c)) || d", "a||b||c||d;"); + test_minify("a || (b || (c || d))", "a||b||c||d;"); test_minify("a || b && c", "a||b&&c;"); test_minify("(a || b) && c", "(a||b)&&c;"); test_minify("a, b || c, d", "a,b||c,d;"); @@ -201,7 +201,7 @@ fn logical_or() { #[test] fn logical_and() { test_minify("a && b && c", "a&&b&&c;"); - test_minify("a && ((b && c) && d)", "a&&(b&&c&&d);"); + test_minify("a && ((b && c) && d)", "a&&b&&c&&d;"); test_minify("((a && b) && c) && d", "a&&b&&c&&d;"); test_minify("(a || b) && (c || d)", "(a||b)&&(c||d);"); test_minify("a, b && c, d", "a,b&&c,d;"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 375432cb7a48d..3b04256db6ca8 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -10,17 +10,17 @@ Original | Minified | esbuild | Gzip | esbuild 544.10 kB | 73.48 kB | 72.48 kB | 26.12 kB | 26.20 kB | lodash.js -555.77 kB | 276.49 kB | 270.13 kB | 91.15 kB | 90.80 kB | d3.js +555.77 kB | 276.48 kB | 270.13 kB | 91.15 kB | 90.80 kB | d3.js -1.01 MB | 467.60 kB | 458.89 kB | 126.74 kB | 126.71 kB | bundle.min.js +1.01 MB | 467.59 kB | 458.89 kB | 126.73 kB | 126.71 kB | bundle.min.js 1.25 MB | 662.86 kB | 646.76 kB | 164.00 kB | 163.73 kB | three.js -2.14 MB | 741.57 kB | 724.14 kB | 181.45 kB | 181.07 kB | victory.js +2.14 MB | 741.55 kB | 724.14 kB | 181.45 kB | 181.07 kB | victory.js 3.20 MB | 1.02 MB | 1.01 MB | 332.01 kB | 331.56 kB | echarts.js 6.69 MB | 2.39 MB | 2.31 MB | 496.10 kB | 488.28 kB | antd.js -10.95 MB | 3.56 MB | 3.49 MB | 911.23 kB | 915.50 kB | typescript.js +10.95 MB | 3.56 MB | 3.49 MB | 911.24 kB | 915.50 kB | typescript.js