forked from scala-js/scala-js
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix scala-js#4129: Warn if the default global execution context is used.
The behavior in the presence of imports is unfortunately not ideal here, especially when the implicit ExecutionContext is imported. Ideally, we'd like a warning on the import already and the ability to silence the warning once for an entire file. However, imports get fully resolved in the typer phase, so this would require processing imports in the pre-typer phase, where the entire suppression infrastructure is not available (it requires typing of the @nowarn annotations). Therefore, we live with the sub-optimal situation, especially given that there is a compiler flag to disable the warnings overall.
- Loading branch information
Showing
20 changed files
with
292 additions
and
5 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 47 additions & 0 deletions
47
compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextNoWarnTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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() | ||
} | ||
} |
122 changes: 122 additions & 0 deletions
122
compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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() | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 changes: 6 additions & 0 deletions
6
partest-suite/src/test/resources/scala/tools/partest/scalajs/2.11.12/neg/choices.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
4 changes: 4 additions & 0 deletions
4
...suite/src/test/resources/scala/tools/partest/scalajs/2.11.12/neg/partestInvalidFlag.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
5 changes: 5 additions & 0 deletions
5
partest-suite/src/test/resources/scala/tools/partest/scalajs/2.12.15/neg/choices.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
4 changes: 4 additions & 0 deletions
4
...suite/src/test/resources/scala/tools/partest/scalajs/2.12.15/neg/partestInvalidFlag.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
5 changes: 5 additions & 0 deletions
5
partest-suite/src/test/resources/scala/tools/partest/scalajs/2.13.6/neg/choices.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
4 changes: 4 additions & 0 deletions
4
...-suite/src/test/resources/scala/tools/partest/scalajs/2.13.6/neg/partestInvalidFlag.check
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.