Eclipse warns that the "if" statements' true blocks are dead, and
indeed the conditions being tested here can never be true. It's a
little subtle why that's so, though. Changing them to "assert"
statements removes two warnings about deprecated code, while still
helping human readers understand what invariants must hold here.
The assert will throw AssertionError if it finds a bad argument,
unless the code is running without assertion checking enabled. The
"if" will throw an IllegalArgumentException if it finds the same bad
argument, and that check is always enabled. Better to be consistent
in what exception is used to report a bad argument. Since the "if" is
always active, let's go with that and remove the redundant,
not-always-active assert.
We used to use junit.framework.Assert for test assertions, and that
class reports failures by throwing
junit.framework.AssertionFailedError. However, junit.framework.Assert
is obsoleted by org.junit.Assert, and we've already replaced the
former with the latter. org.junit.Assert reports failures by throwing
java.lang.AssertionError, so that's what we need to expect in our
testing infrastructure.
All of these involve conditionals that check some static, final debug
flag or debug level. The code will indeed be dead if WALA is built
with those debug facilities turned off. But we still want the code
present in case someone needs to turn some aspect of debugging on for
a while.
The supposedly deprecated function (CGNode.addTarget) really seems to
be intended for internal use, not deprecated per se. We are an
internal user here, so presumably it's OK for us to be using this API
entry point.
Near as I can tell, the request for a deprecated version here is
intentional. The non-deprecated version (AST.JLS8) is the latest and
greatest, but as far as I can tell we really do want the older version
here.
This arises at some point during "mvn compile install". I'm not sure
exactly when, but it's definitely automated, and therefore not
appropriate to track in git.
This fixes the remaining 34 Eclipse "Resource '...' should be managed
by try-with-resource" warnings that were still left after the previous
commit.
Unlike the fixes in that previous commit, the changes here are *not*
plugging potential resource leaks. However, in many cases that is
simply because the code before the close() call cannot currently throw
exceptions. If exceptions became possible in the future, leaks could
result. Using try-with-resource preemptively avoids that.
Furthermore, in code that was already dealing with exceptions, the
try-with-resource style is usually considerably simpler.
This fixes 33 out of 37 Eclipse "Potential resource leak: '...' may
not be closed" warnings. It also fixes 3 out of 37 Eclipse "Resource
'...' should be managed by try-with-resource" warnings, although that
was not the main focus of this effort.
The remaining 4 warnings about potential resource leaks all involve a
leaked JarFile instance that is passed to a JarFileModule constructor
call. JarFileModile never attempts to close its underlying JarFile;
this code is written as though JarFile cleanup were the caller's
responsibility. However, the JarFile often cannot be closed by the
code that creates the JarFileModule either, since the JarFile needs to
remain open while the JarFileModule is in use, and some of these
JarFileModules stay around beyond the lifetime of the code that
created them. Truly fixing this would essentially require making
JarFileModule implement Closeable, which in turn would probably
require that Module implement Closeable, which in turn would require
changes to lots of code that deals with Module instances to arrange
for them to be properly closed. That's more invasive than I'm
prepared to take on right now.
Instead, rely on Java's ability to infer type parameters in many
contexts. This removes 665 Eclipse warnings.
Note: a few of these changes are to files under "test" subdirectories.
Presumably those are files that serve as test inputs rather than being
part of WALA code proper. As far as I can tell, these changes do not
break any WALA tests. But if any of those tests were specifically
intended to exercise WALA on code with non-inferred generic type
parameters, then I really should be leaving those alone.
Manu Sridharan
Ben Liblit
Raffi Khatchadourian
Julian Dolby