Merge pull request scala#4016 from lrytz/t8731

SI-8731 warning if @switch is ignored

Author: gkossakowski

src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala

@@ -550,8 +550,24 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
                 case _ => false
               }
               val suppression = Suppression(suppressExhaustive, supressUnreachable)
+              val hasSwitchAnnotation = treeInfo.isSwitchAnnotation(tpt.tpe)
               // matches with two or fewer cases need not apply for switchiness (if-then-else will do)
-              val requireSwitch = treeInfo.isSwitchAnnotation(tpt.tpe) && casesNoSubstOnly.lengthCompare(2) > 0
+              // `case 1 | 2` is considered as two cases.
+              def exceedsTwoCasesOrAlts = {
+                // avoids traversing the entire list if there are more than 3 elements
+                def lengthMax3[T](l: List[T]): Int = l match {
+                  case a :: b :: c :: _ => 3
+                  case cases =>
+                    cases.map({
+                      case AlternativesTreeMaker(_, alts, _) :: _ => lengthMax3(alts)
+                      case c => 1
+                    }).sum
+                }
+                lengthMax3(casesNoSubstOnly) > 2
+              }
+              val requireSwitch = hasSwitchAnnotation && exceedsTwoCasesOrAlts
+              if (hasSwitchAnnotation && !requireSwitch)
+                reporter.warning(scrut.pos, "matches with two cases or fewer are emitted using if-then-else instead of switch")
               (suppression, requireSwitch)
             case _ =>
               (Suppression.NoSuppression, false)

test/files/neg/t6902.scala

@@ -16,7 +16,7 @@ object Test {
   // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50)
   // at scala.tools.nsc.Global.abort(Global.scala:249)
   // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850)
-  ((1: Byte): @unchecked @annotation.switch) match {
+  ((1: Byte): @unchecked) match {
     case 1 => 2
     case 1 => 3 // crash
   }

test/files/neg/t8731.check

@@ -0,0 +1,9 @@
+t8731.scala:5: warning: matches with two cases or fewer are emitted using if-then-else instead of switch
+  def f(x: Int) = (x: @annotation.switch) match {
+                       ^
+t8731.scala:10: warning: could not emit switch for @switch annotated match
+  def g(x: Int) = (x: @annotation.switch) match {
+                       ^
+error: No warnings can be incurred under -Xfatal-warnings.
+two warnings found
+one error found

test/files/neg/t8731.flags

@@ -0,0 +1 @@
+-Xfatal-warnings

test/files/neg/t8731.scala

@@ -0,0 +1,15 @@
+class C {
+  // not a compile-time constant due to return type
+  final val K: Int = 20
+
+  def f(x: Int) = (x: @annotation.switch) match {
+    case K => 0
+    case 2 => 1
+  }
+
+  def g(x: Int) = (x: @annotation.switch) match {
+    case K => 0
+    case 2 => 1
+    case 3 => 2
+  }
+}

test/files/pos/switch-small.flags

@@ -1,5 +1,6 @@
 a with oef
 a with oef
+a with oef
 a
 def with oef
 def

test/files/pos/switch-small.scala

@@ -1,12 +1,11 @@
 import scala.annotation.switch
 
 object Test extends App {
-  // TODO: should not emit a switch
-  // def noSwitch(ch: Char, eof: Boolean) = (ch: @switch) match {
-  //   case 'a' if eof => println("a with oef") // then branch
-  // }
+  def noSwitch(ch: Char, eof: Boolean) = ch match {
+    case 'a' if eof => println("a with oef") // then branch
+  }
 
-  def onlyThen(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def onlyThen(ch: Char, eof: Boolean) = ch match {
     case 'a' if eof => println("a with oef") // then branch
     case 'c' =>
   }
@@ -18,7 +17,7 @@ object Test extends App {
     case 'c' =>
   }
 
-  def defaultUnguarded(ch: Char, eof: Boolean) = (ch: @switch) match {
+  def defaultUnguarded(ch: Char, eof: Boolean) = ch match {
     case ' ' if eof => println("spacey oef")
     case _ => println("default")
   }
@@ -44,7 +43,7 @@ object Test extends App {
   //   case 'c' =>
   // }
 
-  // noSwitch('a', true)
+  noSwitch('a', true)
   onlyThen('a', true)       // 'a with oef'
   ifThenElse('a', true)     // 'a with oef'
   ifThenElse('a', false)    // 'a'

test/files/run/t5830.check

@@ -6,7 +6,7 @@ object Test extends App {
   // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50)
   // at scala.tools.nsc.Global.abort(Global.scala:249)
   // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850)
-  ((1: Byte): @unchecked @annotation.switch) match {
+  ((1: Byte): @unchecked) match {
     case 1 => 2
     case 1 => 3 // crash
   }