This gives the WALA maintainers the option of doing future 1.4.5+
releases from of a pre-Gradle branch if these merged Gradle changes
turn out to be more disruptive than expected.
These settings files currently are generated with an initial timestamp
comment line, which is not something we'd want to track in version
control. Fortunately, the contents of these files are entirely
mundane, so there should be no problem with having Buildship generate
them anew each time a developer imports WALA into Eclipse as an
existing Gradle project.
Apparently Buildship generates these when one uses Import -> Existing
Gradle Project:
<https://discuss.gradle.org/t/buildship-eclipse-plug-in-multiproject-builds/24030/5>.
We can use the Gradle "eclipse" plugin if customizations are
necessary, but my impression is that the intent is to treat ".project"
and ".classpath" as generated files, not sources to be tracked in
source control.
I was confused about the differences among:
srcDir 'foo'
srcDirs ['foo']
srcDirs = ['foo']
As it turns out, the first two append to the set of source
directories, while the last replaces this set entirely. I generally
want replacement, since WALA's current directory layout never matches
Gradle's assumed defaults.
By default, each subproject's Javadoc task depends on the same
subproject's Java compilation task, and uses the same classpath.
Thus, any classes that some Java code uses will also be visible when
building the same Java code's documentation.
In this case, we need to see one of the "com.ibm.wala.core" classes in
order to build the "com.ibm.wala.util" documentation. However, we
cannot have Java compilation of "com.ibm.wala.util" depend on Java
compilation of "com.ibm.wala.core", because that would create a
dependency cycle. So we need to add this as a special dependency just
for the "com.ibm.wala.util" documentation task, and add the
appropriate classpath as well.
I'm quite proud of myself for figuring out how to do this properly.
Eclipse's automated code clean-up tool did most of the heavy lifting
here: it specifically has a clean-up option for converting functional
interfaces to lambdas. I merely had to revert the automated changes
for a single enumeration class for which it produced invalid results,
and for a few test inputs that apparently aren't set up to be compiled
with Java 8.
Previously FilterIterator was very permissive regarding the type
relationships between the original iterator, the filtered iterator,
and the predicate used to prune the former down to the latter. Now we
enforce those relationships more strictly, including proper use of
covariant ("<? extends T>") and contravariant ("<? super T>")
polymorphic type parameters where appropriate.
This lets us get rid of seven suppressed warnings about generic types
and/or unchecked conversions. It also moves us toward being able to
use modern Java features like lambdas and streams more easily.
In Eclipse projects that currently have no definite or potential
resource leaks, treat any such diagnostics as errors in the future.
In `com.ibm.wala.core`, enable warnings about definite or potential
resource leaks. Previously these diagnostics were turned off entirely
in this project. So we actually end up with more warnings now than we
had before, but they are all warnings we should eventually look into.
These are all problems that Eclipse can detect, but that it detects no
instances of right now. Treating these as warnings instead of errors
should help prevent us from slipping backward in the future.
Instead of having two `switch` statements on the Dot format
enumeration, give each Dot enumeration value a way to identify its own
preferred file suffix and command-line format name. This removes some
warnings about `switch` statements without default cases. It also
creates strong static enforcement that any new Dot format *must* also
provide this information.
Most of these are harmless, and are best fixed simply by removing the
redundant check or assignment. The one in FlowType.compareBlocks,
however, revealed a real problem. This code checks for nullness of
`a` *after* having called a method on `a`. Assuming that `a` can
indeed be `null` here, the check must come first to avoid a
`NullPointerException`.
In several places, I saw code of this form:
if (thing == null)
assert thing != null : ... ;
I honestly don't understand the purpose of that `if` statement. Why
not just have the `assert` statement there directly? I removed the
seemingly irrelevant `if` statements in these cases, but if this is
some intentional pattern, please explain it to me.
In a few places where nullness is statically known but non-obvious,
add assert statements to point out what's going on to help future
developers.
Upgrade future such warnings to errors to keep us moving in a cleaner
direction.
Ben Liblit
Sungho Lee
Julian Dolby
Manu Sridharan