Merge pull request #4706 from sjrd/avoid-identifier-await

Fix #4705: Avoid emitting identifiers called 'await'.

compiler/src/main/scala/org/scalajs/nscplugin/CompatComponent.scala

@@ -127,6 +127,7 @@ trait CompatComponent {
   object WarningCategoryCompat {
     object Reporting {
       object WarningCategory {
+        val Deprecation: Any = null
         val Other: Any = null
       }
     }

compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

@@ -6639,31 +6639,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
           js.JSSelect(qualTree, item)
 
         case MaybeGlobalScope.GlobalScope(_) =>
-          item match {
-            case js.StringLiteral(value) =>
-              if (js.JSGlobalRef.isValidJSGlobalRefName(value)) {
-                js.JSGlobalRef(value)
-              } else if (js.JSGlobalRef.ReservedJSIdentifierNames.contains(value)) {
-                reporter.error(pos,
-                    "Invalid selection in the global scope of the reserved " +
-                    s"identifier name `$value`." +
-                    GenericGlobalObjectInformationMsg)
-                js.JSGlobalRef("erroneous")
-              } else {
-                reporter.error(pos,
-                    "Selecting a field of the global scope whose name is " +
-                    "not a valid JavaScript identifier is not allowed." +
-                    GenericGlobalObjectInformationMsg)
-                js.JSGlobalRef("erroneous")
-              }
-
-            case _ =>
-              reporter.error(pos,
-                  "Selecting a field of the global scope with a dynamic " +
-                  "name is not allowed." +
-                  GenericGlobalObjectInformationMsg)
-              js.JSGlobalRef("erroneous")
-          }
+          genJSGlobalRef(item, "Selecting a field", "selection")
       }
     }
 
@@ -6686,31 +6662,43 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
           js.JSMethodApply(receiverTree, method, args)
 
         case MaybeGlobalScope.GlobalScope(_) =>
-          method match {
-            case js.StringLiteral(value) =>
-              if (js.JSGlobalRef.isValidJSGlobalRefName(value)) {
-                js.JSFunctionApply(js.JSGlobalRef(value), args)
-              } else if (js.JSGlobalRef.ReservedJSIdentifierNames.contains(value)) {
-                reporter.error(pos,
-                    "Invalid call in the global scope of the reserved " +
-                    s"identifier name `$value`." +
-                    GenericGlobalObjectInformationMsg)
-                js.Undefined()
-              } else {
-                reporter.error(pos,
-                    "Calling a method of the global scope whose name is not " +
-                    "a valid JavaScript identifier is not allowed." +
-                    GenericGlobalObjectInformationMsg)
-                js.Undefined()
-              }
-
-            case _ =>
-              reporter.error(pos,
-                  "Calling a method of the global scope with a dynamic " +
-                  "name is not allowed." +
-                  GenericGlobalObjectInformationMsg)
-              js.Undefined()
+          val globalRef = genJSGlobalRef(method, "Calling a method", "call")
+          js.JSFunctionApply(globalRef, args)
+      }
+    }
+
+    private def genJSGlobalRef(propName: js.Tree,
+        actionFull: String, actionSimpleNoun: String)(
+        implicit pos: Position): js.JSGlobalRef = {
+      propName match {
+        case js.StringLiteral(value) =>
+          if (js.JSGlobalRef.isValidJSGlobalRefName(value)) {
+            if (value == "await") {
+              global.runReporting.warning(pos,
+                  s"$actionFull of the global scope with the name '$value' is deprecated.\n" +
+                  "  It may produce invalid JavaScript code causing a SyntaxError in some environments." +
+                  GenericGlobalObjectInformationMsg,
+                  WarningCategory.Deprecation,
+                  currentMethodSym.get)
+            }
+            js.JSGlobalRef(value)
+          } else if (js.JSGlobalRef.ReservedJSIdentifierNames.contains(value)) {
+            reporter.error(pos,
+                s"Invalid $actionSimpleNoun in the global scope of the reserved identifier name `$value`." +
+                GenericGlobalObjectInformationMsg)
+            js.JSGlobalRef("erroneous")
+          } else {
+            reporter.error(pos,
+                s"$actionFull of the global scope whose name is not a valid JavaScript identifier is not allowed." +
+                GenericGlobalObjectInformationMsg)
+            js.JSGlobalRef("erroneous")
           }
+
+        case _ =>
+          reporter.error(pos,
+              s"$actionFull of the global scope with a dynamic name is not allowed." +
+              GenericGlobalObjectInformationMsg)
+          js.JSGlobalRef("erroneous")
       }
     }
 

compiler/src/test/scala/org/scalajs/nscplugin/test/JSExportTest.scala

@@ -1033,12 +1033,6 @@ class JSExportTest extends DirectTest with TestHelpers {
 
   }
 
-  private def since(v: String): String = {
-    val version = scala.util.Properties.versionNumberString
-    if (version.startsWith("2.11.")) ""
-    else s" (since $v)"
-  }
-
   @Test
   def noExportTopLevelTrait(): Unit = {
     """

compiler/src/test/scala/org/scalajs/nscplugin/test/JSGlobalScopeTest.scala

@@ -13,14 +13,19 @@
 package org.scalajs.nscplugin.test
 
 import org.scalajs.nscplugin.test.util._
+import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn
 
 import org.junit.Test
 import org.junit.Ignore
+import org.junit.Assume._
 
 // scalastyle:off line.size.limit
 
 class JSGlobalScopeTest extends DirectTest with TestHelpers {
 
+  override def extraArgs: List[String] =
+    super.extraArgs :+ "-deprecation"
+
   override def preamble: String = {
     """
     import scala.scalajs.js
@@ -244,6 +249,9 @@ class JSGlobalScopeTest extends DirectTest with TestHelpers {
     }
     """ hasErrors
     s"""
+      |newSource1.scala:41: warning: method apply in object global is deprecated${since("forever")}: The global scope cannot be called as function.
+      |        val a = js.Dynamic.global(3)
+      |                           ^
       |newSource1.scala:41: error: Loading the global scope as a value (anywhere but as the left-hand-side of a `.`-selection) is not allowed.
       |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
       |        val a = js.Dynamic.global(3)
@@ -390,4 +398,103 @@ class JSGlobalScopeTest extends DirectTest with TestHelpers {
     }
   }
 
+  @Test
+  def warnAboutAwaitReservedWord_Issue4705(): Unit = {
+    val reservedIdentifiers = List("await")
+
+    for (reservedIdentifier <- reservedIdentifiers) {
+      val spaces = " " * reservedIdentifier.length()
+
+      s"""
+      @js.native
+      @JSGlobalScope
+      object CustomGlobalScope extends js.Any {
+        var `$reservedIdentifier`: Int = js.native
+        @JSName("$reservedIdentifier")
+        def `${reservedIdentifier}2`(x: Int): Int = js.native
+      }
+
+      object Main {
+        def main(): Unit = {
+          val a = js.Dynamic.global.`$reservedIdentifier`
+          js.Dynamic.global.`$reservedIdentifier` = 5
+          val b = js.Dynamic.global.`$reservedIdentifier`(5)
+
+          val c = CustomGlobalScope.`$reservedIdentifier`
+          CustomGlobalScope.`$reservedIdentifier` = 5
+          val d = CustomGlobalScope.`${reservedIdentifier}2`(5)
+        }
+      }
+      """ hasWarns
+      s"""
+        |newSource1.scala:49: warning: Selecting a field of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          val a = js.Dynamic.global.`$reservedIdentifier`
+        |                             ^
+        |newSource1.scala:50: warning: Selecting a field of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          js.Dynamic.global.`$reservedIdentifier` = 5
+        |                     ^
+        |newSource1.scala:51: warning: Calling a method of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          val b = js.Dynamic.global.`$reservedIdentifier`(5)
+        |                                      $spaces^
+        |newSource1.scala:53: warning: Selecting a field of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          val c = CustomGlobalScope.`$reservedIdentifier`
+        |                                    ^
+        |newSource1.scala:54: warning: Selecting a field of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          CustomGlobalScope.`$reservedIdentifier` = 5
+        |                               $spaces^
+        |newSource1.scala:55: warning: Calling a method of the global scope with the name '$reservedIdentifier' is deprecated.
+        |  It may produce invalid JavaScript code causing a SyntaxError in some environments.
+        |  See https://www.scala-js.org/doc/interoperability/global-scope.html for further information.
+        |          val d = CustomGlobalScope.`${reservedIdentifier}2`(5)
+        |                                       $spaces^
+      """
+    }
+  }
+
+  @Test
+  def noWarnAboutAwaitReservedWordIfSelectivelyDisabled(): Unit = {
+    assumeTrue(scalaSupportsNoWarn)
+
+    val reservedIdentifiers = List("await")
+
+    for (reservedIdentifier <- reservedIdentifiers) {
+      val spaces = " " * reservedIdentifier.length()
+
+      s"""
+      import scala.annotation.nowarn
+
+      @js.native
+      @JSGlobalScope
+      object CustomGlobalScope extends js.Any {
+        var `$reservedIdentifier`: Int = js.native
+        @JSName("$reservedIdentifier")
+        def `${reservedIdentifier}2`(x: Int): Int = js.native
+      }
+
+      object Main {
+        @nowarn("cat=deprecation")
+        def main(): Unit = {
+          val a = js.Dynamic.global.`$reservedIdentifier`
+          js.Dynamic.global.`$reservedIdentifier` = 5
+          val b = js.Dynamic.global.`$reservedIdentifier`(5)
+
+          val c = CustomGlobalScope.`$reservedIdentifier`
+          CustomGlobalScope.`$reservedIdentifier` = 5
+          val d = CustomGlobalScope.`${reservedIdentifier}2`(5)
+        }
+      }
+      """.hasNoWarns()
+    }
+  }
+
 }

