Fix partial function conversion with inner Match body

[dos65]

The conversion of x => $sel match {<cases>} into PartialFunction wasn't checking if the function body is a match by an actual arg.
That was leading to incorrect conversion if $sel was some other expr

val map = Map(4 -> "foo")
List(1).collect { i =>
  map.get(i) match {
    case Some(i) => i
  }
}
// was converted into icorrect PF
// `new PartialFunction { def applyOrElse(x, default) = map.get(i) match { <cases> } }`

The following case was noticed by missing warning for None case.

[som-snytt]

I have a PR to be revived in this area. #10167

I don't remember what my PR does, but the goal was to improve hygiene or error messages.

The ticket says the example is desirable behavior (toward the end of the discussion).

[lrytz]

The ticket says the example is desirable behavior (toward the end of the discussion).

scala/bug#4940 (comment)

The example also works in Scala 3

[som-snytt]

The example also works in Scala 3

@lrytz I think on x.com they say in their profiles, "The example also works in Scala 3" is not an endorsement, but in this case, I think it is.

x.com is really terrible: I added that to my profile, but I had to delete:

Likes or retweets not an endorsement of not respecting it's vs its.

"The quantum of time isn’t a measure that concerns me."

Edit: also took a minute to update my aspirational avatar.

[dos65]

@som-snytt @lrytz Thanks for the links.

Now I think that it's definitely not a choice to change this behaviour as it won't lead to any compilation error and will change the behaviour of existing code.

I'm wondering, if it makes sense to try to make a special warning for this case.
Something like:

val map = Map(4 -> "foo")
List(1).collect { i =>
  map.get(i) match {  // <- if $sel != param.head & `match` is not exhaustive
// warning: the following `map.get(i) match ...` will be converted into PartialFunction
// to avoid that rewrite the code like `{ i => {val x = map.get(i); x match ... }}` 
// or add `@nowarn`
    case Some(i) => i
  }
}

[som-snytt]

I agree that the missing exhaustiveness check is unnerving. That can probably be remedied by excluding the synthetic arm when checking exhaustiveness.

[SethTisue]

I have now closed scala/bug#4940 as "not planned", as the behavior is plausibly desirable, and if we decided it wasn't, the process of deprecating or disallowing it would need to begin in Scala 3.

@dos65 thanks for calling this to our attention

[SethTisue]

One piece of this story is continuing at #10486