Skip to content

Commit

Permalink
Address new warnings with the strict patmat exhaustivity of 2.13.4.
Browse files Browse the repository at this point in the history
In 2.13.4, the pattern match exhaustivity analysis has become much
stricter. It remains active in the presence of non-sealed classes,
guards, and custom extractors. This change causes a lot of new
warnings in our codebase.

See scala/scala#9140

We address most warnings with `@unchecked`, but for some we throw
more specialized exceptions instead.
  • Loading branch information
sjrd committed Nov 2, 2020
1 parent aa81c35 commit cf8c9d1
Show file tree
Hide file tree
Showing 23 changed files with 98 additions and 50 deletions.
23 changes: 18 additions & 5 deletions compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
Expand Up @@ -273,9 +273,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
try {
def collectClassDefs(tree: Tree): List[ClassDef] = {
tree match {
case EmptyTree => Nil
case PackageDef(_, stats) => stats flatMap collectClassDefs
case cd: ClassDef => cd :: Nil
case EmptyTree => Nil
case PackageDef(_, stats) => stats.flatMap(collectClassDefs)
case cd: ClassDef => cd :: Nil
case _ => abort(s"Unexpected top-level tree at ${tree.pos}:\n$tree")
}
}
val allClassDefs = collectClassDefs(cunit.body)
Expand Down Expand Up @@ -1652,6 +1653,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
(Nil, applyCtor)
case js.Block(prepStats :+ (applyCtor: js.ApplyStatic)) =>
(prepStats, applyCtor)
case _ =>
abort(s"Unexpected body for JS constructor dispatch resolution at ${body.pos}:\n$body")
}
val js.ApplyStatic(_, _, js.MethodIdent(ctorName), js.This() :: ctorArgs) =
applyCtor
Expand Down Expand Up @@ -1731,6 +1734,9 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
if method.name.isConstructor =>
method.name
}.get

case _ =>
abort(s"Unexpected secondary constructor body at ${tree.pos}:\n$tree")
}

val (primaryCtor :: Nil, secondaryCtors) = ctors.partition {
Expand Down Expand Up @@ -2759,6 +2765,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
true
case pat @ Bind(_, _) =>
isMaybeJavaScriptException(pat.symbol.tpe)
case pat =>
abort(s"Unexpected pattern at ${pat.pos}: $pat")
}
}

Expand Down Expand Up @@ -2791,6 +2799,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
val ident = encodeLocalSym(pat.symbol)
val origName = originalNameOfLocal(pat.symbol)
(pat.symbol.tpe, Some((ident, origName)))
case _ =>
abort(s"Unexpected pattern at ${pat.pos}: $pat")
})

// Generate the body that must be executed if the exception matches
Expand Down Expand Up @@ -5200,7 +5210,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
} else if (jsInterop.isJSBracketAccess(sym)) {
assert(argc == 1 || argc == 2,
s"@JSBracketAccess methods should have 1 or 2 non-varargs arguments")
argsNoSpread match {
(argsNoSpread: @unchecked) match {
case List(keyArg) =>
genSelectGet(keyArg)
case List(keyArg, valueArg) =>
Expand Down Expand Up @@ -6418,7 +6428,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
import jsPrimitives._
if (isPrimitive(sym)) {
getPrimitive(sym) match {
case UNITVAL => js.Undefined()
case UNITVAL =>
js.Undefined()
case _ =>
abort(s"Unexpected primitive in genStaticField at $pos: ${sym.fullName}")
}
} else {
val className = encodeClassName(sym.owner)
Expand Down
Expand Up @@ -166,7 +166,8 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)

checkJSNameAnnots(sym)

val transformedTree: Tree = tree match {
// @unchecked needed on 2.13.4+ because MemberDef is not marked `sealed`
val transformedTree: Tree = (tree: @unchecked) match {
case tree: ImplDef =>
if (shouldPrepareExports)
registerClassOrModuleExports(sym)
Expand Down Expand Up @@ -828,6 +829,9 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
}
Some(loadSpec)

case Some(annot) =>
abort(s"checkAndGetJSNativeLoadingSpecAnnotOf returned unexpected annotation $annot")

case None =>
/* We already emitted an error. Invent something not to cause
* cascading errors.
Expand Down
2 changes: 1 addition & 1 deletion ir/src/main/scala/org/scalajs/ir/Trees.scala
Expand Up @@ -1235,7 +1235,7 @@ object Trees {

def isConstructor: Boolean = (ordinal & ConstructorFlag) != 0

def prefixString: String = this match {
def prefixString: String = (this: @unchecked) match {
case Public => ""
case Private => "private "
case PublicStatic => "static "
Expand Down
2 changes: 1 addition & 1 deletion javalib/src/main/scala/java/math/BigDecimal.scala
Expand Up @@ -222,7 +222,7 @@ object BigDecimal {
val absFraction = Math.abs(fraction)
val sigFraction = java.lang.Integer.signum(fraction)
// the carry after rounding
roundingMode match {
(roundingMode: @unchecked) match {
case UP => sigFraction
case DOWN => 0
case CEILING => Math.max(sigFraction, 0)
Expand Down
Expand Up @@ -116,7 +116,7 @@ abstract class CharsetDecoder protected (cs: Charset,
if (result2.isUnmappable()) unmappableCharacterAction()
else malformedInputAction()

action match {
(action: @unchecked) match {
case CodingErrorAction.REPLACE =>
if (out.remaining() < replacement().length) {
CoderResult.OVERFLOW
Expand Down
Expand Up @@ -139,7 +139,7 @@ abstract class CharsetEncoder protected (cs: Charset,
if (result2.isUnmappable()) unmappableCharacterAction()
else malformedInputAction()

action match {
(action: @unchecked) match {
case CodingErrorAction.REPLACE =>
if (out.remaining() < replacement().length) {
CoderResult.OVERFLOW
Expand Down
Expand Up @@ -53,7 +53,7 @@ object ModuleKind {
extends Fingerprint[ModuleKind] {

override def fingerprint(moduleKind: ModuleKind): String = {
moduleKind match {
(moduleKind: @unchecked) match {
case NoModule => "NoModule"
case ESModule => "ESModule"
case CommonJSModule => "CommonJSModule"
Expand Down
Expand Up @@ -31,7 +31,7 @@ object ModuleSplitStyle {
extends Fingerprint[ModuleSplitStyle] {

override def fingerprint(moduleSplitStyle: ModuleSplitStyle): String = {
moduleSplitStyle match {
(moduleSplitStyle: @unchecked) match {
case FewestModules => "FewestModules"
case SmallestModules => "SmallestModules"
}
Expand Down
Expand Up @@ -110,7 +110,7 @@ object Report {
}

private def writeModuleKind(kind: ModuleKind): Unit = {
val i = kind match {
val i = (kind: @unchecked) match {
case ModuleKind.NoModule => 0
case ModuleKind.ESModule => 1
case ModuleKind.CommonJSModule => 2
Expand Down
Expand Up @@ -571,7 +571,7 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) {
methodFun0
}

val field = namespace match {
val field = (namespace: @unchecked) match {
case MemberNamespace.Private => "p"
case MemberNamespace.PublicStatic => "s"
case MemberNamespace.PrivateStatic => "ps"
Expand Down Expand Up @@ -1220,7 +1220,7 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) {
private def genConstValueExportDef(exportName: String,
exportedValue: js.Tree)(
implicit pos: Position): WithGlobals[js.Tree] = {
moduleKind match {
(moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
genAssignToNoModuleExportVar(exportName, exportedValue)

Expand Down Expand Up @@ -1261,7 +1261,7 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) {

val varScope = (className, field.name)

moduleKind match {
(moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
/* Initial value of the export. Updates are taken care of explicitly
* when we assign to the static field.
Expand Down
Expand Up @@ -78,7 +78,7 @@ final class Emitter(config: Emitter.Config) {
def emit(moduleSet: ModuleSet, logger: Logger): Result = {
val WithGlobals(body, globalRefs) = emitInternal(moduleSet, logger)

moduleKind match {
(moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
assert(moduleSet.modules.size <= 1)
val topLevelVars = moduleSet.modules
Expand Down Expand Up @@ -294,7 +294,7 @@ final class Emitter(config: Emitter.Config) {
)
).toList.sortBy(_._1.name)

moduleKind match {
(moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
WithGlobals(Nil)

Expand Down
Expand Up @@ -581,8 +581,7 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
}

case Assign(select @ ArraySelect(array, index), rhs) =>
unnest(List(array, index, rhs)) {
case (List(newArray, newIndex, newRhs), env0) =>
unnest(array, index, rhs) { (newArray, newIndex, newRhs, env0) =>
implicit val env = env0
val genArray = transformExprNoChar(newArray)
val genIndex = transformExprNoChar(newIndex)
Expand Down Expand Up @@ -615,8 +614,8 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
}

case Assign(select @ JSSelect(qualifier, item), rhs) =>
unnest(List(qualifier, item, rhs)) {
case (List(newQualifier, newItem, newRhs), env0) =>
unnest(qualifier, item, rhs) {
(newQualifier, newItem, newRhs, env0) =>
implicit val env = env0
js.Assign(
genBracketSelect(transformExprNoChar(newQualifier),
Expand All @@ -625,8 +624,8 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
}

case Assign(select @ JSSuperSelect(superClass, qualifier, item), rhs) =>
unnest(List(superClass, qualifier, item, rhs)) {
case (List(newSuperClass, newQualifier, newItem, newRhs), env0) =>
unnest(superClass, qualifier, item, rhs) {
(newSuperClass, newQualifier, newItem, newRhs, env0) =>
implicit val env = env0
genCallHelper("superSet", transformExprNoChar(newSuperClass),
transformExprNoChar(newQualifier), transformExprNoChar(item),
Expand Down Expand Up @@ -1069,17 +1068,39 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
/** Same as above, for a single argument */
def unnest(arg: Tree)(makeStat: (Tree, Env) => js.Tree)(
implicit env: Env): js.Tree = {
unnest(List(arg)) {
case (List(newArg), env) => makeStat(newArg, env)
unnest(List(arg)) { (newArgs, env) =>
val newArg :: Nil = newArgs
makeStat(newArg, env)
}
}

/** Same as above, for two arguments */
def unnest(lhs: Tree, rhs: Tree)(
def unnest(arg1: Tree, arg2: Tree)(
makeStat: (Tree, Tree, Env) => js.Tree)(
implicit env: Env): js.Tree = {
unnest(List(lhs, rhs)) {
case (List(newLhs, newRhs), env) => makeStat(newLhs, newRhs, env)
unnest(List(arg1, arg2)) { (newArgs, env) =>
val newArg1 :: newArg2 :: Nil = newArgs
makeStat(newArg1, newArg2, env)
}
}

/** Same as above, for 3 arguments */
def unnest(arg1: Tree, arg2: Tree, arg3: Tree)(
makeStat: (Tree, Tree, Tree, Env) => js.Tree)(
implicit env: Env): js.Tree = {
unnest(List(arg1, arg2, arg3)) { (newArgs, env) =>
val newArg1 :: newArg2 :: newArg3 :: Nil = newArgs
makeStat(newArg1, newArg2, newArg3, env)
}
}

/** Same as above, for 4 arguments */
def unnest(arg1: Tree, arg2: Tree, arg3: Tree, arg4: Tree)(
makeStat: (Tree, Tree, Tree, Tree, Env) => js.Tree)(
implicit env: Env): js.Tree = {
unnest(List(arg1, arg2, arg3, arg4)) { (newArgs, env) =>
val newArg1 :: newArg2 :: newArg3 :: newArg4 :: Nil = newArgs
makeStat(newArg1, newArg2, newArg3, newArg4, env)
}
}

Expand Down Expand Up @@ -1702,8 +1723,8 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
}

case JSSuperSelect(superClass, qualifier, item) =>
unnest(List(superClass, qualifier, item)) {
case (List(newSuperClass, newQualifier, newItem), env) =>
unnest(superClass, qualifier, item) {
(newSuperClass, newQualifier, newItem, env) =>
redo(JSSuperSelect(newSuperClass, newQualifier, newItem))(env)
}

Expand Down
Expand Up @@ -188,6 +188,10 @@ private[emitter] final class SJSGen(
case BoxedFloatClass => genIsFloat(expr)
case BoxedDoubleClass => typeof(expr) === "number"
case BoxedStringClass => typeof(expr) === "string"

case _ =>
throw new IllegalArgumentException(
s"hijacked class required but got ${className.nameString}")
}
}

Expand Down Expand Up @@ -351,7 +355,7 @@ private[emitter] final class SJSGen(
}

case irt.JSNativeLoadSpec.ImportWithGlobalFallback(importSpec, globalSpec) =>
moduleKind match {
(moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
genLoadJSFromSpec(globalSpec, keepOnlyDangerousVarNames)
case ModuleKind.ESModule | ModuleKind.CommonJSModule =>
Expand Down
Expand Up @@ -207,7 +207,7 @@ private[emitter] final class VarGen(jsGen: JSGen, nameGen: NameGen,
if (moduleContext.public) {
WithGlobals(tree)
} else {
val export = config.moduleKind match {
val export = (config.moduleKind: @unchecked) match {
case ModuleKind.NoModule =>
throw new AssertionError("non-public module in NoModule mode")

Expand Down
Expand Up @@ -1486,7 +1486,7 @@ object IRChecker {
extends AnyVal {

override def toString(): String = {
val (pos, name) = nodeOrLinkedClass match {
val (pos, name) = (nodeOrLinkedClass: @unchecked) match {
case tree: IRNode => (tree.pos, tree.getClass.getSimpleName)
case linkedClass: LinkedClass => (linkedClass.pos, "ClassDef")
}
Expand Down
Expand Up @@ -45,7 +45,7 @@ final class LinkerFrontendImpl private (config: LinkerFrontendImpl.Config)

private[this] val refiner: Refiner = new Refiner(config.commonConfig)

private[this] val splitter: ModuleSplitter = config.moduleSplitStyle match {
private[this] val splitter: ModuleSplitter = (config.moduleSplitStyle: @unchecked) match {
case ModuleSplitStyle.FewestModules => ModuleSplitter.maxSplitter()
case ModuleSplitStyle.SmallestModules => ModuleSplitter.minSplitter()
}
Expand Down
Expand Up @@ -155,7 +155,7 @@ final class ModuleSplitter private (analyzer: ModuleAnalyzer) {
add(tle.tree.moduleID, tle.staticDependencies)

for (mi <- unit.moduleInitializers) {
val dep = mi.initializer match {
val dep = (mi.initializer: @unchecked) match {
case ModuleInitializerImpl.VoidMainMethod(className, _) =>
className
case ModuleInitializerImpl.MainMethodWithArgs(className, _, _) =>
Expand Down
4 changes: 2 additions & 2 deletions test-suite/js/src/test/resources/SourceMapTestTemplate.scala
Expand Up @@ -486,7 +486,7 @@ class Json extends Writer2{
linePos = chLinePos
charPos = chCharPos
val kind: Int = chKind
kind match {
(kind: @unchecked) match {
case Letter =>
val first = chMark
while (chKind == Letter || chKind == Digit) {
Expand Down Expand Up @@ -628,7 +628,7 @@ class Json extends Writer2{
}
def getJson(): JsValue = {
val kind: Int = tokenKind
val result: JsValue = kind match {
val result: JsValue = (kind: @unchecked) match {
case ID =>
val result: JsValue = tokenValue match {
case "true" => JsTrue
Expand Down
Expand Up @@ -72,7 +72,7 @@ class InstanceTestsHijackedBoxedClassesTest {
assertTrue((1.2: Any).isInstanceOf[Float])

// from the bug report
def test(x: Any): String = x match {
def test(x: Any): String = (x: @unchecked) match {
case f: Float => "ok"
}
assertEquals("ok", test(0.2))
Expand Down
Expand Up @@ -169,7 +169,7 @@ class JSSymbolTest {
@Test def native_with_overloaded_runtime_dispatch_methods(): Unit = {
val obj = mkObject(
sym1 -> { (x: Any) =>
x match {
(x: @unchecked) match {
case x: Int => x + 3
case x: String => "Hello " + x
}
Expand Down
Expand Up @@ -309,9 +309,11 @@ class ArrayOpsTest {
import js.Any.jsArrayOps

val array = js.Array[Any](1, "one", 2, "two", 3, "three")
val resultInferType = array.partitionMap {
case x: Int => Left(x)
case x: String => Right(x)
val resultInferType = array.partitionMap { x =>
(x: @unchecked) match {
case x: Int => Left(x)
case x: String => Right(x)
}
}
val result: (js.Array[Int], js.Array[String]) = resultInferType
assertJSArrayPairEquals((js.Array(1, 2, 3), js.Array("one", "two", "three")), result)
Expand Down

0 comments on commit cf8c9d1

Please sign in to comment.