compiler/src/test/scala/org/scalajs/nscplugin/test/util/TestHelpers.scala

@@ -32,6 +32,12 @@ trait TestHelpers extends DirectTest {
   /** will be prefixed to every code that is compiled. use for imports */
   def preamble: String = ""
 
+  protected def since(v: String): String = {
+    val version = scala.util.Properties.versionNumberString
+    if (version.startsWith("2.11.")) ""
+    else s" (since $v)"
+  }
+
   /** pimps a string to compile it and apply the specified test */
   implicit class CompileTests(val code: String) {
     private lazy val (success, output) = {

ir/shared/src/main/scala/org/scalajs/ir/Trees.scala

@@ -863,6 +863,11 @@ object Trees {
      *  - The identifier `arguments`, because any attempt to refer to it always
      *    refers to the magical `arguments` pseudo-array from the enclosing
      *    function, rather than a global variable.
+     *
+     *  This set does *not* contain `await`, although it is a reserved word
+     *  within ES modules. It used to be allowed before 1.11.0, and even
+     *  browsers do not seem to reject it. For compatibility reasons, we only
+     *  warn about it at compile time, but the IR allows it.
      */
     final val ReservedJSIdentifierNames: Set[String] = Set(
         "arguments", "break", "case", "catch", "class", "const", "continue",

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/NameGen.scala

@@ -355,20 +355,22 @@ private object NameGen {
    *  - All ECMAScript 2015 keywords;
    *  - Identifier names that are treated as keywords in ECMAScript 2015
    *    Strict Mode;
+   *  - Identifier names that are treated as keywords in some contexts, such as
+   *    ES modules;
    *  - The identifiers `arguments` and `eval`, because they cannot be used for
    *    local variable names in ECMAScript 2015 Strict Mode;
    *  - The identifier `undefined`, because that's way too confusing if it does
    *    not actually mean `void 0`, and who knows what JS engine performance
    *    cliffs we can trigger with that.
    */
   private final val ReservedJSIdentifierNames: Set[String] = Set(
-      "arguments", "break", "case", "catch", "class", "const", "continue",
-      "debugger", "default", "delete", "do", "else", "enum", "eval", "export",
-      "extends", "false", "finally", "for", "function", "if", "implements",
-      "import", "in", "instanceof", "interface", "let", "new", "null",
-      "package", "private", "protected", "public", "return", "static", "super",
-      "switch", "this", "throw", "true", "try", "typeof", "undefined", "var",
-      "void", "while", "with", "yield"
+      "arguments", "await", "break", "case", "catch", "class", "const",
+      "continue", "debugger", "default", "delete", "do", "else", "enum",
+      "eval", "export", "extends", "false", "finally", "for", "function", "if",
+      "implements", "import", "in", "instanceof", "interface", "let", "new",
+      "null", "package", "private", "protected", "public", "return", "static",
+      "super", "switch", "this", "throw", "true", "try", "typeof", "undefined",
+      "var", "void", "while", "with", "yield"
   )
 
   private val compressedPrefixes: List[(UTF8String, String)] = {

linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala

@@ -5954,11 +5954,11 @@ private[optimizer] object OptimizerCore {
      *  (with ASCII characters only).
      */
     private val EmitterReservedJSIdentifiers = List(
-        "arguments", "break", "case", "catch", "class", "const", "continue",
-        "debugger", "default", "delete", "do", "else", "enum", "eval",
-        "export", "extends", "false", "finally", "for", "function", "if",
-        "implements", "import", "in", "instanceof", "interface", "let", "new",
-        "null", "package", "private", "protected", "public", "return",
+        "arguments", "await", "break", "case", "catch", "class", "const",
+        "continue", "debugger", "default", "delete", "do", "else", "enum",
+        "eval", "export", "extends", "false", "finally", "for", "function",
+        "if", "implements", "import", "in", "instanceof", "interface", "let",
+        "new", "null", "package", "private", "protected", "public", "return",
         "static", "super", "switch", "this", "throw", "true", "try", "typeof",
         "undefined", "var", "void", "while", "with", "yield"
     )