False positive unreachable code warning with sealed abstract class

[rossabaker]

I will minimize this after Typelevel Summit, but filing a placeholder now.

• Scala version: 2.13.0-M5
• Java version: 1.8.0_151

In this snippet, the Ok(req.body) is falsely flagged as unreachable code.

  val testService = HttpRoutes.of[IO] {
    case GET -> Root / "request" =>
      Ok("request response")
    case req @ POST -> Root / "post" =>
      Ok(req.body)
  }

We have many similar warnings in the tests using this DSL. No warning is generated in Scala 2.12.8 or 2.11.12.

First spotted in http4s/http4s#2393.

[SethTisue]

"good first issue" because a newcomer could at least attempt to isolate it. (and once isolated, perhaps a fix would be within reach, too.)

[rossabaker]

Sorry, I've failed to come back to minimize this, but 👍 to the good-first-issue. If anyone needs help with the mechanics of building http4s or a tutorial on what that code is doing, I'm happy to help here or on Gitter.

[aeons]

This is still present in 2.13.0.

[rossabaker]

I minimized this to sealed abstract case classes. This warns on false:

sealed abstract class Foo(val a: String)

object Foo {
  def unapply(foo: Foo): Some[String] =
    Some(foo.a)
}

class Issue11457 {
  val root: PartialFunction[Foo, Boolean] = {
    case Foo("a") => true
    case Foo("b") => false
  }
}

If Foo is an unsealed abstract class or a concrete class, there is no warning.

[Igosuki]

Got this when trying to port http4s/rho to 2.13 with the following code :

 def routes(implicit cs: ContextShift[IO]): HttpRoutes[IO] = HttpRoutes.of[IO] {
    // Swagger User Interface
    case req @ GET -> Root / "css" / _       => fetchResource(swaggerUiDir + req.pathInfo, req)
    case req @ GET -> Root / "images" / _    => fetchResource(swaggerUiDir + req.pathInfo, req)
    case req @ GET -> Root / "lib" / _       => fetchResource(swaggerUiDir + req.pathInfo, req)
    case req @ GET -> Root / "swagger-ui"    => fetchResource(swaggerUiDir + "/index.html", req)
    case req @ GET -> Root / "swagger-ui.js" => fetchResource(swaggerUiDir + "/swagger-ui.min.js", req)
  }

[lrytz]

Moved to 2.13.1 since this warning is emitted by default, without special compiler flags.

[lrytz]

I worked on a fix for the minimization, but unfortunately that doesn't fix the original issue in the http4s repo.

The minimization did regress between M4 and M5, in https://github.com/scala/scala/pull/6485/files. I'm not sure if the http4s example also regressed with that change, but it seems likely.

What seems special in the minimization: we are matching on a sealed abstract class with no subclasses MatchAnalysis.scala#L144-L154. Since sealed abstract types are excluded (see scala/scala@311b7de861), the domain in the patmat analyis will be empty Logic.scala#L547; I'm not sure if this violates a precondition. The following WIP fixes the minimization, but as I said earlier not the http4s example. https://github.com/scala/scala/compare/2.13.x...lrytz:t11457?expand=1

@eed3si9n do you maybe have time to take a look?

[leonardehrenfried]

I have just hit this issue when migrating our service to scala 2.13.

Is there a workaround to make the code compile even when turning warnings into errors?

[eed3si9n]

do you maybe have time to take a look?

sorry, I somehow missed this.

Is there a workaround to make the code compile even when turning warnings into errors?

There's a planed feature @lrytz is working on to help there, but in the meantime maybe try silencer? - https://github.com/ghik/silencer

[leonardehrenfried]

Thanks for the tip, @eed3si9n. I can make our code base compile with 2.13.

Since I want to disable the smallest possible number of warnings, does anyone know that the precise name of this warning is? https://docs.scala-lang.org/overviews/compiler-options/index.html

[SethTisue]

#11649 is a duplicate or near-duplicate; #10251, also

[julienrf]

I believe the following snippet is another way to exhibit the same bug: https://scastie.scala-lang.org/eSn923EqTFy4aIHEdp94sg

sealed trait Command {
  type Reply
}

final case class Create() extends Command {
  type Reply = CreateReply
  val reply: Reply = CreateReply()
}

final case class Set() extends Command {
  type Reply = SetReply
  val reply: Reply = SetReply()
}

case class CreateReply()
case class SetReply()

def process[R](command: Command { type Reply = R }): R =
  command match {
    case create: Create => create.reply
    case set: Set       => set.reply
//                         ^
// Warning: unreachable code
  }

[SethTisue]

we haven't succeeded in nerd-sniping anyone into attempting a fix, it seems. but we have attempted to nerd-snipe @som-snytt yet?

"nerd-snipe som snytt yet": say it three times fast

[som-snytt]

I started looking at this and took it on a jog this afternoon. Returning by Coyote Trail, I came to the same conclusion as Lukas (because I'd failed to read his comment first), namely, an extractor never matches null (as I recall from the regex extractor); since a sealed abstract class with no subclasses (including anonymous) can have no instances, an extractor taking that type can never match.

However, in the minimization, just one case should fail to compile, as it must fail to match.

patmat is a nontrivial learning curve. I wasn't sure whether to look first at Scala 2 (as mature) or Scala 3 (as a better rewrite).