We now download and verify checksums as a single task, rather than as
two separate tasks. This simplifies other task dependencies, since we
no longer have a checksum-verified "stamp" file separate from the
download itself. Unfortunately the combined task now has a
significant amount of repeated boilerplate. I'm hoping to refactor
that all out into a custom task class, but haven't yet figured out the
details:
<https://github.com/michel-kraemer/gradle-download-task/issues/108>.
We now also use ETags to be smarter about when a fresh download is or
is not actually needed. I think there are still opportunities for
improved caching here, but this is a step in the right direction.
A cleaned tree is now much closer to a pristine tree that has just
been checked out and never built. The only extra created files that
are left behind are ".gradle", "buildSrc/.gradle", and
"buildSrc/build".
I think these were previously not being compiled at all. Now, with
Buildship generating Eclipse ".project" settings automatically, these
are being processed. In general we don't care much about questionable
code in test data, though.
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.
Julian Dolby assures me that WALA is now supposed to be using Java 8
everywhere. This covers nearly all remaining places that I can find
where an earlier Java version was still being used. (The few
exceptions are places where switching to Java 8 causes test failures.
I'll address those separately, probably by reaching out to the WALA
maintainers for help.)
This fixes two Eclipse Plug-in Development warnings of the form "The
'javacProjectSettings' build entry should be set when there are
project specific compiler settings".
This removes three Eclipse Plug-in Development warnings of the form
"This plug-in does not export all of its packages"
I assume that omitting some exports is OK, because apparently nothing
else fails to build against these. If an omitted export were needed
elsewhere, something would fail to build.
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 things one might consider fixing in real application
data. Java code used as test inputs, though, serves a different
purpose. Weird code is generally acceptable or even intentional.
Previously we had 5 such warnings. That's not very many, but it
suggests that the WALA developers consider this to be an acceptable
coding style. If that's so, then it's better to hide these warnings
rather than keep them around as a perpetual distraction.
Previously we had 227 such warnings. That large number suggests that
the WALA developers consider this to be an acceptable coding style.
If that's so, then it's better to hide these warnings rather than keep
them around as a perpetual distraction.
Previously we had 242 such warnings. That large number suggests that
the WALA developers consider this to be an acceptable coding style.
If that's so, then it's better to hide these warnings rather than keep
them around as a perpetual distraction.
The fix is to add "static" where appropriate, of course. I've also
simplified calls to such methods to reflect the fact that they no
longer need a specific object to call the method on.
In projects that contain test inputs, I've left the non-static
declarations unchanged, and instead downgraded the warning to be
ignored. In all other projects, this warning has been upgraded to an
error.
Along the way, I also converted many "for (;;)" loops into modern
"for (:)" loops. I didn't systematically look for all opportunities
to do this, though. I merely made this change where I was already
converting raw Iterator uses into modern Iterator<...> uses.
Better use of generics also allowed many casts to become statically
redundant. I have removed all such redundant casts.
Only three raw-types warnings remain after this batch of fixes. All
three involve raw uses of CallGraphBuilder. I've tried to fix these
too, but it quickly snowballs into a cascade of changes that may or
may not eventually reach a statically-type-save fixed point. I may
give these last few problem areas another go in the future. For now,
though, the hundreds of other fixes seem worth keeping even if there
are a few stragglers.
This commit may change some public APIs, but only by making weaker
type signatures stronger by replacing raw types with generic types.
For example, we may change something like "Set" into "Set<String>",
but we're not adding new arguments, changing any
underlying (post-generics-erasure) types, etc.
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.
In regular application code, these warnings should be taken seriously
and fixed. But for test code, it's better to keep things simple and
not add any methods that aren't strictly needed by the test.
In the `com.ibm.wala.util` project, configure Eclipse to treat any
future violations of this as errors, not merely warnings.
However, in `com.ibm.wala.cast.java.test.data`, configure Eclipse to
silently ignore missing @Override annotations. The JLex code in this
project is machine-generated, and we don't have a way to get the
generator to produce @Override annotations.
In general, test code may do all sorts of things that would be
considered poor style in production code. I assume that these
potentially-static methods are declared non-static by design.
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.