Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable and suppressable warnings #333

Closed
adriaanm opened this issue Mar 8, 2017 · 9 comments
Closed

Configurable and suppressable warnings #333

adriaanm opened this issue Mar 8, 2017 · 9 comments

Comments

@adriaanm
Copy link
Member

@adriaanm adriaanm commented Mar 8, 2017

Motivation

Broken Windows Theory: clean codebases that keep warnings under control encourage to keep the codebase clean.

-Werror can often not be enabled, for example due to non-avoidable deprecation warnings when cross-building.

This proposal has two goals: make warnings globally configurable using compiler flags, and allow suppressing warnings locally.

What we have today

  • @unchecked in pattern matching
    • on scrutinee: no exhaustivity check
    • in type patterns: no warning about unchecked erased types
  • @unused: omit -Ywarn-unused / -Xlint:unused warnings for annotated declarations (added in scala/scala#7623, discussion at scala/bug#10790)
  • @migration
    • with -Xmigration:<fromScalaVersion>, show warnings that have a version bigger than fromScalaVersion
    • not for everyone to use, the annotation is private[scala]

Considerations for new functionality

Global Configuration: allow users to configure what is reported as error, warning, info, or not at all.

  • Generalization of -Werror, which escalates all warnings
  • -Wconfig was proposed in scala/scala#7790 (comment)
    • only for configuring the propsed @restricted warnings
  • deprecations
    • configure by since version -- for cross-building, upgrading (adriaan has a WIP, see below)
    • difficulty: since is not only used in standard library, each library has its own version numbers
    • scaladoc of deprecated says "Library authors should prepend the name of their library to the version number"
  • lint, other flags in -W
    • configure by category
  • Compiler warnings
    • configure by regex match?
  • Compiler errors: are there any that can be downgraded to warnings?

Managing many errors/warnings

  • -Xmaxerrs, -Xmaxwarns
  • Summarizing
    • -deprecation and -feature to show all warnings, one-line-summary otherwise
    • scala/bug#8829 suggest showing the warnings by default
    • perhaps the new config mechanism will subsume the two flags
    • note: the summary is a also a warning, so -Werror still breaks scala/bug#8829
  • Grouping: deprecations, maybe others
    • Show message once on first occurence, add a suffix ("x more occurences", maybe more details)

Local suppression

  • support an annotation like @SuppressWarnings, @silent (see below)
  • other tools (scalafix) should be able to use the same syntax?

Links, Discussions, WIPs

Existing Tools

Java: @java.lang.SuppressWarnings

Silencer https://github.com/ghik/silencer

  • @silent for scoped suppression
  • @silent("deprecated") for message pattern, regular expresison
  • compiler flag to error if nothing is silenced (good idea!)
  • compiler flag for global filters (regexes)
  • compiler flag for file name filters (regexes, unix-style paths, all warnings in those files are silenced)

Wartremover http://www.wartremover.org/

  • Configuration in sbt: wartremoverErrors ++= Warts.unsafe, wartremoverWarnings ++= ...
  • Local Suppression
    • @SuppressWarnings(Array("org.wartremover.warts.Var", "org.wartremover.warts.Null")) on declaration
    • wartremoverExcluded += baseDirectory.value / "src" / "main" / "scala" / "SomeFile.scala" in sbt

Related Topics

Downstream impact

  • IDEs, support local suppression (e.g., for deprecations)
  • sbt

Better warning & error messages (considered out of scope / separate proposal)

Rough Syntax Proposal

Syntax needs to be clarified and probably made less ambiguous, so far i only thought about categories, filters and severities.

Global Configuration

-Wconf <config>

  • categories
    • lint-x for every -Xlint:<x>, lint for all lints
    • x for every -W<x> (dead-code, value-discard, unused)
    • deprecation
    • language-x for every -language:<x>, language for all feature flags
    • in the future, when improving error/warnign messages, make new categories
    • any: matches any warning, whether or not it's in one of the categories above - filters
    • pos:com.package.Class where the warning is triggered, displayed
    • msg:expr -- need to decide details (full regex? only simple wildcard? substring match?)
    • only for deprecations: from:com.package.Class where the deprecated entity is declared
    • only for deprecations: since<2.12. see https://github.com/scala/scala/pull/7728/files#diff-3da64c3c223da7bbf9970ac26f9fb9b8R108. examples: since<Library 2.3, since<*2.3 to match any character prefix ("foo bar 2.3"), seems useful in combination with from:pkg
  • severity:
    • e(rror)
    • w(arning) / w-summary / w-group
    • i(nfo) / i-summary / i-group: show, but don't fail on -Werror (useful?)
    • s(ilent)
  • for warning and info:
    • summary: there were n warnings, per category.
    • group: show every warning with the same mesage once, mention how many instances.

Note: this only configures how warnings are emitted. -Xlint, -Wunused, Wdead-code etc are still required to make the compiler do the additional checks and emit warnings.

First matching rule wins

Relation to existing functionality

  • compiler default: -Wconf deprecation:language:warning-summary,any:warning
  • Adding a -Wconf adds rules on the left, i.e., the default comes last
  • -deprecation is the same as -Wconf deprecation:warning
  • -feature is -Wconf language:warning
  • -language:x is -Wconf language-x:silent
  • import scala.language.x is @silent("language-x") (see below)

Examples

  • -Wconf any:pos:com.corp.yolo:silent
  • -Wconf deprecation:from:org.fancy.library:since<*2.2:e,deprecation:i-group error on some deprecations, info-group others
  • -Wconf 'any:msg:pure expression does nothing in statement position:s'

Local Suppression

@silent: basically the same as https://github.com/ghik/silencer#annotation-based-suppression

  • on declarations @silent class C, @silent def f
  • on expressions as annotation ascription: {foo; bar}: @silent
  • by default, silence everything
  • categories: @silent("deprecated"), @silent("deprecated,language-higherKinds")
  • regex on message: @silent(msg="comparing true and false will always")
  • compiler error if @silent doesn't silence anything
@SethTisue
Copy link
Member

@SethTisue SethTisue commented Jun 6, 2017

perhaps the right context to tackle this would be in association with backporting the structured-errors stuff form Dotty

@olafurpg
Copy link
Member

@olafurpg olafurpg commented Jun 7, 2017

This would be useful for tools like scalafix. In particular, it would be useful to have the ability override the severity level per message kind, despite presence of -Xfatal-warnings. It would also be useful to have unique identifiers attached to each reported message (cc/ scalameta/scalameta#924)

@ShaneDelmore
Copy link

@ShaneDelmore ShaneDelmore commented Jun 7, 2017

Unique identifiers would be appreciated for scala-clippy as well. Currently we are using regexes to try to determine the message type.

@pmpfr
Copy link

@pmpfr pmpfr commented Nov 15, 2017

The other thing that is sorely lacking currently is a general escape hatch mechanism usable per-use-site, e.g. @SuppressWarnings(Seq(CompilerWarning.MissingInterpolator)). Currently people often have to globally disable useful warning because of a tiny number of false positives.

@jvican
Copy link
Member

@jvican jvican commented Nov 16, 2017

I think having @warnoff or @suppressWarnings in the standard library would be very useful. Currently, linters are forced to use comments because if they used their own annotation that would add a runtime dependency on downstream users. This approach would be much extensible and elegant and could enable other tools to reuse them.

@pmpfr
Copy link

@pmpfr pmpfr commented Nov 16, 2017

Agree. Wartremover uses @SuppressWarnings rather than comments, but only because it has to implement that itself. scalastyle uses comments, as you say, and it's very fragile/dangerous. It would be amazing if there was a single annotation where you could specify the particular warnings to silence, and it was respected by scalac, scalafix linters, wartremover, ...

@dwijnand
Copy link
Member

@dwijnand dwijnand commented Nov 8, 2018

Get excited? lampepfl/dotty#5337

@dwijnand
Copy link
Member

@dwijnand dwijnand commented Nov 24, 2018

Maybe bundled with this we can do scala/bug#8829.

@lrytz lrytz changed the title Configurable error reporting Configurable and suppressable warnings Jul 12, 2019
@SethTisue
Copy link
Member

@SethTisue SethTisue commented Mar 9, 2020

done!!! scala/scala#8373

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants