Merge pull request #4556 from gzm0/ec-warn

Fix #4129: Warn if the default global execution context is used.

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

@@ -135,6 +135,11 @@ trait JSDefinitions {
 
     lazy val EnableReflectiveInstantiationAnnotation = getRequiredClass("scala.scalajs.reflect.annotation.EnableReflectiveInstantiation")
 
+    lazy val ExecutionContextModule = getRequiredModule("scala.concurrent.ExecutionContext")
+      lazy val ExecutionContext_global = getMemberMethod(ExecutionContextModule, newTermName("global"))
+
+    lazy val ExecutionContextImplicitsModule = getRequiredModule("scala.concurrent.ExecutionContext.Implicits")
+      lazy val ExecutionContextImplicits_global = getMemberMethod(ExecutionContextImplicitsModule, newTermName("global"))
   }
 
   // scalastyle:on line.size.limit

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

@@ -377,6 +377,34 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
                 |program is unlikely to function properly.""".stripMargin)
           super.transform(tree)
 
+        case tree if tree.symbol == ExecutionContext_global ||
+            tree.symbol == ExecutionContextImplicits_global =>
+          if (scalaJSOpts.warnGlobalExecutionContext) {
+            global.runReporting.warning(tree.pos,
+                """The global execution context in Scala.js is based on JS Promises (microtasks).
+                  |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
+                  |
+                  |Unfortunately, there is no way with ECMAScript only to implement a performant
+                  |macrotask execution context (and hence Scala.js core does not contain one).
+                  |
+                  |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
+                  |Please refer to the README.md of that project for more details regarding
+                  |microtask vs. macrotask execution contexts.
+                  |
+                  |If you do not care about macrotask fairness, you can silence this warning by:
+                  |- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
+                  |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
+                  |- Using scala.scalajs.concurrent.JSExecutionContext.queue
+                  |  (the implementation of ExecutionContext.global in Scala.js) directly.
+                  |
+                  |If you do not care about performance, you can use
+                  |scala.scalajs.concurrent.QueueExecutionContext.timeouts().
+                  |It is based on setTimeout which makes it fair but slow (due to clamping).
+                """.stripMargin,
+                WarningCategory.Other, tree.symbol)
+          }
+          super.transform(tree)
+
         // Validate js.constructorOf[T]
         case TypeApply(ctorOfTree, List(tpeArg))
             if ctorOfTree.symbol == JSPackage_constructorOf =>

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

@@ -38,6 +38,11 @@ trait ScalaJSOptions {
    *  should they be mapped to)? */
   def sourceURIMaps: List[URIMap]
 
+  /** Whether to warn if the global execution context is used.
+   *
+   *  See the warning itself or #4129 for context.
+   */
+  def warnGlobalExecutionContext: Boolean
 }
 
 object ScalaJSOptions {

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

@@ -62,6 +62,7 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
       else
         relSourceMap.toList.map(URIMap(_, absSourceMap))
     }
+    var warnGlobalExecutionContext: Boolean = true
     var _sourceURIMaps: List[URIMap] = Nil
     var relSourceMap: Option[URI] = None
     var absSourceMap: Option[URI] = None
@@ -121,6 +122,8 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin {
         fixClassOf = true
       } else if (option == "genStaticForwardersForNonTopLevelObjects") {
         genStaticForwardersForNonTopLevelObjects = true
+      } else if (option == "nowarnGlobalExecutionContext") {
+        warnGlobalExecutionContext = false
       } else if (option.startsWith("mapSourceURI:")) {
         val uris = option.stripPrefix("mapSourceURI:").split("->")
 

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

@@ -0,0 +1,47 @@
+/*
+ * Scala.js (https://www.scala-js.org/)
+ *
+ * Copyright EPFL.
+ *
+ * Licensed under Apache License 2.0
+ * (https://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package org.scalajs.nscplugin.test
+
+import org.scalajs.nscplugin.test.util._
+import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn
+
+import org.junit.Assume._
+import org.junit.Test
+
+class GlobalExecutionContextNoWarnTest extends DirectTest with TestHelpers {
+
+  override def extraArgs: List[String] =
+    super.extraArgs ::: List("-P:scalajs:nowarnGlobalExecutionContext")
+
+  @Test
+  def noWarnOnUsage: Unit = {
+    """
+    import scala.concurrent.ExecutionContext.global
+
+    object Enclosing {
+      global
+    }
+    """.hasNoWarns()
+  }
+
+  @Test
+  def noWarnOnImplicitUsage: Unit = {
+    """
+    import scala.concurrent.ExecutionContext.Implicits.global
+
+    object Enclosing {
+      scala.concurrent.Future { }
+    }
+    """.hasNoWarns()
+  }
+}

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

@@ -0,0 +1,122 @@
+/*
+ * Scala.js (https://www.scala-js.org/)
+ *
+ * Copyright EPFL.
+ *
+ * Licensed under Apache License 2.0
+ * (https://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package org.scalajs.nscplugin.test
+
+import org.scalajs.nscplugin.test.util._
+import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn
+
+import org.junit.Assume._
+import org.junit.Test
+
+class GlobalExecutionContextWarnTest extends DirectTest with TestHelpers {
+
+  @Test
+  def warnOnUsage: Unit = {
+    """
+    import scala.concurrent.ExecutionContext.global
+
+    object Enclosing {
+      global
+    }
+    """ hasWarns
+    """
+      |newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks).
+      |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
+      |
+      |Unfortunately, there is no way with ECMAScript only to implement a performant
+      |macrotask execution context (and hence Scala.js core does not contain one).
+      |
+      |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
+      |Please refer to the README.md of that project for more details regarding
+      |microtask vs. macrotask execution contexts.
+      |
+      |If you do not care about macrotask fairness, you can silence this warning by:
+      |- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
+      |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
+      |- Using scala.scalajs.concurrent.JSExecutionContext.queue
+      |  (the implementation of ExecutionContext.global in Scala.js) directly.
+      |
+      |If you do not care about performance, you can use
+      |scala.scalajs.concurrent.QueueExecutionContext.timeouts().
+      |It is based on setTimeout which makes it fair but slow (due to clamping).
+      |
+      |      global
+      |      ^
+    """
+  }
+
+  @Test
+  def warnOnImplicitUsage: Unit = {
+    """
+    import scala.concurrent.ExecutionContext.Implicits.global
+
+    object Enclosing {
+      scala.concurrent.Future { }
+    }
+    """ hasWarns
+    """
+      |newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks).
+      |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably.
+      |
+      |Unfortunately, there is no way with ECMAScript only to implement a performant
+      |macrotask execution context (and hence Scala.js core does not contain one).
+      |
+      |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor
+      |Please refer to the README.md of that project for more details regarding
+      |microtask vs. macrotask execution contexts.
+      |
+      |If you do not care about macrotask fairness, you can silence this warning by:
+      |- Adding @nowarn("cat=other") (Scala >= 2.13.x only)
+      |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option
+      |- Using scala.scalajs.concurrent.JSExecutionContext.queue
+      |  (the implementation of ExecutionContext.global in Scala.js) directly.
+      |
+      |If you do not care about performance, you can use
+      |scala.scalajs.concurrent.QueueExecutionContext.timeouts().
+      |It is based on setTimeout which makes it fair but slow (due to clamping).
+      |
+      |      scala.concurrent.Future { }
+      |                              ^
+    """
+  }
+
+  @Test
+  def noWarnIfSelectivelyDisabled: Unit = {
+    assumeTrue(scalaSupportsNoWarn)
+
+    """
+    import scala.annotation.nowarn
+    import scala.concurrent.ExecutionContext.global
+
+    object Enclosing {
+      global: @nowarn("cat=other")
+    }
+    """.hasNoWarns()
+  }
+
+  @Test
+  def noWarnQueue: Unit = {
+    /* Test that JSExecutionContext.queue does not warn for good measure.
+     * We explicitly say it doesn't so we want to notice if it does.
+     */
+
+    """
+    import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
+
+    object Enclosing {
+      scala.concurrent.Future { }
+    }
+    """.hasNoWarns()
+  }
+
+}

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

@@ -17,7 +17,7 @@ import java.util.concurrent.ConcurrentHashMap
 import scala.util.matching.Regex
 
 object ScalaJSVersions extends VersionChecks(
-    current = "1.7.2-SNAPSHOT",
+    current = "1.8.0-SNAPSHOT",
     binaryEmitted = "1.7"
 )
 

junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala

@@ -13,7 +13,13 @@
 package org.scalajs.junit
 
 import scala.concurrent.Future
-import scala.concurrent.ExecutionContext.Implicits.global
+
+/* Use the queue execution context (based on JS promises) explicitly:
+ * We do not have anything better at our disposal and it is accceptable in
+ * terms of fairness: All we use it for is to map over a completed Future once.
+ */
+import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
+
 import scala.util.{Try, Success}
 
 package object async {

junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala

@@ -13,7 +13,14 @@
 package org.scalajs.junit
 
 import scala.concurrent.Future
-import scala.concurrent.ExecutionContext.Implicits.global
+
+/* Use the queue execution context (based on JS promises) explicitly:
+ * We do not have anything better at our disposal and it is accceptable in
+ * terms of fairness: We only use it for test dispatching and orchestation.
+ * The real async work is done in Bootstrapper#invokeTest which does not take
+ * an (implicit) ExecutionContext parameter.
+ */
+import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue
 
 import scala.util.{Try, Success, Failure}
 

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.11.12/neg/choices.check

@@ -0,0 +1,6 @@
+error: Usage: -Yresolve-term-conflict:<strategy>
+ where <strategy> choices are package, object, error (default: error)
+error: bad option: '-Yresolve-term-conflict'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
+error: flags file may only contain compiler options, found: -Yresolve-term-conflict
+four errors found

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.11.12/neg/partestInvalidFlag.check

@@ -0,0 +1,4 @@
+error: bad option: '-badCompilerFlag'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -Yopt:badChoice
+error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -Yopt:badChoice
+three errors found

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.12.15/neg/choices.check

@@ -0,0 +1,5 @@
+error: Usage: -Yresolve-term-conflict:<strategy> where <strategy> choices are package, object, error (default: error).
+error: bad option: '-Yresolve-term-conflict'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
+error: flags file may only contain compiler options, found: -Yresolve-term-conflict
+four errors found

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.12.15/neg/partestInvalidFlag.check

@@ -0,0 +1,4 @@
+error: bad option: '-badCompilerFlag'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -opt:badChoice
+error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -opt:badChoice
+three errors found

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.13.6/neg/choices.check

@@ -0,0 +1,5 @@
+error: Usage: -Yresolve-term-conflict:<strategy> where <strategy> choices are package, object, error (default: error).
+error: bad option: '-Yresolve-term-conflict'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -Yresolve-term-conflict
+error: flags file may only contain compiler options, found: -Yresolve-term-conflict
+4 errors

partest-suite/src/test/resources/scala/tools/partest/scalajs/2.13.6/neg/partestInvalidFlag.check

@@ -0,0 +1,4 @@
+error: bad option: '-badCompilerFlag'
+error: bad options: -P:scalajs:nowarnGlobalExecutionContext -badCompilerFlag notAFlag -opt:badChoice
+error: flags file may only contain compiler options, found: -badCompilerFlag notAFlag -opt:badChoice
+3 errors

partest/src/main/scala-new-partest/scala/tools/partest/scalajs/ScalaJSRunner.scala

@@ -12,6 +12,8 @@
 
 package scala.tools.partest.scalajs
 
+import java.io.File
+
 import scala.tools.partest.nest
 import scala.tools.partest.nest.{AbstractRunner, DirectCompiler, TestInfo}
 
@@ -23,6 +25,12 @@ class ScalaJSRunner(testInfo: ScalaJSTestInfo, suiteRunner: AbstractRunner,
     new DirectCompiler(this) with ScalaJSDirectCompiler
   }
 
+  override def flagsForCompilation(sources: List[File]): List[String] = {
+    // Never warn, so we do not need to update tons of checkfiles.
+    "-P:scalajs:nowarnGlobalExecutionContext" ::
+    super.flagsForCompilation(sources)
+  }
+
   override def extraJavaOptions = {
     super.extraJavaOptions ++ Seq(
         s"-Dscalajs.partest.optMode=${options.optMode.id}",

partest/src/main/scala-old-partest/scala/tools/partest/scalajs/ScalaJSRunner.scala

@@ -47,6 +47,13 @@ class ScalaJSRunner(testFile: File, suiteRunner: SuiteRunner,
   }
 
   override def newCompiler = new DirectCompiler(this) with ScalaJSDirectCompiler
+
+  override def flagsForCompilation(sources: List[File]): List[String] = {
+    // Never warn, so we do not need to update tons of checkfiles.
+    "-P:scalajs:nowarnGlobalExecutionContext" ::
+    super.flagsForCompilation(sources)
+  }
+
   override def extraJavaOptions = {
     super.extraJavaOptions ++ Seq(
         s"-Dscalajs.partest.optMode=${options.optMode.id}",