X

Musings on JDK development

Try Oracle Cloud Platform For Free
  • Java
    January 25, 2015

Advice on removing javac lint warnings

Joe Darcy

Over the last twelve months or so, one of my projects has been fixing and reviewing fixes of javac lint warnings in the JDK 9 code base
(varargs,
fallthrough,
serial,
finally,
overrides,
deprecation,
raw and unchecked) and once a warning category is cleared, making new instances of that category a fatal build error.
Ultimately, all the warnings in the jdk repository were resolved and -Xlint:all -Werror is now used in the build.

Being involved in fixing several thousand warnings, I'd like to share some tips for developers who want to undertake an analogous task of cleaning up the technical debt of javac lint warnings in their own code base.
First, I recommend tackling the warnings in a way that aligns well with the build system of the project, with a consideration of getting some code protected by the compiler from some warning categories as soon as possible.
While the build of the JDK has been re-engineered over the course of the warnings cleanup, to a first approximation the build has been organized around Hg repositories. (At present, in JDK 9 the build is actually arranged around modules. A few years ago, the build was organized around Java packages rather than repositories.)
A warnings cleanup isn't really done until introducing new instances of the warning cause a build failure; new warnings are too easy to ignore otherwise.
Therefore, for JDK 9, the effort was organized around clearing the whole jdk repository of a warning category and then enabling that warning category in the build as opposed to, say, completely clearing a particular package of all warnings and then moving to the next package.

There are two basic approaches to resolving a warning: suppressing it using the @SuppressWarnings mechanism or actually fixing the code triggering the warning.
The first approach is certainly more expedient. While it doesn't directly improve the code base, it can offer an indirect benefit of creating a situation where new warnings can be kept out of the code base by allowing a warning to be turned on in the build sooner.
The different warning categories span a range of severity levels and while some warnings are fairly innocuous, others are suspicious enough that I'd recommend always fixing them if a fix is feasible.
When resolving warnings in the JDK, generally the non-deprecation warnings categories were fixed while the deprecation warnings were suppressed with a follow-up bug filed. The non-deprecation warnings mostly require Java language expertise to resolve and little area expertise; deprecation warnings are the reverse, often quite deep area expertise is needed to develop and evaluate a true fix.

Tips on addressing specific categories of lint warnings:



[cast]: Warn about use of unnecessary casts.


Since these warnings are generated entirely from the the contents of method bodies, there is no impact to potential callers of the code. Also, the casts analyzed as redundant by javac are easy and safe to remove; fixing cast warnings is essentially a zero-risk change.

[fallthrough]: Warn about falling through from one case of a switch statement to the next.


When such a falling through is not intentional, it can be a very serious bug. All fallthrough switch cases should be examined for correctness.
An idiomatic and intentional fallthrough should have two parts: first, the cases in question should be documented in comments explaining that the fallthrough is expected and second, an @SuppressWarnings({"fallthrough"}) annotation should be added to the method containing the switch statement.

See also the discussion of switch statements in Java Puzzlers, Puzzler 23: No Pain, No Gain.


[static]: Warn about accessing a static member using an instance.


This is an unnecessary and misleading coding idiom that should be unconditionally removed. The fix is to simply refer to the static member using the name of the type rather than an instance of the type.

This coding anti-pattern is discussed in Java Puzzlers, Puzzle 48: All I Get Is Static.


[dep-ann]: Warn about items marked as deprecated in JavaDoc but not using the @Deprecated annotation


Since Java SE 5.0, the way to mark an element as deprecated is to modify it with a @Deprecated annotation. While a @deprecated javadoc tag should be used to describe all @Deprecated elements, the javadoc tag is informative only and does not mean the element is treated as deprecated by the compiler.

A element should have an @deprecated javadoc tag in its javadoc if and only if the element is @Deprecated.

Therefore, the fix should be to either remove the @deprecated javadoc tag if the element should not be deprecated or add the @Deprecated annotation if it should be deprecated.


[serial]: Warn about Serializable classes that do not provide a serialVersionUID.


Serialization is a subtle and complex protocol whose compatibility impact on evolving a type should not be underestimated.
To check for compatibility between the reader of serial stream data and the writer of the data, besides matching the names of the reader and writer, identification codes of the reader and the writer are also compared and the serial operation fails if the codes don't match.
When present, a serialVersionUID field of a class stores the identification code, called a Stream Unique Identifier (SUID) in serialization parlance. When a serialVersionUID field is not present, a particular hashing algorithm is used to compute the SUID instead. The hash algorithm is perturbed by many innocuous changes to a class and can therefore improperly indicate a serial incompatibility when no such incompatibility really exists.
To avoid this hazard, a serialVersionUID field should be present on all Serializable classes following the usual cross-version serialization contracts, including Serializable abstract superclasses.

