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.
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.
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.
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.
In general, these diagnostics are now errors in projects for which all
such warnings have been fixed. There are three unfixed warnings in
two projects, so this diagnostic remains a warning (not an error) in
those projects.
There are also many places where rwa-types-usage warnings have been
locally suppressed using @SuppressWarnings annotations. I haven't
systematically revisited those to see if any can be fixed properly.
So for those projects this diagnostic must also remain a warning (not
an error), since @SuppressWarnings does not work on things Eclipse is
configured to treat as errors.
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.
Specifically, we're turning off Eclipse warnings about missing version
constraints on required bundles ("Require-Bundle"), exported
packages ("Export-Package"), and imported packages ("Import-Package").
We're not turning these off absolutely everywhere, though: only in
packages where one or more such warnings were actually being reported.
So if a given package was already providing all version constraints
for, say, package imports, then we've kept that warning on in that
package.
Honestly I don't entirely understand the practical implications of
these warnings. However, there were 355 of them across many WALA
subprojects. I take this as evidence that the WALA developers do not
consider these version constraints to be important. Therefore, we may
as well stop warning about something we have no intention of fixing.
That being said, if we *do* want to fix some or all of these, I
welcome any advice on what those fixes should look like. I am rather
ignorant about all things OSGi.
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.
Previously, the various Eclipse projects' Java configurations used
mixtures of 1.6, 1.7, and 1.8. Many were internally inconsistent,
such as requiring 1.7 in "MANIFEST.MF" but 1.6 in the Eclipse JDT
build preferences. The Travis-CI configuration tests against both 1.7
and 1.8, but does not test against 1.6.
Across all projects, the most common version was 1.7. So I'm going to
assume that 1.7 is the intended build target. This commit makes 1.7
the selected version nearly everywhere.
"com.ibm.wala.core.testdata" is the one exception. This specific
project uses a few features only found in 1.8, such as lambda
expressions. Previously, "com.ibm.wala.core.testdata" used 1.7 in
some aspects of its configuration but 1.8 in others. Now it
consistently targets 1.8. I wish this one project didn't need to be
inconsistent with the rest of WALA, but at least now it's consistent
with itself.
(Personally, I'd be happy to target 1.8 only. But my impression
across all of these configuration files is that the WALA developers
still want to be compatible with 1.7. If that is no longer a
requirement, let me know and I will adjust these changes accordingly
to target 1.8 only.)
This change eliminates 11 "There is no 'jre.compilation.profile' build
entry and the project has Java compliance preferences set" warnings
and 13 "The JRE container on the classpath is not a perfect match to
the 'JavaSE-1.7' execution environment" warnings. However, it also
adds 450 "Redundant specification of type arguments <...>" warnings
and 17 "Resource '...' should be managed by try-with-resource"
warnings. So this seems like a net step backward in my wish to reduce
WALA warnings. However, those new warnings concern Java 1.7 language
features that we were not previously using to good effect in projects
that targeted 1.6. If we all agree that we can now target 1.7
instead, then we can use these helpful features as the newly-added
warnings suggest. So I call that a step in the right direction.
I think the "target/p2artifacts.xml" and "target/p2content.xml" files
are generated by Maven. They are well-formed XML but Eclipse's XML
validator legitimately warns that they lack grammar constraints.
Since we're not maintaining the tool that creates these files, we are
not in a position to do anything about that. Therefore, we may as
well exclude these from validation entirely. That way we can
more-clearly recognize warnings that we *can* do something about.
Eclipse validation warns about invalid HTML content in all
Maven-generated "target/site/dependency-convergence.html" files. The
warnings are legitimate: these HTML files are indeed invalid.
However, we don't maintain the tool that generates these files, so we
are not in a position to fix them. Better, therefore, to suppress
these warnings so that we can notice and fix other problems over which
we do have control.
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.
Eclipse Mars Service Release 2 finds 45 potential null pointer accesses
across WALA's various Eclipse projects. Eclipse ignores these by
default, but any individual user may have changed their personal Eclipse
configuration to treat them as warnings or errors. Thus, some people
will find that the code builds while others find that it fails. Better
to explicitly use a known-good configuration.
In the long run someone should inspect these cases one-by-one and fix
them where appropriate. But that is probably better managed as part of a
larger effort to tidy up nulls in WALA. I'm not planning to take that on
now or any time soon, though, so this is a better setup for now.
Julian Dolby
Ben Liblit
Manu Sridharan
sjfink