From d62defb42d4fb3aa23a784ac965feffeb88e3ff4 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:35:09 +0000 Subject: [PATCH] fix(codegen): do not print trailing commas for `ArrayExpression` (#5551) closes #5532 --- crates/oxc_codegen/src/gen.rs | 13 +++++--- .../tests/integration/pure_comments.rs | 6 ++-- .../tests/integration/snapshots/ts.snap | 2 +- crates/oxc_codegen/tests/integration/unit.rs | 6 ++++ tasks/coverage/codegen_runtime_test262.snap | 31 ++----------------- 5 files changed, 21 insertions(+), 37 deletions(-) diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs index 80ab2cc676e8a..8240b96880ef6 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -1442,7 +1442,7 @@ impl<'a> Gen for ArrayExpressionElement<'a> { self.to_expression().gen_expr(p, Precedence::Comma, Context::empty()); } Self::SpreadElement(elem) => elem.gen(p, ctx), - Self::Elision(_span) => {} + Self::Elision(_span) => p.print_comma(), } } } @@ -1459,9 +1459,14 @@ impl<'a> Gen for ArrayExpression<'a> { fn gen(&self, p: &mut Codegen, ctx: Context) { p.add_source_mapping(self.span.start); p.print_char(b'['); - p.print_list(&self.elements, ctx); - if self.trailing_comma.is_some() { - p.print_comma(); + for (index, item) in self.elements.iter().enumerate() { + item.gen(p, ctx); + if index != self.elements.len() - 1 { + if !matches!(item, ArrayExpressionElement::Elision(_)) { + p.print_comma(); + } + p.print_soft_space(); + } } p.print_char(b']'); p.add_source_mapping(self.span.end); diff --git a/crates/oxc_codegen/tests/integration/pure_comments.rs b/crates/oxc_codegen/tests/integration/pure_comments.rs index 8b1dea3ec4287..3c0af87954981 100644 --- a/crates/oxc_codegen/tests/integration/pure_comments.rs +++ b/crates/oxc_codegen/tests/integration/pure_comments.rs @@ -14,7 +14,7 @@ fn annotate_comment() { /* #__NO_SIDE_EFFECTS__ */ async function*() {}, /* #__NO_SIDE_EFFECTS__ */ async function* y() {}, ])", - r"x([/* #__NO_SIDE_EFFECTS__ */ function() {}, /* #__NO_SIDE_EFFECTS__ */ function y() {}, /* #__NO_SIDE_EFFECTS__ */ function* () {}, /* #__NO_SIDE_EFFECTS__ */ function* y() {}, /* #__NO_SIDE_EFFECTS__ */ async function() {}, /* #__NO_SIDE_EFFECTS__ */ async function y() {}, /* #__NO_SIDE_EFFECTS__ */ async function* () {}, /* #__NO_SIDE_EFFECTS__ */ async function* y() {},]); + r"x([/* #__NO_SIDE_EFFECTS__ */ function() {}, /* #__NO_SIDE_EFFECTS__ */ function y() {}, /* #__NO_SIDE_EFFECTS__ */ function* () {}, /* #__NO_SIDE_EFFECTS__ */ function* y() {}, /* #__NO_SIDE_EFFECTS__ */ async function() {}, /* #__NO_SIDE_EFFECTS__ */ async function y() {}, /* #__NO_SIDE_EFFECTS__ */ async function* () {}, /* #__NO_SIDE_EFFECTS__ */ async function* y() {}]); ", ); @@ -28,7 +28,7 @@ fn annotate_comment() { /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => (y), ])", - r"x([/* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ () => {}, /* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ async (y) => y, /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => y,]); + r"x([/* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ () => {}, /* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ async (y) => y, /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => y]); ", ); test( @@ -41,7 +41,7 @@ fn annotate_comment() { /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => (y), ])", - r"x([/* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ () => {}, /* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ async (y) => y, /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => y,]); + r"x([/* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ () => {}, /* #__NO_SIDE_EFFECTS__ */ (y) => y, /* #__NO_SIDE_EFFECTS__ */ async (y) => y, /* #__NO_SIDE_EFFECTS__ */ async () => {}, /* #__NO_SIDE_EFFECTS__ */ async (y) => y]); ", ); // diff --git a/crates/oxc_codegen/tests/integration/snapshots/ts.snap b/crates/oxc_codegen/tests/integration/snapshots/ts.snap index a5ecd7f2cb2e6..dd3646a463123 100644 --- a/crates/oxc_codegen/tests/integration/snapshots/ts.snap +++ b/crates/oxc_codegen/tests/integration/snapshots/ts.snap @@ -15,7 +15,7 @@ let x: string[] = ['abc', 'def', 'ghi']; let x: string[] = ['abc', 'def', 'ghi']; let x: Array = ['abc', 'def', 'ghi',]; -let x: Array = ['abc', 'def', 'ghi',]; +let x: Array = ['abc', 'def', 'ghi']; let x: [string, number] = ['abc', 123]; let x: [string, number] = ['abc', 123]; diff --git a/crates/oxc_codegen/tests/integration/unit.rs b/crates/oxc_codegen/tests/integration/unit.rs index b4066f662be55..b0955f2b4123f 100644 --- a/crates/oxc_codegen/tests/integration/unit.rs +++ b/crates/oxc_codegen/tests/integration/unit.rs @@ -92,6 +92,12 @@ fn regex() { #[test] fn comma() { + test("[1, 2, 3]", "[1, 2, 3];\n"); + test("[1, 2, 3,]", "[1, 2, 3];\n"); + test("[,]", "[,];\n"); + test("[,,]", "[, ,];\n"); + test("[,1]", "[, 1];\n"); + test_minify("1, 2, 3", "1,2,3;"); test_minify("1, a = b, 3", "1,a=b,3;"); test_minify("1, (2, 3), 4", "1,2,3,4;"); diff --git a/tasks/coverage/codegen_runtime_test262.snap b/tasks/coverage/codegen_runtime_test262.snap index 7a722f144e5d2..bb16c4dc2967a 100644 --- a/tasks/coverage/codegen_runtime_test262.snap +++ b/tasks/coverage/codegen_runtime_test262.snap @@ -2,31 +2,7 @@ commit: d62fa93c codegen_runtime_test262 Summary: AST Parsed : 18456/18456 (100.00%) -Positive Passed: 17978/18456 (97.41%) -tasks/coverage/test262/test/annexB/language/global-code/block-decl-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/if-decl-else-decl-a-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/if-decl-else-decl-b-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/if-decl-else-stmt-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/if-decl-no-else-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/if-stmt-else-decl-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/switch-case-global-init.js -runtime error: Test262Error: descriptor should be enumerable - -tasks/coverage/test262/test/annexB/language/global-code/switch-dflt-global-init.js -runtime error: Test262Error: descriptor should be enumerable - +Positive Passed: 17987/18456 (97.46%) tasks/coverage/test262/test/language/expressions/assignment/fn-name-lhs-cover.js runtime error: Test262Error: descriptor value should be ; object value should be @@ -1054,10 +1030,7 @@ tasks/coverage/test262/test/language/expressions/prefix-increment/operator-prefi runtime error: Test262Error: Expected a ReferenceError but got a TypeError tasks/coverage/test262/test/language/global-code/decl-func.js -runtime error: Test262Error: descriptor should be enumerable; descriptor should not be configurable - -tasks/coverage/test262/test/language/global-code/decl-var.js -runtime error: Test262Error: descriptor should be enumerable +runtime error: Test262Error: descriptor should not be configurable tasks/coverage/test262/test/language/literals/regexp/u-surrogate-pairs-atom-escape-decimal.js runtime error: Test262Error: Expected SameValue(«true», «false») to be true