Support -Wconf and @nowarn

[lrytz]

This PR adds a -Wconf compiler flag that allows filtering and configuring compiler warnings (silence them, or turn them into errors).

It also integrates the fantastic silencer compiler plugin by @ghik into the compiler, which allows suppressing warnings locally using the @nowarn annotation.

-Wconf and @nowarn work largely the same way as in Scala 2, but most filters for selecting warnings are different. The output of -Wconf:help gives more details:

Syntax: -Wconf:<filters>:<action>,<filters>:<action>,...
multiple <filters> are combined with &, i.e., <filter>&...&<filter>

<filter>
  - Any message: any

  - Message categories: cat=deprecation, cat=feature, cat=unchecked

  - Message content: msg=regex
    The regex need only match some part of the message, not all of it.

  - Message id: id=E129
    The message id is printed with the warning.

  - Message name: name=PureExpressionInStatementPosition
    The message name is printed with the warning in verbose warning mode.

In verbose warning mode the compiler prints matching filters for warnings.
Verbose mode can be enabled globally using `-Wconf:any:verbose`, or locally
using the @nowarn annotation (example: `@nowarn("v") def test = try 1`).

<action>
  - error / e
  - warning / w
  - verbose / v (emit warning, show additional help for writing `-Wconf` filters)
  - info / i    (infos are not counted as warnings and not affected by `-Werror`)
  - silent / s

The default configuration is empty.

User-defined configurations are added to the left. The leftmost rule matching
a warning message defines the action.

Examples:
  - change every warning into an error: -Wconf:any:error
  - silence deprecations: -Wconf:cat=deprecation:s

Note that Scala 2 supports many other filters. This PR delivers a good starting point for Scala 3 and we should wait and see if users actually want other filters.

Co-authored by @som-snytt.

Fixes #8908

Other changes in this PR

• Add the -Wunused flag, for now only with -Wunused:nowarn (and -Wunused:all). I assume that (long-term) migrating the other unused warnings from Scala 2 is desired.
• The -unchecked flag was previously unused, unchecked warnings were reported as normal warnings. Now unchecked warnings are reported as such, and -unchecked is enabled by default (as in Scala 2).
• In neg compilation tests, warnings are now emitted and represented in .check files.

[lrytz]

OK, this is finally green. @smarter would you mind looking over this first draft? The diff is not that big.

Feature suggestions are very welcome. What I saw actually in use in Scala 2:

• filter by warning site (the fully qualified path to the location where the warning is emitted). this would requrie going through the warning(..) calls in the compiler and add the site parameter (in phases that have a typer we can probably take it from the context).
• filter by more categories (currently, this PR has only has deprecations and feature warnings). this would also require adjusting all warning(..) calls in the compiler to add the category.
• filter deprecations by origin, i.e. fully qualified path to the deprecated symbol

I'll add more tests, but first would like to know: how can I test compiler warnings?

[smarter]

I had a quick look but I don't think I'll have the time to review this thoroughly.

Feature suggestions are very welcome.

I think it'd be great to be able to refer to messages by their name in https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala (minus the ID suffix) to have a stable identifier for them instead of having to use regexps.

how can I test compiler warnings?

With a .check file in a neg test, though that requires at least one error in the test or -Xfatal-warnings to be enabled (like in tests/neg-custom-args/fatal-warnings)

[som-snytt]

Someone's twitter joke made me think -Werror:deprecation,unchecked,lint-infer-any might be helpful simplification.

It encourages boosting warnings to errors instead of suppressing warnings.

Now I wonder if -Wconf could be -Werror with default any:error, filter type default to cat= and action default to :error.

[som-snytt]

Also worth advertising -Wconf:cat=lint-nullary-unit&site=.*Test:s, I tried and failed on twitter, which I don't know how to use.

[lrytz]

I think it'd be great to be able to refer to messages by their name in https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala (minus the ID suffix) to have a stable identifier for them instead of having to use regexps.

I added filtering by message ID (id=E129), not the error ID's enum case name. The problem with the name is: how can people figure out, given a warning message, what's the ID's name? It doesn't show up anywhere:

-- [E129] Potential Issue Warning: Test.scala:6:12 -----------------------------
6 |  def h = { 1; 3 }
  |            ^
  |A pure expression does nothing in statement position; you may be omitting necessary parentheses

We could add a -Vwarnings compiler flag to include the ID's name in the message, but it's quite annoying in the workflow having to go change the project's build definition just for that... Better ideas? Maybe we could reuse the @nowarn annotation, something like

@nowarn("VERBOSE PLS") def h = { 1; 3 }

Using the error ID works, but it's not great because reading the config later (@nowarn("id=E129")) you don't see what warning is being silenced.

[lrytz]

