Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8356043

Add lint warnings for unnecessary warning suppression

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Unresolved
    • Icon: P4 P4
    • None
    • tools
    • None
    • behavioral
    • low
    • Hide
      As with any new warning, builds that succeeded before with `-Xlint:all -Werror` can start to fail, requiring adjustment to the code and/or the `-Xlint` flag. But this is also the worst case: that is, disabling the new lint category should revert the compiler to its previous behavior.
      Show
      As with any new warning, builds that succeeded before with `-Xlint:all -Werror` can start to fail, requiring adjustment to the code and/or the `-Xlint` flag. But this is also the worst case: that is, disabling the new lint category should revert the compiler to its previous behavior.
    • JDK

      Summary

      Add a new compiler warning that warns about unnecessary warning suppressions in @SuppressWarnings annotations.

      Problem

      The Java compiler emits warnings to alert developers about code that appears incorrect or imprudent. It is then up to the developer to apply human judgement to decide whether to change the code, ignore the warning, or suppress the warning.

      For lint warnings (which is most of them), suppression is accomplished at a per-declaration level by @SuppressWarnings annotations and globally by the -Xlint:-category command line flag.

      There are two problems with this mechanism.

      The first problem is that @SuppressWarnings and -Xlint:-category flags are very blunt instruments. In fact, the latter is maximally blunt: it covers the entire compilation. The former is still quite blunt, because while affected warnings occur at specific lines of code, the @SuppressWarnings annotation always applies to an entire class, method, or variable declaration.

      This imprecision is a problem because that exessive coverage creates opportunity for false negatives. That is, warnings other than the particular one breing targeted could be suppressed unwittingly, creating an opportunity for uncaught bugs.

      @SuppressWarnings annotations are sometimes added even when they're never actually needed, i.e., proactively as the code is written, because the developer thinks they might be needed and wants to potentially save one compile & debug cycle. But the compiler provides the same feedback (i.e., none) whether or not there was actually any warning there, so this type of mistake is rarely caught.

      The second problem is that once a warning is suppressed, the affected code can continue to evolve, but the continued need for the suppression is typically rarely, if ever, reviewed. This is due mostly to the "out of sight, out of mind" effect, but also even for a developer diligent enough to track this, doing such a review would require a tedious manual, per-warning audit. This also creates an opportunity for false negatives, i.e., more uncaught bugs.

      Perhaps most frustrating, the compiler by definition has all the information needed to detect unnecessary warning suppression, yet there's no way for developers to automatically extract or apply this information.

      An analysis of the JDK reveals over 400 unnecessary suppressions currently being applied, so this problem is real.

      Solution

      The solution is to enable the compiler to report when a lint category is being suppressed unnecessarily. This CSR proposes to warn only about unnecesssary warning suppression via @SuppressWarnings annotations; unnecessary suppression via -Xlint:-category flags is left for future study.

      Definition of Unnecessary Suppression

      First, it is important to be precise about the meaning of "unnecessary suppression". A suppression is the occurrence of a lint category within a @SuppressWarnings annotation. Such a suppression is unnecessary if recompiling with (a) that suppression removed and (b) that lint category enabled would not result in the generation of new warning(s) in that lint category in any of the code covered by the suppression.

      Condition (b) is because we want the determination of whether a suppression is unnecessary to be a function of the code itself, not the details of how a particular compiler run is configured. Therefore, the determination is always made as if the corresponding category were enabled. That is, disabling all lint categories doesn't suddenly make all suppressions unnecessary.

      When it comes to reporting unnecessary suppressions, ideally the implementation is perfect, but if not, a crucial observation is that false positives are much worse than false negatives: a false negative is simply equivalent to the current behavior, while a false positive could put a developer in a situation where it would be impossible to ever build with no warnings (short of removing code). Therefore, it is critical that false positives be avoided. When given a choice, the implementation should err on the side of false negatives.

      Implementation of Unnecessary Suppression Warnings

      A new suppression lint category is added to allow developers to control whether this warning is enabled using the usual mechanism. Even though this warning relates to the suppression of other warnings, it is still just a normal compiler warning.

      By default, the suppression lint category will be disabled: the -Xlint flag must be used to enable it.

      Suppresion tracking and consequent warning generation is not enabled for all lint categories. The following lint categories are always excluded from unnecessary supression warnings: path, options, and suppression itself. Warnings will never be generated for unnecessary suppression of these categories.

      The meaning of unnecessary suppression by a @SuppressWarnings annotation is clear except possibly in the scenario where a warning is suppressed by two levels of @SuppressWarnings annotations:

      @SuppressWarnings("divzero")
      public class A {
      
          @SuppressWarnings("divzero")
          public int m() {
              return 1/0;
          }
      }

      Clearly one of them is unnecessary, but which one? The implementation always assumes that the innermost annotation is the "real" one doing the suppression and any containing annotations are the "unnecessary" ones. In the above example, the outer annotation on class A would generate the warning. This behavior is consistent with the principle that warning suppression should be apply as narrowly as possible; removing the suppression indicated by the warning always ensures that this principle is applied.

      The JLS recommends that unrecognized strings in @SuppressWarnings annotations be ignored. If a @SuppressWarnings category key is not recognized as a LintCategory, it will never generate an unnecessary suppression warning (that includes doclint keys, which are not LintCategorys).

      As mentioned above, some lint categories are not tracked for unnecessary suppression: path, options, and suppression itself. Warnings will never be generated for suppression of these categories.

      Other lint categories are tracked for unnecessary suppression, but don't support @SuppressWarnings (for example, output-file-clash). If a @SuppressWarnings category key is recognized and tracked but does not support suppression via @SuppressWarnings, then it will always generate an unnecessary suppression warning.

      What about @SuppressWarnings("suppression")? The lint category suppression is itself excluded from suppression tracking, so this could never generate a warning for unnecessary suppression. However, the annotation is completely valid for other lint categories, where the usual thing happens: no unnecessary suppression warnings will be generated by that annotation or any @SuppressWarnings annotations on any nested declarations.

      Specification

      The output of javac --help-lint changes as follows:

      --- x0  2025-05-01 14:38:06.521664879 -0500
      +++ x1  2025-05-01 14:38:30.359932368 -0500
      @@ -167,6 +167,8 @@
                                Also warn about other suspect declarations in Serializable and Externalizable classes and interfaces.
           static               Warn about accessing a static member using an instance.
           strictfp             Warn about unnecessary use of the strictfp modifier.
      +    suppression          Warn about recognized @SuppressWarnings values that don't actually suppress any warnings.
           synchronization      Warn about synchronization attempts on instances of value-based classes.
           text-blocks          Warn about inconsistent white space characters in text block indentation.
           this-escape          Warn when a constructor invokes a method that could be overriden in an external subclass.

      The warning for unneccessary suppression via @SuppressWarnings looks like this:

          $ javac -Xlint:suppression Test.java
          Test.java:10: warning: [suppression] unnecessary warning suppression: "divzero"
              @SuppressWarnings("divzero")
              ^
          1 warning

      If multiple categories in the same annotation are being warned about, the warning looks like this:

          $ javac -Xlint:suppression Test.java
          Test.java:10: warning: [suppression] unnecessary warning suppression: "rawtypes", "unchecked"
              @SuppressWarnings({ "rawtypes", "unchecked" })
              ^
          1 warning

            acobbs Archie Cobbs
            acobbs Archie Cobbs
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: