Unable to discard non-unit value using `: Unit`

[WojciechMazur]

trafficstars

Scala 2 allows to discard non-unit value using : Unit, however this behaviour seems to be missing in Scala 3.

Compiler version

All Scala 3 versions

Minimized code

//> using options -Wvalue-discard -Wnonunit-statement

object Tests {
  class Assertion(assert: => Any){
    def shouldPass(): Assertion = ???
  }
  def Test: Assertion = {
    new Assertion("").shouldPass(): Unit
    new Assertion("another").shouldPass()
  }
}

Output

[warn] ./test.scala:8:5
[warn] discarded non-Unit value of type Tests.Assertion
[warn]     new Assertion("").shouldPass(): Unit
[warn]     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expectation

Should ignore non-uni statements with explicit :Unit When : Unit is missing compiler should hint how to silence the warning similarly as it's done in Scala 2

Compiling project (Scala 2.13.14, JVM (17))
[warn] ./test.scala:9:5
[warn] unused value of type Tests.Assertion (add `: Unit` to discard silently)
[warn]     new Assertion("").shouldPass()
[warn]     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[dwijnand]

Scala 2 PR: https://github.com/scala/scala/pull/7563

[SethTisue]

@som-snytt in 2.13.15, iirc you removed the "(add : Unit to discard silently)" advice that 2.13.14 used to give. if this gets fixed in Scala 3 (I hope it does), should we add that advice back?

[som-snytt]

I consider that "warn allowance" to be a relic of pre-nowarn, pre-Wconf years.

In particular, it bestows extra semantics on ordinary syntax used in a weird way.

[SethTisue]

hmm.

: Unit does have the advantage of being short and only involving ordinary syntax (even if arguably in a “weird way”)

the design discussion at https://github.com/scala/scala/pull/7563 is worth reviewing. Adriaan made some pretty persuasive arguments in favor of : Unit

bare@nowarn is not great because then I'm suppressing all warnings, not just the warning I had in mind

and writing out a more targeted @nowarn expression is the sort of thing that sends pretty much all of us running off to consult reference materials to figure out how to do it. whereas : Unit, I can remember.

so my initial reaction is that #7563 wasn't just a mistake or an artifact of @nowarn not existing yet. but perhaps I should do a fresh round of pondering.

[som-snytt]

I wonder if @discard is a better path. Annotations are a last resort, but are tool-aligned.

Worth adding that we use phrases "ascription" and "annotation" interchangeably, which suggests these are the same use case, but just different syntaxes.

[dwijnand]

I think I recently found out that you can have multiple val _ = .. in scala 3, and they work - which don't work in Scala 2. So that's another fairly handy way to expressly discard a non-unit value.

[som-snytt]

@dwijnand that works in Scala 2. I think in Scala 3 it is ignored as local store, but in Scala 2 introduces a field (result is retained).

class C {
  val _ = 42
  val _ = 26
}

in Scala 2

under.scala:3: warning: Pattern definition introduces Unit-valued member of C; consider wrapping it in `locally { ... }`.
  val _ = 42
      ^
under.scala:4: warning: Pattern definition introduces Unit-valued member of C; consider wrapping it in `locally { ... }`.
  val _ = 26
      ^
2 warnings

[dwijnand]

I remember some "already named _" errors, but who knows. Maybe they're 2.12 or something.

[som-snytt]

Yes, it was "improved". But I sympathize with your cognitive dissonance. You have led so many Scala lives.

[mbovel]

If we decide to implement this, it seems like it could be a nice Spree-sized issue.

What is needed to take decision? Should this be further discussed here, or during a compiler or core meeting?

[dwijnand]

It seems small and innocent and has been a part of the Scala 2 language for a while, so unless @odersky come around and strongly argues against this before the next spree, I think we can go ahead and propose a PR for this.

[odersky]

I am not sure what exactly is proposed here.

[mbovel]

We discussed this today during the compiler meeting and decided we should implement this. Let's tackle it during the next Spree 🥳

[mbovel]

This issue was picked for the Scala Issue Spree of tomorrow, Monday, October 21st. @rochala and @nmcb will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[som-snytt]

Sorry I have no insight, but "if it makes Seth happy, it can't be that baaad."

[mbovel]

This issue was picked again for the Scala Issue Spree of Monday, November 11th. @mbovel and @nmcb will continue working on it.

[goshacodes]

This is not only Seth hapiness :) E.g. we use scalapb which generates code with warnings and we would be happy to disable it on grpc module

Sorry I have no insight, but "if it makes Seth happy, it can't be that baaad."

[mbovel]

Different warnings:

object Tests {
  class Assertion(assert: => Any){
    def shouldPass(): Assertion = ???
  }
  def Test: Unit = {
    1 + 1: Unit // pt: Unit, warning: Discarded non-Unit value of type Int. You may want to use `()`.
    val x: Unit = 1 + 1 // pt: Unit, warning: Discarded non-Unit value of type Int. You may want to use `()`.
    1 + 1 // pt: Wildcard, warning: unused value of type (2 : Int)
    val y: Int = 1 + 1 // pt: Int, no warning

    new Assertion("").shouldPass(): Unit // pt: Unit, warning only with -Wvalue-discard: discarded non-Unit value of type Tests.Assertion
    val x1: Unit = new Assertion("another").shouldPass() // pt: Unit, warning only with -Wvalue-discard: discarded non-Unit value of type Tests.Assertion
    new Assertion("yet another").shouldPass() // pt: Wildcard, warning only with -Wnonunit-statement: unused value of type Tests.Assertion
    val y1: Assertion = new Assertion("another other").shouldPass() // pt: Assertion, no warning

    ()
  }
}

We want to remove warnings for both cases with : Unit.