Warn if match in block is not used for PF

Author: som-snytt

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -223,6 +223,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case IllegalUnrollPlacementID // errorNumber: 207
   case ExtensionHasDefaultID // errorNumber: 208
   case FormatInterpolationErrorID // errorNumber: 209
+  case MatchIsNotPartialFunctionID // errorNumber: 210
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3447,5 +3447,35 @@ end IllegalUnrollPlacement
 
 class BadFormatInterpolation(errorText: String)(using Context) extends Message(FormatInterpolationErrorID):
   def kind = MessageKind.Interpolation
-  def msg(using Context) = errorText
-  def explain(using Context) = ""
+  protected def msg(using Context) = errorText
+  protected def explain(using Context) = ""
+
+class MatchIsNotPartialFunction(using Context) extends SyntaxMsg(MatchIsNotPartialFunctionID):
+  protected def msg(using Context) =
+    "match expression in result of block will not be used to synthesize partial function"
+  protected def explain(using Context) =
+    i"""A `PartialFunction` can be synthesized from a function literal if its body is just a pattern match.
+       |
+       |For example, `collect` takes a `PartialFunction`.
+       |  (1 to 10).collect(i => i match { case n if n % 2 == 0 => n })
+       |is equivalent to using a "pattern-matching anonymous function" directly:
+       |  (1 to 10).collect { case n if n % 2 == 0 => n }
+       |Compare an operation that requires a `Function1` instead:
+       |  (1 to 10).map { case n if n % 2 == 0 => n case n => n + 1 }
+       |
+       |As a convenience, the "selector expression" of the match can be an arbitrary expression:
+       |  List("1", "two", "3").collect(x => Try(x.toInt) match { case Success(i) => i })
+       |In this example, `isDefinedAt` evaluates the selector expression and any guard expressions
+       |in the pattern match in order to report whether an input is in the domain of the function.
+       |
+       |However, blocks of statements are not supported by this idiom:
+       |  List("1", "two", "3").collect: x =>
+       |    val maybe = Try(x.toInt) // statements preceding the match
+       |    maybe match
+       |    case Success(i) if i % 2 == 0 => i // throws MatchError on cases not covered
+       |
+       |This restriction is enforced to simplify the evaluation semantics of the partial function.
+       |Otherwise, it might not be clear what is computed by `isDefinedAt`.
+       |
+       |Efficient operations will use `applyOrElse` to avoid computing the match twice,
+       |but the `apply` body would be executed "per element" in the example."""

compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala

@@ -126,10 +126,16 @@ class ExpandSAMs extends MiniPhase:
     // The right hand side from which to construct the partial function. This is always a Match.
     // If the original rhs is already a Match (possibly in braces), return that.
     // Otherwise construct a match `x match case _ => rhs` where `x` is the parameter of the closure.
-    def partialFunRHS(tree: Tree): Match = tree match
+    def partialFunRHS(tree: Tree): Match =
+      inline def checkMatch(): Unit =
+        tree match
+        case Block(_, m: Match) => report.warning(reporting.MatchIsNotPartialFunction(), m.srcPos)
+        case _ =>
+      tree match
       case m: Match => m
       case Block(Nil, expr) => partialFunRHS(expr)
       case _ =>
+        checkMatch()
         Match(ref(param.symbol),
           CaseDef(untpd.Ident(nme.WILDCARD).withType(param.symbol.info), EmptyTree, tree) :: Nil)
 

tests/warn/i21649.scala

@@ -0,0 +1,16 @@
+//> using options -explain
+// do warn if function literal adaptation has match in non-empty block
+
+import scala.util.*
+import PartialFunction.condOpt
+
+val f: PartialFunction[String, Boolean] = _.toUpperCase match { case "TRUE" => true }
+
+val g: PartialFunction[String, Boolean] = x =>
+  val uc = x.toUpperCase
+  uc match // warn
+  case "TRUE" => true
+
+def kollekt = List("1", "two", "3").collect(x => Try(x.toInt) match { case Success(i) => i })
+
+def lesen = List("1", "two", "3").flatMap(x => condOpt(Try(x.toInt)) { case Success(i) => i })