Strict mode for value discard warning

[som-snytt]

Disable dispensations under -Ywarn-value-discard-strict.

[dwijnand]

Continuing from your comments in #7563 (comment):

I'll explore whether -Ywarn-value-discard:strict would help by turning off the escape hatches.
(...)
Obviously, this trick is in lieu of SupressWarnings.

It's not a trick. There's no opting-out of value discarding in Scala. The best you can do is get warnings (and then choose if you want any warning to be fatal). However then you have to deal with the cases when you do want to discard a value. In that case the desire is (which started in #7530 and then #7560) to be explicit about wanting to discard the value.

I don't think we should add a flag to disable how one opts out, even if it's an experimental -Y flag.

Write a SIP to disable value discarding entirely, now that's an effort I would assist you with.

[som-snytt]

@dwijnand It's a trick, I tell you. You can tell it's a trick because of the bug noted on the other ticket, where Future[Unit] still warns because the tree attachment isn't propagated.

To make sure we're on the same page, we're still talking about enabling warnings, not forked code semantics.

Anyway, the context is folks using Akka with Future[Unit] and are liable to be confused about the value named Unit when their best friend hasn't been merged and is growing old in PR jail. (Or do you say gaol?)

Were I they, I wouldn't want to wonder if some weirdness is because I wrote something a certain way for some reason I don't know what, and now it doesn't work.

In this question, the accepted answer had it wrong until help came along. Similarly for Future.successful(Unit).

In this context, it's simplest to just give them a tool that removes all doubt -- see the doubts expressed on the other ticket. It's clear that the Future[Unit] thing has bitten a lot of people over many years. The worst thing we can do is send them to stackoverflow.

[dwijnand]

Before this is merged I'd like if it was clear what the intended goal is with introducing this. Particularly I'm wondering when it's expected this flag will be used, because I think it'll be so loud and noisy it'll at best only useful for gists and stackoverflow snippets, but won't ever be of use in a sizeable codebase. Are gists and stackoverflow snippets acceptable use cases for introducing a new experimental -Y flag?

If type ascribing Unit is confusing, then I'd rather replace #7563 with a better way to suppress value discarding warnings, perhaps going back to Unit.void[A](x: A): Unit idea in #7530. Rather than have a strict mode that overrides express intent to suppress...

If ()/Unit/Unit/Unit.type is confusing, let's fix that. Perhaps by making value Unit unuseable (your struggling PR, #7698). Perhaps by making value Unit synonymous to (). Perhaps by throwing Unit away and just making it () for both value and type.

[maphi]

Unit.void or maybe Unit.apply and similar ways sound like a good option. I think one can expect folks to know it when they go and turn the discard value warnings on.
Besides that a SIP to remove value discarding would be great.

Regarding the noisiness: Depends how far the project grew. It's always a tradeoff to enable it afterwards or not but when used from beginning it may prevent a lot of errors. It's definitely worth it.

Discarded values often point to errors. At work we use wartremover's NonUnitStatement to help our newcomers: (i know this is about statements not value discarding at end of block)

def getAndIncDbCounter: Future[Unit] = {
  val counterValue: Future[Long] = someDbCall
  //Future[Future[Unit]]; no error handling; concurrent calls to getAndIncDbCounter
  counterValue.map(cnt => updateCntInDb(cnt+1))
  counterValue
}

(similar to real code written by a programmer unexperienced with scala)

[dwijnand]

Even from the get go I don't think this option would survive because it means noise for each and every this-returning and non-unit returning method, with no cheap way to disable the warning.

[som-snytt]

WIPing it until the concerns are sorted. I see it as just a way to turn warnings up or down; I understand Dale to be saying what's the point of turning it up if it introduces noise and reduces effectiveness.

The orthogonal concerns include whether value discard is a good feature, and whether warnings can be filtered.

Separately, I'll take a shot at filtering. That would not obviate this PR, maybe allow users to turn warnings up or down and then filter on the result.

[maphi]

Filtering sound interesting. Besides that i like the strict mode. One can opt in and out with the preferred level of warnings.

I first thought when #7563 gets released in 2.13.0 the behaviour can't be changed for 2.13.x. But adding strict mode later on would be possible, right?