Nothing warns about unconsumed values in for comprehension

[keynmol]

Compiler version

3.3.1+ (tested on nightly as well)

Minimized code

This shows several places where it works and the one where it doesn't

//> using scala 3.nightly
//> using option -Wnonunit-statement 

def method: List[Int] = List(25)

@main def repro = 
  method // WARNING here ✅
  
  val _ = for 
    x <- method 
    _ = method // Nothing here ❌
  yield x

  method.flatMap: x =>
    method.map: y=>
      method // WARNING here ✅
      List()

Output

// TODO add output here

Expectation

I expected a warning because it's a non-unit statement (?).

[keynmol]

As a note, fixing this would be a huge improvement in purely functional codebases (CE/ZIO) - this failure mode is responsible for quietly missed log statements, assertions that aren't actioned, concurrency bugs, etc.

[SethTisue]

Scala 2.13 doesn't warn either:

% cat S.scala      
//> using scala 2.13.12
//> using option -Wnonunit-statement

object Repro extends App {
  def method: List[Int] = List(25)
  def foo(): List[Int] =
    for {
      x <- method
      _ = method // Nothing here ❌
    } yield x
}
% scala-cli compile S.scala
Compiling project (Scala 2.13.12, JVM (17))
Compiled project (Scala 2.13.12, JVM (17))

@som-snytt probably knows if there's a Scala 2 ticket on that

[szymon-rd]

I will look at that after I finish the current couple of tasks on linting.

[aherlihy]

Closing this issue as not a bug, because the purpose of this syntax (_ = method) is to explicitly discard this warning. It would be inconsistent to add a warning here, but not in other contexts where _ = ... is used. We could consider adding a different, new warning to catch this particular case, or contribute to the discussion on improvements on for comprehension syntax, but either case wouldn't make use of -Wnounit-statement.

[som-snytt]

Trees at typer for scala 2 & 3:

➜  snips scalac -Wnonunit-statement -Xlint -Vprint -d /tmp/sandbox for-unused.scala
[[syntax trees at end of                     typer]] // for-unused.scala
package <empty> {
  class C extends scala.AnyRef {
    def <init>(): C = {
      C.super.<init>();
      ()
    };
    def i: Int = 42;
    def f(xs: List[Int]): List[Int] = xs.map[(Int, Int)](((x: Int) => {
  val y: Int = C.this.i;
  scala.Tuple2.apply[Int, Int](x, y)
})).map[Int](((x$1: (Int, Int)) => (x$1: (Int, Int) @unchecked) match {
      case (_1: Int, _2: Int): (Int, Int)((x @ _), (y @ _)) => x
    }))
  }
}

and

➜  snips scalac -Wnonunit-statement -Wunused:all -Vprint:typer -d /tmp/sandbox for-unused.scala
[[syntax trees at end of                     typer]] // for-unused.scala
package <empty> {
  class C() extends Object() {
    def i: Int = 42
    def f(xs: List[Int]): List[Int] =
      xs.map[(Int, Int)](
        {
          def $anonfun(x: Int): (Int, Int) =
            {
              val y: Int = this.i
              Tuple2.apply[Int, Int](x, y)
            }
          closure($anonfun)
        }
      ).map[Int](
        {
          def $anonfun(x$1: (Int, Int)): Int =
            x$1:(x$1 : (Int, Int)) @unchecked match
              {
                case Tuple2.unapply[Int, Int](x @ _, y @ _) => x:Int
              }
          closure($anonfun)
        }
      )
  }
}

That is for

class C {
  def i = 42
  def f(xs: List[Int]) =
    for (x <- xs; y = i) yield x
}

where the y is consumed by the tupling for "midstream assignment".

To notice it is unused, you must fast-forward to the map where the pattern var is unused; but it may be marked synthetic? I no longer remember why it is not marked unused.

The scala 2 umbrella ticket (for improved diagnostics of for comprehensions) is scala/bug#10287.

When the "improved for comprehension" PR is merged, we'll need "improved diagnostics for improved for comprehensions".