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 https://github.com/zio/zio-prelude/pull/1175) 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’d 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

1 Like

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 “dirty”, then annotating it @nowarn spares other people from that terrible knowledge.

But the coder doesn’t 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’s determined that the warnings are “expected”, which is to say, “benign”, 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 “selector” 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, “Nothing to see here,” but to warn that something is so terrible that you don’t 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’s 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’s 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’ve 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’m sorry, I’m not really convinced by the -Wconf + regex approach.

First reason: regex. Regexes are always a mess. They’re 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’t have been silenced.

As you said, it’s 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’m reporting feels to me like getting a bazooka to kill a fly.

  • It’s a “fly” 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 “not so precise” by re-using sbt syntax: "2.12.x", silencing the warning for all the Scala 2.12 versions)
  • It’s a “bazooka” 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 “global issues” of the project: issues that we want to silence everywhere in the project: “if 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’s complex and not so well documented (there’s 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’s start by fixing this first issue. We could worry about the JDK version later if anyone feels like it’s an issue.

Which filter are you talking about?

By “specific 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’t help that.

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

I don’t 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 “unused @unused” from last year. I don’t 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’t add versioning to the config DSL, but probably I was about to.

For some reason, I don’t 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’s 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…

Today I made a typo in the filter category.
The error message wrote this:

Invalid message filter
unknown category: deprecated
@nowarn("cat=deprecated")

Also, it would be nice to mention the categories in scala.annotation.nowarn ScalaDoc.

1 Like

Unfortunately, like Java’s SuppressWarning, categories are compiler-specific. There is special verbiage to request Java compilers to cooperate about what can be suppressed.

It should definitely say, did you mean deprecation?. On the basis of the prefix, if not the edit distance. It’s just a name; other parts of the string are regexes, etc.