Scala: Improving the `@nowarn` annotation by taking an additional optional parameter: the Scala version(s) that must be silenced

Hi everyone :slightly_smiling_face:

I was migrating zio-prelude from Silencer to @nowarn recently (See Give some love to the project by guizmaii 路 Pull Request #1175 路 zio/zio-prelude 路 GitHub) and I noticed that the @nowarn was itself emitting a warning:

@nowarn annotation does not suppress any warnings

The issue is that some of the silenced warnings are only emitted by some Scala versions, which makes the compilation emit this @nowarn annotation does not suppress any warnings warning when the code compiles with the other Scala versions.

This makes maintaining cross-Scala versions projects more difficult.

In the end, the solution I found was to change these warnings into info level messages
(See Give some love to the project by guizmaii 路 Pull Request #1175 路 zio/zio-prelude 路 GitHub)

That鈥檇 be less confusing for the other maintainers if we could just specify on which version of Scala we expect these warnings to be emitted.

Something like:

@nowarn("msg=...", Some(List("2.12.x")))
def emittingWarningOnScala212Function = ...

So that if we see a @nowarn annotation does not suppress any warnings warning being emitted by Scalac, we know we actually can remove this @nowarn

Please let me know what you think :slightly_smiling_face:
Jules

I think (without much empirical evidence) that the build is the best point to intervene in the natural order of things.

Usually, we prefer local solutions, such as keeping unit tests close to the code under test, because the person writing the code has the most knowledge about it.

If the code is known to be 鈥渄irty鈥, then annotating it @nowarn spares other people from that terrible knowledge.

But the coder doesn鈥檛 really know about versioning constraints, let alone future versions.

If someone comes along and bumps a version in the build, and suddenly there are warnings, but it鈥檚 determined that the warnings are 鈥渆xpected鈥, which is to say, 鈥渂enign鈥, then they should managed at the point of change, in the build.

I see the code has nowarn annotations for unused imports, for example. That might be a good candidate for decluttering the code in favor of sweeping it under the rug of build configuration.

The unused imports can be selected by src file, site of the enclosing element, and origin to mean the import 鈥渟elector鈥 that causes the warning. For example, for import collection.{immutable, mutable}, you might specify origin=scala.collection.mutable. (In the following, origin is a regex so the star is glob, not the import syntax. origin=scala.collection\._ matches collection._ and collection.*.)

鉃  scalac -d /tmp -Xsource:3 -Xlint "-Wconf:cat=unused-imports&site=example.C:e,cat=unused-imports&origin=scala.collection\..*&src=unused-import.scala:i" unused-import.scala
unused-import.scala:7: error: Unused import
  import concurrent.*
                    ^
1 error

to distinguish these imports

package example

import collection.*

class C {
  import concurrent.*
  def p() = println("hello, world")
}

The same considerations apply to nowarn.

There is precedent for versioning because deprecations know when they were deprecated. For usages of this API:

class D {
  @deprecated("old stuff", since="0.1")
  def d() = println("old stuff")
  @deprecated("newer stuff", since="Dlib 0.2")
  def e() = println("newer stuff")
}

the deprecations can be selected by version with a simply clever syntax:

"-Wconf:cat=deprecation&origin=example\.D\..*&since>0.1:s"

The origin is a regex, so literal dot is escaped, and the version string is snipped from the since string.

Then similar syntax could be adopted for nowarn:

"-Wconf:cat=unused-nowarn&origin=example\.D\..*&since>2.12:s"

But this would only control for versions of the Scala compiler and libraries, which is a narrow restriction.

For example, suppose D is amended in Dlib 1.0 with a better signature:

  def f(): this.type = this
  //def f(): D = this

where the old API warns under -Wnonunit-statement for a usage def run(): Unit = d.f().

Currently, the build (which knows about versions) does not communicate with Wconf. If it did, I could -Wconf:cat=unused-nowarn&since<Dlib 1.0 or similarly for cat=w-flag-value-discard.

In sum, I am suspicious of sprinkling warning suppression everywhere, without an ability to audit what was suppressed. The purpose of inline @nowarn is not to say, 鈥淣othing to see here,鈥 but to warn that something is so terrible that you don鈥檛 want to know. (I do like it for special usages, such as tests of deprecated API, where suppression is locally known to be justified.)

The purpose of -Wconf is to externalize versioning concerns about warnings, especially under -Werror.

It鈥檚 nice to avoid cracking open a source file whenever messaging changes due to unrelated causes.

I also found @SuppressWarnings an annoyance, since it warned variously depending on what tool was processing the annotation.

It鈥檚 worth mentioning the dotty project guidance of minimizing annotations such as @tailrec, which is clutter unless there is a practical danger of stack overflow.

This post is just an excuse for me to learn more about how -Wconf works (in Scala 2); its length should not be taken to mean I鈥檝e given the topic a lot of thought.

There could be other factors than the Scala version, like the JDK version. I guess I recommend to disable that warning (with -Wunused:-nowarn,_) and check them manually from time to time. Also narrow down the @nowarn by using a specific filter.

Hi mates
Thanks for answering :slightly_smiling_face:

I鈥檓 sorry, I鈥檓 not really convinced by the -Wconf + regex approach.

First reason: regex. Regexes are always a mess. They鈥檙e complicated to make, to maintain and to understand.

Second reason: I can only see this -Wconf:<config line> config line become so complex no one understands what it does nor how it works. Which will lead to misconfigurations and silenced warnings that shouldn鈥檛 have been silenced.

As you said, it鈥檚 better to keep the code where the knowledge is.
In this case, the knowledge of the issue is in the source code: in this particular file, for this particular version of Scala, we know that this particular line of code generates a warning we want to ignore.

The -WConf solution for the problem I鈥檓 reporting feels to me like getting a bazooka to kill a fly.

  • It鈥檚 a 鈥渇ly鈥 because the issue is very localized: we want to silence this precise warning on this precise line of code for this precise version of Scala (could be let 鈥渘ot so precise鈥 by re-using sbt syntax: "2.12.x", silencing the warning for all the Scala 2.12 versions)
  • It鈥檚 a 鈥渂azooka鈥 because you configure something globally for your project just to remove this precise warning on this precise line of code for this precise version of Scala.

IMO, -WConf should be kept for 鈥済lobal issues鈥 of the project: issues that we want to silence everywhere in the project: 鈥渋f this warning happens anywhere, I want to ignore it鈥 (a dangerous thing to do IMO but can be justified in some cases, I guess)

Let me conclude with my own experience with -WConf.
I tried to configure it in some of my projects. I mostly failed and abandoned.
I think it鈥檚 complex and not so well documented (there鈥檚 no msg=regex example for example. How should we write this regex? Do we need to surround it with "? See Configuring and suppressing warnings in Scala | The Scala Programming Language)
It could be nice if Scala and/or sbt was providing a better way than just this textual config line to configure this -WConf flag.

Well, let鈥檚 start by fixing this first issue. We could worry about the JDK version later if anyone feels like it鈥檚 an issue.

Which filter are you talking about?

By 鈥渟pecific filter鈥, I assume Lukas meant @nowarn("cat=deprecation") or as specific as the stringly condition can be constructed. That lowers the chance of suppressing the wrong warning, but if @nowarn does not warn at all, then a narrow filter doesn鈥檛 help that.

I agree the stringly notation is unwieldly, if powerful. Or, it鈥檚 an opportunity for a budding consultancy.

I don鈥檛 think anyone considered something structured. (@nowarn is a StaticAnnotation but need not be, perhaps.)

@nowarn(Deprecation & since"1.0" | msg"missing").

I need to return to a PR for linting 鈥渦nused @unused鈥 from last year. I don鈥檛 remember the challenges, but I commented:

That is also why I fixed the ordering of ScalaVersion so that 2.13.11-development < 2.13.11.

I didn鈥檛 add versioning to the config DSL, but probably I was about to.

For some reason, I don鈥檛 like adding a parameter to the annotation, but I like adding versions to the config. Perhaps the config honors both since and version.

@nowarn(version"MyLib 1.0" & version"JDK 21" | since"Akka 2.5" | since"Scala 2.12.x")

If the version is part of the config, then either the annotation or the -Wconf option can supply it.

I assume (or guess) that the danger of suppressing the wrong message is why Odersky et al resisted local suppression for so long. So I think mechanical verification is important. You should be able to detect annotations that suppress too little and those that suppress too much. Maybe it should produce an audit which can be checked at CI as well as by hand.

Hi, I have encountered a similar problem when trying to cross-compile a project for both Scala 2 and Scala 3.

I would like to suppress a value discard warning but the filter differs:

@nowarn("cat=w-flag-value-discard") // Scala 2
@nowarn("name=ValueDiscarding")     // Scala 3

And of course, I cannot simply use both of the above because then I get a compilation error

[error] Invalid message filter:
[error] unknown filter: name=ValueDiscarding
[error]   @nowarn("name=ValueDiscarding")
[error]    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

(for Scala 2, and for Scala 3 it鈥檚 the same with the other annotation)

I guess in my case I can solve it by just using nowarn without a filter, but it feels suboptimal鈥