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 a couple of 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 excessive coverage creates opportunity for false negatives. That is, warnings other than the particular one being 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 in practice the continued need for the suppression is 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. If the compiler could detect and report unnecessary warning suppression, developers could ensure their warning suppression is as narrow as it can possibly be.

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

      Solution

      The solution is add the ability for the compiler to generate an "unnecessary suppression" warning when a lint category is being suppressed unnecessarily.

      This CSR proposes to warn only about unnecesssary warning suppression via @SuppressWarnings annotations; unnecessary suppression via the -Xlint:-category flag 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 compiler will be perfectly accurate, 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 without 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.

      New Lint Category

      A new "suppression" lint category will be added to allow developers to control whether this new warning is enabled using the usual mechanism. Even though the new warning relates to the suppression of other warnings, it is still just a normal compiler warning with an associated lint category that can be controlled using the normal mechanisms, i.e., @SuppressWarnings annotations and -Xlint flags.

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

      Existing Lint Categories

      A few lint categories don't support suppression via @SuppressWarnings: these are "classfile", "incubating", "options", "output-file-clash", and "path". Therefore, occurrences of (for example) @SuppressWarnings("classfile") have no effect and are therefore always unnecessary. When unnecessary suppression warnings are enabled, such occurrences will always trigger a warning.

      On the other hand, warnings will never be generated for unrecognized lint categories, e.g. @SuppressWarnings("blah"). This is consistent with the JLS recommendation for compilers to ignore such values. This includes doclint keys, which are not lint categories.

      Nested Annotations

      The determination of whether a suppression is unnecessary is unambiguous except possibly in the scenario where a warning is suppressed twice at two different levels, for example:

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

      Clearly one of the @SuppressWarnings annotations is unnecessary, but which one? The implementation always assumes that the innermost annotation is the "real" one doing the suppression, and therefore any outer annotations are the "unnecessary" ones. In the above example, the outer annotation on class A would generate an unnecessary suppression warning. This behavior is consistent with the principle that warning suppression should be apply as narrowly as possible: removing the annotation indicated by the warning will ensure that this property is maintained.

      Self Reference

      What is the meaning of @SuppressWarnings("suppression")? Although self-referential, this annotation is both valid and useful. It means no unnecessary suppression warnings (i.e., the warnings in lint category "suppression") will arise from that annotation, or from any annotation contained by the corresponding declaration.

      For example, no unnecessary suppression warning would be generated by this code:

      @SuppressWarnings("suppression")
      public class A {
      
          @SuppressWarnings("unchecked")
          public abstract int m();
      }

      or by this code:

      @SuppressWarnings({ "suppression", "unchecked" })
      public class A {
      
          public abstract int m();
      }

      As mentioned above, @SuppressWarnings("suppression") applies equally to itself. So no unnecessary suppression warning would be generated by this code either:

      @SuppressWarnings("suppression")
      public class A { }

      Put another way, an unnecessary suppression warning can never be generated for the "suppression" lint category.

      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 unnecessary 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: