Inconsistent behavior of -Ywarn-value-discard flag

[ChernikovP]

It looks like -Ywarn-value-discard flag doesn't always warn on value discarding:

bos-pchernikov-mac:personal pchernikov$ cat UnitOfTrust.scala 
object UnitOfTrust {
  import scala.util._

  private def unitRight[A]: Either[A, Unit] = Right(())

  // fails with:
  //   discarded non-Unit value
  // def test1: Either[Int, Unit] = Right(Right(()))
  // def test2: Either[Int, Unit] = Right(()).map(_ => unitRight[Int])
  // def test3: Either[Int, Unit] = Right(()).map { case _ => unitRight[Int] }

  // fails with:
  //   error: type mismatch;
  //   found   : scala.util.Right[Nothing,Unit]
  //   required: Unit => Unit
  // def test4: Either[Int, Unit] = Right(()).map(Right(()))

  // compiles just fine
  def test5: Either[Int, Unit] = Right(()).map { case _ => unitRight }
  def test6: Either[Int, Unit] = Right(()).map { _ => unitRight }
  def test7: Either[Int, Unit] = Right(()).map(_ => unitRight)
}

scalac output:

bos-pchernikov-mac:personal pchernikov$ ~/Downloads/scala-2.12.8/bin/scalac -Ywarn-value-discard -Xfatal-warnings -print UnitOfTrust.scala
[[syntax trees at end of                   cleanup]] // UnitOfTrust.scala
package <empty> {
  object UnitOfTrust extends Object {
    private def unitRight(): scala.util.Either = new scala.util.Right(scala.runtime.BoxedUnit.UNIT);
    def test5(): scala.util.Either = new scala.util.Right(scala.runtime.BoxedUnit.UNIT).map({
      ((x0$1: scala.runtime.BoxedUnit) => UnitOfTrust.this.$anonfun$test5$1(x0$1))
    });
    def test6(): scala.util.Either = new scala.util.Right(scala.runtime.BoxedUnit.UNIT).map({
      ((x$1: scala.runtime.BoxedUnit) => UnitOfTrust.this.$anonfun$test6$1(x$1))
    });
    def test7(): scala.util.Either = new scala.util.Right(scala.runtime.BoxedUnit.UNIT).map({
      ((x$2: scala.runtime.BoxedUnit) => UnitOfTrust.this.$anonfun$test7$1(x$2))
    });
    final <artifact> private[this] def $anonfun$test5$1(x0$1: scala.runtime.BoxedUnit): Unit = {
      case <synthetic> val x1: scala.runtime.BoxedUnit = x0$1;
      case4(){
        matchEnd3({
          UnitOfTrust.unitRight();
          scala.runtime.BoxedUnit.UNIT
        })
      };
      matchEnd3(x: scala.runtime.BoxedUnit){
        ()
      }
    };
    final <artifact> private[this] def $anonfun$test6$1(x$1: scala.runtime.BoxedUnit): Unit = {
      UnitOfTrust.unitRight();
      ()
    };
    final <artifact> private[this] def $anonfun$test7$1(x$2: scala.runtime.BoxedUnit): Unit = {
      UnitOfTrust.unitRight();
      ()
    };
    def <init>(): UnitOfTrust.type = {
      UnitOfTrust.super.<init>();
      ()
    }
  }
}

This may also have a regression from scala 2.11 to 2.12:

bos-pchernikov-mac:personal pchernikov$ cat UnitOfTrust2_11.scala
object UnitOfTrust {
  import scala.util._

  private def unitRight[A]: Either[A, Unit] = Right(())

  // fails with:
  //   discarded non-Unit value
  // def test1: Either[Int, Unit] = Right(Right(()))
  // def test2: Either[Int, Unit] = Right(()).right.map(_ => unitRight[Int])
  // def test3: Either[Int, Unit] = Right(()).right.map { case _ => unitRight[Int] }

  // fails with:
  //   error: type mismatch;
  //   found   : scala.util.Right[Nothing,Unit]
  //   required: Unit => Unit
  // def test4: Either[Int, Unit] = Right(()).right.map(Right(()))

  // compiles just fine
  def test5: Either[Int, Unit] = Right(()).right.map { case _ => unitRight }
  def test6: Either[Int, Unit] = Right(()).right.map { _ => unitRight }
  def test7: Either[Int, Unit] = Right(()).right.map(_ => unitRight)
}
bos-pchernikov-mac:personal pchernikov$ ~/Downloads/scala-2.11.12/bin/scalac -Ywarn-value-discard -Xfatal-warnings UnitOfTrust2_11.scala
bos-pchernikov-mac:personal pchernikov$

[ChernikovP]

A bit richer example will be with scalaz.\/:

bos-pchernikov-mac:personal pchernikov$ cat UnitOfTrust.scala 
object UnitOfTrust {
  import scalaz._
  import scalaz.Scalaz._

  private def unitRight[A]: A \/ Unit = ().right[A]

  // fails with: discarded non-Unit value
  // def test1: \/[String, Unit] = \/-(\/-(()))
  // def test2: \/[String, Unit] = ().right[String].map(_ => unitRight[String])
  // def test3: \/[String, Unit] = ().right[String].map { case _ => unitRight[String] }

  // fails with: type mismatch;
  // def test4: \/[String, Unit] = ().right[String].right[String]

  // fails with: polymorphic expression cannot be instantiated to expected type;
  // def test5: \/[String, Unit] = ().right[String].map(().right)

  // compiles just fine
  def test6: \/[String, Unit] = ().right[String].map { case _ => unitRight }
  def test7: \/[String, Unit] = ().right[String].map { _ => unitRight}
  def test8: \/[String, Unit] = ().right[String].map(_ => unitRight)
}bos-pchernikov-mac:personal pchernikov$ ~/Downloads/scala-2.12.8/bin/scalac -classpath /Users/pchernikov/.ivy2/cache/org.scalaz/scalaz-core_2.12/bundles/scalaz-core_2.12-7.2.27.jar -Ywarn-value-discad -Xfatal-warnings UnitOfTrust.scala 
bos-pchernikov-mac:personal pchernikov$ 

[nivekastoreth]

I've created an example that I think demonstrates that this issue is very likely related to having multiple type parameters:
Full gist is available here: https://gist.github.com/nivekastoreth/fbe5dfb12687c565587f279909ff04f0

Example specifically pertaining to this claim:

class OneTypeParam[B](value: B) {
  def map[B1](fn: B => B1): OneTypeParam[B1] = new OneTypeParam(fn(value))
  def unitValue: OneTypeParam[Unit] = new OneTypeParam(())
  def checkCompiler: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
}

class TwoTypeParam[A, B](value: B) {
  def map[B1](fn: B => B1): TwoTypeParam[A, B1] = new TwoTypeParam(fn(value))
  def unitValue[C]: TwoTypeParam[C, Unit] = new TwoTypeParam(())
  def checkCompiler: TwoTypeParam[String, Unit] = unitValue.map(_ => unitValue)
}

Compiling this with both -Xfatal-warnings and -Ywarn-value-discard yields only a single warning for OneTypeParam but compiles TwoTypeParam even though they compile to identical class files:

$ rm -f *.class; /build/env/scala-2.12.8/bin/scalac -Xfatal-warnings -Ywarn-value-discard -deprecation -print TypeNumExample.scala
TypeNumExample.scala:4: warning: discarded non-Unit value
  def checkCompiler: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
                                                             ^

[nivekastoreth]

Upon further investigation, this also effects a method marked as having a type that needs solving, even if it isn't used:

class OneTypeParam[B](value: B) {
  def map[B1](fn: B => B1): OneTypeParam[B1] = new OneTypeParam(fn(value))

  def unitValue: OneTypeParam[Unit] = new OneTypeParam(())
  def typedValue[C]: OneTypeParam[Unit] = new OneTypeParam(())

  def checkCompilerUnTyped: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
  def checkCompilerTypedInner: OneTypeParam[Unit] = unitValue.map(_ => typedValue)
  def checkCompilerTypedOuter: OneTypeParam[Unit] = typedValue.map(_ => unitValue)
  def checkCompilerTypedBoth: OneTypeParam[Unit] = typedValue.map(_ => typedValue)
}

This yields warnings on the original footprint (checkCompilerUnTyped as well as on the version with the typed value on the outside (checkCompilerTypedOuter):

$ /build/env/scala-2.11.12/bin/scalac -Xlint -Xfatal-warnings -Ywarn-value-discard -deprecation typeParamNumExample.scala
typeParamNumExample.scala:9: warning: discarded non-Unit value
  def checkCompilerUnTyped: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
                                                                    ^
typeParamNumExample.scala:11: warning: discarded non-Unit value
  def checkCompilerTypedOuter: OneTypeParam[Unit] = typedValue.map(_ => unitValue)

Looking at the typer output yields this tree for checkCompilerUnTyped (warns):

|    |    |    |    |-- ((x$1) => unitValue) : pt=scala.this.Function1[scala.this.Unit,scala.this.Unit] BYVALmode-EXPRmode-POLYmode (site: method checkCompilerUnTyped in OneTypeParam, a MethodSymbol with flags <method>)
|    |    |    |    |    |-- unitValue : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
typeParamNumExample.scala:9: warning: discarded non-Unit value
  def checkCompilerUnTyped: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
                                                                    ^
|    |    |    |    |    |    |-- { OneTypeParam.this.unitValue; () } : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |-- () : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    [adapt] => nonUnit.this.OneTypeParam[scala.this.Unit] adapted to { OneTypeParam.this.unitValue; () } based on pt scala.this.Unit
|    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    \-> scala.this.Function1[scala.this.Unit,scala.this.Unit]

Compared to checkCompilerTypedInner (does not warn)

|    |    |    |    |-- ((x$2) => typedValue) : pt=scala.this.Function1[scala.this.Unit,scala.this.Unit] BYVALmode-EXPRmode-POLYmode (site: method checkCompilerTypedInner in OneTypeParam, a MethodSymbol with flags <method>)
|    |    |    |    |    |-- typedValue : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    [search #1] start `<notype>`, searching for adaptation to pt=scala.this.Function1[nonUnit.this.OneTypeParam[scala.this.Unit],scala.this.Unit] (silent: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>) implicits disabled
|    |    |    |    |    |    [search #2] start `<notype>`, searching for adaptation to pt=scala.this.Function1[scala.this.<byname>[nonUnit.this.OneTypeParam[scala.this.Unit]],scala.this.Unit] (silent: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>) implicits disabled
|    |    |    |    |    |    |-- { OneTypeParam.this.typedValue[C]; () } : pt=scala.this.Unit EXPRmode (silent: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |-- () : pt=scala.this.Unit EXPRmode (silent: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    solving for (C: ?C)
|    |    |    |    |    |    |-- { OneTypeParam.this.typedValue[scala.this.Nothing]; () } : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |-- () : pt=scala.this.Unit EXPRmode (site: value $anonfun in OneTypeParam, a TermSymbol with flags <synthetic>)
|    |    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    |    [adapt] [C]=> nonUnit.this.OneTypeParam[scala.this.Unit] adapted to { OneTypeParam.this.typedValue[scala.this.Nothing]; () } based on pt scala.this.Unit
|    |    |    |    |    |    \-> scala.this.Unit
|    |    |    |    |    \-> scala.this.Function1[scala.this.Unit,scala.this.Unit]

[som-snytt]

As another data point, the minimized example progresses after M5. Maybe that's due to improvements in inference for functions.

$ scalacm -d /tmp -Ywarn-value-discard discarding.scala
discarding.scala:7: warning: discarded non-Unit value
  def checkCompiler: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
                                                             ^
one warning found
$ scalacm -version
Scala compiler version 2.13.0-M5 -- Copyright 2002-2018, LAMP/EPFL and Lightbend, Inc.
$ skalac -version
Scala compiler version 2.13.0-20190122-201544-ecc7a70 -- Copyright 2002-2018, LAMP/EPFL and Lightbend, Inc.
$ skalac -d /tmp -Ywarn-value-discard discarding.scala
discarding.scala:7: warning: discarded non-Unit value
  def checkCompiler: OneTypeParam[Unit] = unitValue.map(_ => unitValue)
                                                             ^
discarding.scala:13: warning: discarded non-Unit value
  def checkCompiler: TwoTypeParam[String, Unit] = unitValue.map(_ => unitValue)
                                                                     ^
two warnings found

[SethTisue]

@som-snytt is this, by any chance, essentially a duplicate of #11998?

[som-snytt]

I pushed the tests for 2.13, but unlikely to backport a fix to 2.12 (which is probably a non-trivial change to type inference).