Merge pull request #4952 from sjrd/fix-stat-starting-object-lit-3

sjrd · web-flow · commit 8dd02c784818 · 2024-03-08T10:58:55.000+01:00
Fix #4949: Always wrap object literals with `()`.
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala
@@ -491,15 +491,17 @@ object Printers {
           printSeparatorIfStat()
 
         case ObjectConstr(Nil) =>
-          if (isStat)
-            print("({});") // force expression position for the object literal
-          else
-            print("{}")
+          /* #4949 Always wrap object literals with () in case they end up at
+           * the start of an `ExpressionStatement`.
+           */
+          print("({})")
+          printSeparatorIfStat()
 
         case ObjectConstr(fields) =>
-          if (isStat)
-            print('(') // force expression position for the object literal
-          print('{')
+          /* #4949 Always wrap object literals with () in case they end up at
+           * the start of an `ExpressionStatement`.
+           */
+          print("({")
           indent()
           println()
           var rest = fields
@@ -517,9 +519,8 @@ object Printers {
           }
           undent()
           printIndent()
-          print('}')
-          if (isStat)
-            print(");")
+          print("})")
+          printSeparatorIfStat()
 
         // Literals
 
diff --git a/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala
@@ -70,8 +70,8 @@ class LibrarySizeTest {
     )
 
     testLinkedSizes(
-      expectedFastLinkSize = 150063,
-      expectedFullLinkSizeWithoutClosure = 92648,
+      expectedFastLinkSize = 150205,
+      expectedFullLinkSizeWithoutClosure = 92762,
       expectedFullLinkSizeWithClosure = 21325,
       classDefs,
       moduleInitializers = MainTestModuleInitializers
diff --git a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala
@@ -163,6 +163,34 @@ class PrintersTest {
     )
   }
 
+  @Test def printObjectLiteral(): Unit = {
+    assertPrintEquals("({});", ObjectConstr(Nil))
+
+    assertPrintEquals(
+      """
+        |({
+        |  "foo": 1
+        |});
+      """,
+      ObjectConstr(List(StringLiteral("foo") -> IntLiteral(1)))
+    )
+
+    assertPrintEquals(
+      """
+        |({
+        |  "foo": 1,
+        |  ["bar"]: 2,
+        |  baz: 3
+        |});
+      """,
+      ObjectConstr(List(
+        StringLiteral("foo") -> IntLiteral(1),
+        ComputedName(StringLiteral("bar")) -> IntLiteral(2),
+        Ident("baz") -> IntLiteral(3)
+      ))
+    )
+  }
+
   @Test def delayedIdentPrintVersusShow(): Unit = {
     locally {
       object resolver extends DelayedIdent.Resolver {
diff --git a/project/Build.scala b/project/Build.scala
@@ -1998,14 +1998,14 @@ object Build {
           case `default212Version` =>
             if (!useMinifySizes) {
               Some(ExpectedSizes(
-                  fastLink = 640000 to 641000,
+                  fastLink = 641000 to 642000,
                   fullLink = 101000 to 102000,
                   fastLinkGz = 77000 to 78000,
                   fullLinkGz = 26000 to 27000,
               ))
             } else {
               Some(ExpectedSizes(
-                  fastLink = 494000 to 495000,
+                  fastLink = 495000 to 496000,
                   fullLink = 337000 to 338000,
                   fastLinkGz = 69000 to 70000,
                   fullLinkGz = 50000 to 51000,
@@ -2015,15 +2015,15 @@ object Build {
           case `default213Version` =>
             if (!useMinifySizes) {
               Some(ExpectedSizes(
-                  fastLink = 462000 to 463000,
+                  fastLink = 463000 to 464000,
                   fullLink = 99000 to 100000,
                   fastLinkGz = 60000 to 61000,
                   fullLinkGz = 26000 to 27000,
               ))
             } else {
               Some(ExpectedSizes(
-                  fastLink = 347000 to 348000,
-                  fullLink = 307000 to 308000,
+                  fastLink = 348000 to 349000,
+                  fullLink = 308000 to 309000,
                   fastLinkGz = 54000 to 55000,
                   fullLinkGz = 49000 to 50000,
               ))
diff --git a/test-suite/js/src/test/scala/org/scalajs/testsuite/jsinterop/DynamicTest.scala b/test-suite/js/src/test/scala/org/scalajs/testsuite/jsinterop/DynamicTest.scala
@@ -109,11 +109,19 @@ class DynamicTest {
     assertJSUndefined(obj_anything)
   }
 
-  @Test def objectLiteralInStatementPosition_Issue1627(): Unit = {
-    // Just make sure it does not cause a SyntaxError
+  @Test def objectLiteralInStatementPosition_Issue1627_Issue4949(): Unit = {
+    @noinline def dynProp(): String = "foo"
+
+    // Just make sure those statements do not cause a SyntaxError
     js.Dynamic.literal(foo = "bar")
-    // and also test the case without param (different code path in Printers)
     js.Dynamic.literal()
+    js.Dynamic.literal(foo = "bar").foo
+    js.Dynamic.literal(foo = () => "bar").foo()
+    js.Dynamic.literal(foo = "bar").foo = "babar"
+    js.Dynamic.literal(foo = "foo").selectDynamic(dynProp())
+    js.Dynamic.literal(foo = "foo").updateDynamic(dynProp())("babar")
+    js.Dynamic.literal(foo = () => "bar").applyDynamic(dynProp())()
+    js.Dynamic.literal(foo = "bar") + js.Dynamic.literal(foobar = "babar")
   }
 
   @Test def objectLiteralConstructionWithDynamicNaming(): Unit = {
