If the true block of an `if` statement is guaranteed to exit early,
such as by a `return` or `throw`, then any code appearing in a
corresponding `else` clause could just as well have appeared after the
`if` statement entirely. Eclipse can warn about this.
However, Manu prefers to let such code stay in the `else` clauses.
OK, sure: this is more a matter of personal taste than something truly
problematic. Per Manu's request, then, we're turning off that Eclipse
warning in the subprojects in which it currently arises.
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.
Fixing these Javadoc comments would require adding packages to various
other packages' build paths. In some of the cases suppressed,
changing build paths in that manner would create circular build
dependencies. In other cases, it would simply add a Javadoc-motivated
dependency that does not exist for the real code, which seems
undesirable. For a few cases, the reference seems to be to types in
code we don't even have here, such as code from "android" or
"org.mozilla" packages.
These changes turn off Eclipse warnings for Javadoc comments with
missing tags, such as "@throw" or "@param".
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 327 of them. Apparently the WALA team's
implicit coding style says that omitting Javadoc tags is OK. If
there's no intent to systematically add these tags, then we may as
well turn off these warnings so that we can see other warnings that we
may want to fix.
These changes turn off Eclipse warnings for documentable items without
Javadoc comments. In some subprojects, we turn these off entirely.
In others, we leave these warnings on for public items but not for
items whose visibility is protected or below.
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 1,366 of them. Apparently the WALA
team's implicit coding style says that omitting Javadoc comments is
OK. If there's no intent to systematically add documentation, then we
may as well turn off these warnings so that we can see other warnings
that we may want to fix.
These arise, for example, when Javadoc documentation on a public class
includes a @link to a private field. I can see how this would be
problematic for closed-source Java code where private items are
invisible to outsiders. However, given that WALA is open source,
nothing is truly non-visible. If the WALA documentation authors
considered non-visible references useful when explaining things,
that's fine with me.
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.
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.
As it turns out, I should have been using "? extends InstanceKey" rather
than "? super InstanceKey". But really, we can just use "?" here since
HeapGraph itself constrains its own type parameter appropriately.
Effectively these two checks could only be false if the instance being
tested were null. So we replace the instanceof checks with null
checks. Sometimes that, in turn, makes other surrounding code
simpler. In the case of ApplicationLoaderFilter.test, for example,
a whole conditional case ("o instanceof LocalPointerKey") becomes
statically impossible. That seems a bit strange to me, but that's
what the code was effectively doing.
In the cases addressed here, the caught exception was being "handled"
by throwing some new exception. Instead of discarding the old
exception, pass it to the new exception's constructor to indicate the
original cause of the newly-created exception. This practice, called
"exception chaining", can often be useful in debugging.
Note: some of these methods are decidedly nontrivial. Perhaps they
should not actually be removed? If any should be kept around, please
identify them to me. I'll revise this change to retain those methods
and simply annotate them as needed to suppress Eclipse's warning.
Specifically, these warnings are always of the form "Statement
unnecessarily nested within else clause. The corresponding then clause
does not complete normally". I can see this being a matter of
personal taste, actually. If the WALA maintainers decide that they
prefer to keep this code as it was before, that's OK with me: I'll
replace this group of code changes with a group of settings changes
that tell Eclipse to quietly ignore this "problem".