fix(codegen): do not print trailing commas

closes #5532
oxc-project · Sep 6, 2024 · abfe897 · abfe897
1 parent c8ab353
commit abfe897
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 37 deletions.
diff --git 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
@@ -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
@@ -15,7 +15,7 @@ let x: string[] = ['abc', 'def', 'ghi'];
 let x: string[] = ['abc', 'def', 'ghi'];
 
 let x: Array<string> = ['abc', 'def', 'ghi',];
-let x: Array<string> = ['abc', 'def', 'ghi',];
+let x: Array<string> = ['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
@@ -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
@@ -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