If a Serializable class without a serialVersionUID has already been shipped in a release, running the serialver tool on the type in the shipped release will return the serialVersionUID declaration needed to maintain serial compatibility.

For further discussion, see Effective Java, 2nd Edition, Item 74: Implement Serializable judiciously.


[overrides]: Warn about issues regarding method overrides.


As explained in Effective Java, 2nd Edition, Item 9: Always Override hashCode when you override equals, for objects to behave properly when used in collections, they must have correct equals and hashCode implementations.
The invariant checked by javac is more nuanced than the one discussed in Effective Java; javac checks that if a class overrides equals, hashCode has been overriden somewhere in the superclass class chain of the class.
It is common for a set of related classes to be able to share a hashCode implementation, say a function of a private field in the root superclass in a set of related types. However, each class will still need to have its own equals method for the usual instanceof check on the argument to equals.

[deprecation]: Warn about use of deprecated items.


Well documented @Deprecated elements suggest a non-deprecated replacement.
When using a replacement is not feasible, or no such replacement exists, @SuppressWarnings("deprecation") can be used to acknowledge the situation and remove the warning.
A small language change made in JDK 9 makes suppressing deprecation warnings tractable.

[rawtypes]: Warn about use of raw types.

[unchecked]: Warn about unchecked operations.


Both rawtypes and unchecked warnings are linked to the same underlying cause: incomplete generification of APIs and their implementations.
Generics shipped in 2004 as part of Java SE 5.0; Java code written and used today should be generics aware!
Being generics-aware has two parts, using generics properly in the signature / declaration of a method, constructor, or class and using generics properly in method and constructor bodies.
Many uses of generics are straightforward; if you have a list that only contains strings, it should probably be declared as a List<String>. However, some uses of generics can be subtle and are out of scope for this blog entry. However, extensive guides are available with detailed advice.
IDEs also provide refactorings for generics; check their documentation for details.

I hope these tips help you make your own Java project warnings-free.

Join the discussion

Comments ( 4 )
  • Anthony Ve Monday, January 26, 2015

    Concerning "serial": as you say, if you don't define serialVersionUID, you might have a situation where 2 versions of a Serializable class are considered incompatible when in fact they aren't.

    However, from my experience, many people define serialVersionUID (because javac/their IDE/someone said so) but don't actually understand what it means. Hence, they never change its value, even when making incompatible changes. Thus you get a situation where 2 versions are considered compatible, when in fact they aren't. (Of course this situation can also arise when you accidentally forget to update the serialVersionUID).

    I would be grateful if you could shed some light on the following questions:

    - why is the one situation worse than the other?

    - why isn't the hash algorithm enhanced to always be correct?

    Thanks, Anthony


  • guest Monday, January 26, 2015

    I sincerely hope that most of the Java developers have already fixed these warnings in their projects where possible. AFAIK most (or even all?) could be recognized by common Java IDEs (e.g. Eclipse) or common static analysis tools (e.g. FindBugs) years ago.

    Some easy cases (e.g. unnecessary casts) can be automatically fixed by IDEs (e.g. using the clean up feature of Eclipse).


  • guest Monday, January 26, 2015

    @Anthony,

    You are correct that there are two basic kinds of compatability errors that could be encountered in serialization:

    1) A serial *in*compatible change is treated as compatible

    2) A serial compatible change is treated as *in*compatible

    In terms of balancing errors (https://blogs.oracle.com/darcy/entry/balance_of_error), the design of serialization treats 1) as the more serious condition. In other words, by being very sensitive to the set of methods and fields in a class, the default serial hash is likely to catch any change that actually introduces a serial incompatability, at the cost of judging some compatible changes as incompatible.

    At least for the JDK, we usually run into the second type of error, which guided my advice abvove.

    In the worst case, it would be impractical for a hash to fully capture serial compatability: consider cases were the internal representation is changed and code in readResolve, writeReplace, etc. is used to output and read in the prior format.

    HTH,

    -Joe


  • Anthony Ve Tuesday, January 27, 2015

    Thank you Joe, that sure helps.


Please enter your name.Please provide a valid email address.Please enter a comment.CAPTCHA challenge response provided was incorrect. Please try again.

Recent Content