Pushed a few more changes.

In "verbose" mode warnings are printed with a list of matching message filters that can be used in -Wconf and/or @nowarn. Example:

-- [E000] Syntax Warning: Test.scala:5:12 --------------------------------------
5 |  def bar = try 1
  |            ^^^^^
  |            A try without catch or finally is equivalent to putting
  |            its body in a block; no exceptions are handled.
Matching filters for @nowarn or -Wconf:
  - id=E0
  - name=EmptyCatchOrFinallyBlock

longer explanation available when compiling with `-explain`

Verbose mode can be enabled in -Wconf, for example for all warnings using -Wconf:any:v.

However, changing compiler flags is slow. To make the workflow more convenient verbose mode can be enabled by (temporarily) annotating the surrounding code with @nowarn("verbose") or @nowarn("v"):

  @nowarn("verbose")
  def bar = try 1

[lrytz]

Also added filtering by the warning's compiler-internal ErrorMessageID name (which is displayed in verbose mode):

@nowarn("name=PureExpressionInStatementPosition") def t4b = { 1; 2 }

[smarter]

Very nice solution! Seems like if we support name=... we could even drop support for id=... since the latter would be much more cryptic, but we should document at the top of ErrorMessageID that the names shouldn't be changed.

[lrytz]

we could even drop support for id=...

Some people might like it as it's more compact, but I'm happy to remove it and see if anyone asks for it.

@nowarn("verbose") def bar = try 1

The same idea would actually work very well as an alternative to the -explain compiler flag. But using @nowarn for that would be confusing, so it would probably need a new annotation.

I vaguely remember @odersky criticizing the overhead of the -explain feature (cannot find it however). I don't remember if it was about the feature as such, or about the fact that having to change a compiler flag is a big overhead.

[lrytz]

Added a warning for @nowarn annotations that don't silence any warnings. This is behind -Wunused:nowarn (-Wunused is new for Scala 3).

[soronpo]

Added a warning for @nowarn annotations that don't silence any warnings. This is behind -Wunused:nowarn (-Wunused is new for Scala 3).

I'm not sure this is a good idea. If users have a common codebase for Scala 2 and 3 with a false warning they want to remove in Scala 2 that does not exist in Scala 3, then this makes everything more difficult for them. @nowarn is often used for workaround compiler issues that may change through versions.

[lrytz]

That's why it's not enabled by default, only with -Wunused:nowarn

[lrytz]

Pushed a few more tests. I'm done with this PR from my side, not planning to push more changes until reviewers chime in.

[lrytz]

(updated the PR description)

[bishabosha]

can this be merged?

[prolativ]

I think we can merge this unless @smarter or @som-snytt have any remarks

[som-snytt]

Sorry I haven't had a chance to dig into -Wconf 3.0. I may have to wait for the Coursera on error messages. I will definitely comment after it lands in my local dotty repo.

[prolativ]

@lrytz as this PR deserved the release-notes label, according to our new release strategy and what we have discussed during the Dotty meeting, you (as the author of the PR) should now be responsible for providing a short description of the changes in a form that could be used as a part of the blog post for the next release (the idea is that the author of the changes should have better expertise in the issue than the person doing the release and this should make the release process smoother). Would you be so kind to write this description? It doesn't have to be very long or go very much into details. You can have a look at the previous blog posts for some inspiration

[lrytz]

Sure, I updated the PR description. The first 3 sentences can go into the release notes.

[prolativ]

Thanks @lrytz
@som-snytt I'm not sure if I understood correctly if you're going to add some remarks in the nearest future or not. The deadline for merging PRs targeted for 3.1.0 is this Friday so we should make this in by then IMHO

[som-snytt]

I won't have time for further input now. I'll try to contribute on an ongoing basis to support this important feature. Thanks!

[prolativ]

@som-snytt what feature do you mean? Anyway, if this is just a feature, wouldn't it make sense to merge the current solution and introduce any improvements later?

[lrytz]

@prolativ yes, that's what he means.

[prolativ]

@lrytz it would be nice if you could squash your commits before merging (preferably into one unless there's a good reason to have a few)

[lrytz]

Sure, I can, but I thought this was usually not done in this repo?

[prolativ]

As far as I can see this is not strictly enforced to have 1 commit per PR but reasonable squashing makes git history easier to read. At the same time we would like to preserve merge commits to manage the release process more easily

[dwijnand]

❤️

[kubukoz]

Is there any reason why the src filter isn't supported (other than nobody asking for it yet :) )? I've been using that on 2.x to ignore warnings (e.g. deprecations) from generated code.

edit: made a feature request for this: #17634

[julienrf]

@lrytz, would you be available to also update the migration guide to mention that -Wconf has been introduced in Scala 3.1.0?