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:
javac
are easy and safe to remove; fixing cast warnings is essentially a zero-risk change.@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
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.
@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.
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.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.
hashCode
when you override equals
, for objects to behave properly when used in collections, they must have correct equals
and hashCode
implementations.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.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
.@Deprecated
elements suggest a non-deprecated replacement.@SuppressWarnings("deprecation")
can be used to acknowledge the situation and remove the warning.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.I hope these tips help you make your own Java project warnings-free.
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
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).
@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
Thank you Joe, that sure helps.