warnUnused is reporting on Assertions about sbt-tpolecat HOT 17 CLOSED

What @henricook suggests is a totally fine way to go about disabling bits of sbt-tpolecat, although you can also use the settings DSL to do this now if you just want a one-liner, e.g.

Test / tpolecatExcludeOptions += ScalacOptions.warnNonUnitStatement

from sbt-tpolecat.

For anyone else in the same boat who wants to disable the -Wnonunit-statement flag that sbt-tpolecat adds:

scalacOptions ~= { options: Seq[String] =>
  options.filterNot(
    Set(
      "-Wnonunit-statement"
    )
  )
}

The Set is overkill but this is a scaffold I use to remove 1 or more flags. Sometimes I remove a flag based on an environment variable, like turning off -Wunused:imports while I'm prototyping with a PROTOTYPE=true env var.

You could maybe try and only do this for your Test profiles to see if your main code body can benefit from it. In my case we have some places where we had to use Java libraries without Scala wrappers that mean this flag causes more hassle than its worth.

from sbt-tpolecat.

Hmm thanks for the report - I think this is due to the addition of -Wnonunit-statement in #115 (which I seem to have missed in the release notes, sorry about that). I think this is because Scalatest assertions return a value of type Assertion as well as throwing an exception on failure, which unfortunately triggers this warning because the value is not usually used.
I think a lot of users were eager to see this setting added so I'm not so sure what to do here!

from sbt-tpolecat.

Do people write FP tests/test assertions?

It's not that crazy, something like this:

for {
  foo <- doFoo()
  _ <- IO(assert(...))
  _ <- IO(assert(...))
} yield ()

from sbt-tpolecat.

I also had this issue and the warning in the Readme was useful, thanks for that.

However, I was still having an extra issue with IntelliJ. I guess the IntelliJ SBT plugin does not parse the Test / scalacOptions separate from the standard scalacOptions, so I had to suppress warnings for the test files, like this:

scalacOptions ++= Seq(
  "-Wconf:cat=other-pure-statement:src=test/.*.scala:silent"
)

More about warning suppression here: https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html

No need to comment or fix anything else. Just jotting this down in case anyone else runs into it.

from sbt-tpolecat.

Do people write FP tests/test assertions? 😅 I'm probably just going to find a way to override/disable this flag if I can rather than approach the new set of 500+ errors in the Test / portion of my project

from sbt-tpolecat.

I've added a (perhaps slightly too prominent) warning about this to the README now - so let's close this :D

from sbt-tpolecat.

I think -Wnonunit-statement really only makes sense for pure FP code. Given that this is sbt-tpolecat, that's a reasonable default? 😅

from sbt-tpolecat.

Reporting the same here. We have multiple test cases with multiple assertions in it, so the change in #115 is hurting us quite bad :) Since we operate with a lot of smaller projects, upgrading the plugin and duplicating the workaround @henricook provided is not ideal.

Not sure what would be the desired behaviour from sbt-tpolecat point of view...

edit: the suggestion below by armanbilge is not really where we want to go - as we use both FP and OOP style :)

from sbt-tpolecat.

Btw, @som-snytt gave some advice in the Scala Discord, about how to silence this warning for specific classes.

that seems to be cat=other-pure-statement, where site= the enclosing definition. You would have match the name of a class that goes unused in the msg=MyClass.
As I just did, scalac -Wconf:help
-Wconf:cat=other&msg=MyClass&site=MyClient:s where s for silent. IIRC.

from sbt-tpolecat.

I will make improvements for 2.13.11, so let me know if there is a useful heuristic here.

It is certainly reasonable to make checks in Test less strict, but also "tests are code," etc.

from sbt-tpolecat.

Do people write FP tests/test assertions?

Yes, with libs like munit-cats-effect.

from sbt-tpolecat.

Do people write FP tests/test assertions?

It's not that crazy, something like this:

for {
  foo <- doFoo()
  _ <- IO(assert(...))
  _ <- IO(assert(...))
} yield ()

With ScalaTest async testing + CE they often look like this:

val assertion = for {
  _ <- doSomething
  result <- doSomethingElse
} yield result shouldBe someKnownValue

assertion.unsafeToFuture()

see e.g. https://github.com/hmrc/transit-movements-guarantee-balance/blob/main/test/services/BalanceRequestCacheServiceSpec.scala#L140

from sbt-tpolecat.

FYI, this is also triggering for Spec2/Scala 2.13 unit tests such as:

package com.foo.app

import org.specs2.mutable.Specification

class UDFsSpec extends Specification {

  import UDFs._

  "concatDiscardingEmpty" >> {
    "ignore empty Strings and nulls" >> {
      val expected = "A:B:C"
      val result   = concatDiscardingEmpty(":", List("A", null, "B", "", "C "))
      result ==== expected
    }
  }
}

I get:

[error] ...UDFsSpec.scala:8:27: unused value of type org.specs2.specification.core.Fragment (add `: Unit` to discard silently)
[error]   "concatDiscardingEmpty" >> {
[error]                           ^
[error] two errors found

from sbt-tpolecat.

I took a look, and foo must be(bar) in ScalaTest results in an Assertion object, so there's nothing to be done on the diagnostic side.

If you want to use ScalaTest this way, write your "block of asserts" as a tuple of expressions:

  "Domain" should "have a name" in (
    domain.status must be(true),
    domain.name must be("testing"),
  )

or use -Wconf:msg=unused value of type org.scalatest.Assertion:s or turn off -Wnonunit-statement in Test. (or ask sbt-tpolecat to do that for you)

from sbt-tpolecat.

scalacOptions ++= Seq(
  "-Wconf:cat=other-pure-statement:src=test/.*.scala:silent"
)

This is a great fix for Intellij/Scala2 but appears be unworkable in Scala3 currently. Possibly we'll get more information in this issue: scala/scala3#18804

from sbt-tpolecat.

I commented on the issue. I didn't try anything out, but if it's fixed on 3.4.1, you can lobby for backport to LTS, which stands for Lobbied something.

from sbt-tpolecat.