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.
In general, my approach was to try to eliminate each unused parameter
using Eclipse's "Change Method Signature" refactoring. That did not
always succeed: a parameter may be unused in some base class method,
but then be used in subclass's override of that method. In cases
where refactoring to eliminate a parameter failed, I instead annotated
the parameter with '@SuppressWarnings("unused")' to silence the
warning.
Note: this group of changes creates a significant risk of
incompatibility for third-party WALA code. Some removed parameters
change externally-visible APIs. Furthermore, these changes do not
necessarily lead to Java compilation errors. For example, suppose
third-party code subclasses a WALA class or interface, overrides a
method, but does not annotate that method as @Override. Removing a
parameter means that the third-party method no longer overrides. This
can quietly change code behavior without compile-time errors or
warnings. This is exactly why one should use @Override wherever
possible, but we cannot guarantee that third-party WALA users have
done that.
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.
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.
Previously some of these were accessing such fields through a subclass
of the declaring class. That creates an unnecessary extra inter-class
dependency lower in the type hierarchy than necessary.
Also, suppress this warning in an automated test input where the
indirect static accesses are explicitly intentional.
If a method is private, there's no risk that a subclass elsewhere
might be overriding it and depending on dynamic dispatch to choose the
right implementation. So all of these private methods can safely be
declared static without risk of regression in either WALA code or
unseen third-party code.
The "potentially" qualifier is here because these methods are visible
outside the WALA source tree. These methods may seem OK to be static
based on the code we have here, but we have no way of knowing whether
third-party code expected to be able to subclass and override. I'm
going to play it safe and assume that we want to allow that.
Note that we are still allowing Eclipse warnings about methods that
can *definitely* be declared static; a different configuration option
controls these. For private methods, final methods, and methods in
final classes, if the code seems static-safe based on what we have
here, then that's good enough: we don't need to worry about
third-party overrides.
In each of these cases, the constructor directly or indirectly has
side effects that we want to keep, even if the object itself is not
retained and used by eht code that invokes `new`.
Specifically, these are all warnings of the form "The
'javacProjectSettings' build entry should be set when there are project
specific compiler settings".
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.
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.
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.
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.
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.
As created by Tycho Surefire, these files are XML documents without
DTD or XML Schema declarations. The XML validator warns about this
omission. However, Surefire is not a WALA component. We are not in a
suitable position to change it to include XML schema or DTD
declarations in the XML files it generates. Better, then, to ignore
this benign problem so we can focus on warnings that we can act on
directly.
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.
now, for me, code works using e44 with maven
dalvik tests refactored for mobile version with android dev tools
IDE tests Eclipse metadata fixed to make e44 work for me
new android entrypoint to fix failure in new droidbench tests
com.ibm.wala.cast.js.rhino.test/harness-src/com/ibm/wala/cast/js/test/TestPrototypeCallGraphShapeRhino.java
com.ibm.wala.cast.js.test/harness-src/com/ibm/wala/cast/js/test/TestPrototypeCallGraphShape.java
com.ibm.wala.cast.js.test.data/examples-src/pages/prototype.html
work (not yet finished) on fixes to property accesses for JavaScript:
com.ibm.wala.cast/source/java/com/ibm/wala/cast/ipa/callgraph/AstSSAPropagationCallGraphBuilder.java
com.ibm.wala.cast.java/src/com/ibm/wala/cast/java/ipa/callgraph/AstJavaSSAPropagationCallGraphBuilder.java
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/ipa/callgraph/JSSSAPropagationCallGraphBuilder.java
currently unused tests to remind me to fix bugs:
com.ibm.wala.cast.js.test/harness-src/com/ibm/wala/cast/js/test/TestSimpleCallGraphShape.java
com.ibm.wala.cast.js.test.data/examples-src/tests/loops.js
com.ibm.wala.cast.js.test.data/examples-src/tests/primitive_strings.js
fixes to exception handler code generation in JavaScript:
com.ibm.wala.cast.js.rhino/source/com/ibm/wala/cast/js/translator/RhinoToAstTranslator.java
com.ibm.wala.cast.js.test.data/examples-src/tests/try.js
com.ibm.wala.cast.js.test/harness-src/com/ibm/wala/cast/js/test/TestSimpleCallGraphShape.java
fixes to make the system build on both juno and luna
com.ibm.wala.cast.js.test.data/pom.xml
pom.xml
targets/e42/e42.target
targets/e44/e44.target
targets/pom.xml
com.ibm.wala.core.tests/META-INF/MANIFEST.MF
com.ibm.wala.dalvik.test/META-INF/MANIFEST.MF
com.ibm.wala.ide.jdt.test/META-INF/MANIFEST.MF
com.ibm.wala.ide.jdt/source/com/ibm/wala/cast/java/translator/jdt/FakeExceptionTypeBinding.java
com.ibm.wala.ide.jdt/source/com/ibm/wala/ide/util/JavaEclipseProjectPath.java
com.ibm.wala.ide.jsdt.tests/META-INF/MANIFEST.MF
com.ibm.wala.ide.jsdt.tests/src/com/ibm/wala/ide/jsdt/tests/AbstractJSProjectScopeTest.java
com.ibm.wala.ide/src/com/ibm/wala/ide/util/EclipseProjectPath.java
com.ibm.wala.ide/src/com/ibm/wala/ide/util/ProgressMonitorDelegate.java
beginnings of "pointer analysis" on top of field-based analysis
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/callgraph/fieldbased/flowgraph/FlowGraph.java
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/callgraph/fieldbased/flowgraph/vertices/PropVertex.java
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/callgraph/fieldbased/flowgraph/vertices/RetVertex.java
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/callgraph/fieldbased/flowgraph/vertices/VarVertex.java
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/callgraph/fieldbased/flowgraph/vertices/VertexFactory.java
com.ibm.wala.core/src/com/ibm/wala/ipa/callgraph/propagation/PointerAnalysis.java
com.ibm.wala.core/src/com/ibm/wala/ipa/callgraph/propagation/cfa/ExceptionReturnValueKey.java
fixes for crashes in correlartion tracking
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/ipa/callgraph/correlations/extraction/ClosureExtractor.java
fixes for Dalvik IR generation
com.ibm.wala.core/src/com/ibm/wala/cfg/BytecodeCFG.java
com.ibm.wala.core/src/com/ibm/wala/cfg/ShrikeCFG.java
com.ibm.wala.core/src/com/ibm/wala/ssa/SSACFG.java
com.ibm.wala.dalvik.test/source/com/ibm/wala/dalvik/drivers/APKCallGraphDriver.java
com.ibm.wala.dalvik.test/source/com/ibm/wala/dalvik/test/callGraph/JVMLDalvikComparison.java
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/classLoader/DexCFG.java
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/dex/instructions/UnaryOperation.java
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/ssa/AbstractIntRegisterMachine.java
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/ssa/DexSSABuilder.java
fixes to stack map generation when instrumenting for Java 7
com.ibm.wala.shrike/src/com/ibm/wala/shrike/cg/DynamicCallGraph.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/ConstantInstruction.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/analysis/Analyzer.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/analysis/ClassHierarchy.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/analysis/Verifier.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/shrikeCT/ClassInstrumenter.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeCT/StackMapConstants.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeCT/StackMapTableReader.java
com.ibm.wala.shrike/src/com/ibm/wala/shrikeCT/StackMapTableWriter.java
Julian Dolby
Ben Liblit
Manu Sridharan
Manu Sridharan
Julian Dolby
Manu Sridharan
Juergen Graf