Unnecessary "throws" declarations tend to cascade. If foo() calls
bar() and bar() falsely declares that it might throw IOException, that
often leads a programmer to declare that foo() might throw IOException
as well. Fixing the bar() throws declaration then reveals that we can
fix the foo() throws declaration too. By the time we reach a fixed
point with cleaning these up, we have removed roughly 320 unnecessary
throws declarations.
In a few cases, this cleanup even lets us remove entire "try
... catch" statements where the only thing being caught was an
exception that we now statically know cannot be thrown. Nice!
In Eclipse project configurations, upgrade any future such shenanigans
from warnings to errors. Now that we've fixed this, we don't want it
coming back again.
There is a potential drawback to this change. Conceivably some public
WALA API entry point might have declared that it could throw some
exception merely to reserve the *option* of throwing that exception in
third-party code that subclasses and overrides the API entry point in
question. I have no idea whether this is a significant concern in
practice, though.
There are two such diagnostics: one for collection methods and one for
equals(). See
<https://www.eclipse.org/eclipse/news/4.7/jdt.php#unlikely-argument-types>
for more information about these two new diagnostics.
For each of these diagnostics, I've set the severity level to
"warning" in projects that have some instances of the suspicious code,
or to "error" in projects that have no instances of the suspicious
code.
These should mostly be things that we've already decided earlier that
we explicitly don't want to "fix" because they simply disagree with
the WALA project's coding style.
The additional diagnostics are ones that were previously being
ignored, but which we seem to have been ignoring by default rather
than as a conscious choice.
For diagnostics of which we currently have *zero* instances, treat
these as errors rather than merely warnings. The intent is to
permanently lock out future regressions of things we've completely
fixed. In the future, whenever we fix the last instance of a given
warning in a given Eclipse project, we should also promote that
diagnostic to an error to keep things clean into the future.
The "potentially" qualifier is here because these methods are visible
outside the WALA source tree. These methods may seem OK to be static
based on the code we have here, but we have no way of knowing whether
third-party code expected to be able to subclass and override. I'm
going to play it safe and assume that we want to allow that.
Note that we are still allowing Eclipse warnings about methods that
can *definitely* be declared static; a different configuration option
controls these. For private methods, final methods, and methods in
final classes, if the code seems static-safe based on what we have
here, then that's good enough: we don't need to worry about
third-party overrides.
Manu requested that we use this approach instead of adding
`@SuppressWarnings("unused")` at each affected catch block. That
seems reasonable to me, given the large number of such warnings and
the lack of likely harm from ignoring such caught exceptions.
These changes turn off Eclipse warnings for Javadoc tags without
descriptions. In some subprojects, we turn these off entirely. In
others, leave on missing-descrption checks for "@return" tags only.
We don't turn this warning off in all projects. Rather, we turn it
off only in projects that were producing at least one such warning.
In other words, if a project was already completely "clean" with
respect to this warning, then we leave this warning enabled for that
project.
Turning off these warnings is a partial declaration of Javadoc
bankruptcy. In an ideal world, we would enable and fix all of these
warnings. However, there are 576 of them. Apparently the WALA team's
implicit coding style says that omitting descriptions is OK. If
there's no intent to systematically add descriptions, then we may as
well turn off these warnings so that we can see other warnings that we
may want to fix.
In general, the WALA code base is not really ready for nullness
checking. It would be nice if we got there some day, but I'm not
planning to take that on now or any time soon. Until then, it's not
useful to warn about missing @NonNullByDefault declarations on WALA
packages.
See also older commit 7b6811b.
Julian Dolby
Ben Liblit
Manu Sridharan
Julian Dolby
Manu Sridharan
dolby-oss