DASCA
/
WALA
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix Eclipse warnings about redundant null checks and assignments 

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.
This commit is contained in:
Ben Liblit 2017-08-05 20:54:42 -05:00 committed by Manu Sridharan
parent 8ebfd7615c
commit cb6d3b282a
21 changed files with 32 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
com.ibm.wala.cast.java/.settings/org.eclipse.jdt.core.prefs
View File

@ -79,7 +79,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=error
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=error
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

12
com.ibm.wala.cast.java/src/com/ibm/wala/cast/java/translator/JavaCAst2IRTranslator.java
View File

@ -216,9 +216,7 @@ public class JavaCAst2IRTranslator extends AstTranslator {
// code bodies, so we may see other things than TYPE_ENTITY here.
IClass owner = loader.lookupClass(makeType(topEntity.getType()).getName());
if (owner == null) {
assert owner != null : makeType(topEntity.getType()).getName() + " not found in " + loader;
}
assert owner != null : makeType(topEntity.getType()).getName() + " not found in " + loader;
((JavaSourceLoaderImpl) loader).defineField(n, owner);
}
@ -231,9 +229,7 @@ public class JavaCAst2IRTranslator extends AstTranslator {
CAstType owningType = methodType.getDeclaringType();
IClass owner = loader.lookupClass(makeType(owningType).getName());
if (owner == null) {
assert owner != null : makeType(owningType).getName().toString() + " not found in " + loader;
}
assert owner != null : makeType(owningType).getName().toString() + " not found in " + loader;
((JavaSourceLoaderImpl) loader).defineAbstractFunction(N, owner);
}
@ -249,9 +245,7 @@ public class JavaCAst2IRTranslator extends AstTranslator {
TypeName typeName = makeType(owningType).getName();
IClass owner = loader.lookupClass(typeName);
if (owner == null) {
assert owner != null : typeName.toString() + " not found in " + loader;
}
assert owner != null : typeName.toString() + " not found in " + loader;
symtab.getConstant(0);
symtab.getNullConstant();

2
com.ibm.wala.cast.js/.settings/org.eclipse.jdt.core.prefs
View File

@ -79,7 +79,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=error
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=error
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

3
com.ibm.wala.cast.js/source/com/ibm/wala/cast/js/ipa/summaries/JavaScriptConstructorFunctions.java
View File

@ -539,8 +539,7 @@ public class JavaScriptConstructorFunctions {
assert fcls != null : "cannot find class for " + fileName;
if (fcls != null)
return makeFunctionConstructor(cls, fcls);
return makeFunctionConstructor(cls, fcls);
} catch (IOException e) {

2
com.ibm.wala.core/.settings/org.eclipse.jdt.core.prefs
View File

@ -89,7 +89,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=ignore
org.eclipse.jdt.core.compiler.problem.rawTypeReference=ignore
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=ignore
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=ignore
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=ignore
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

8
com.ibm.wala.core/src/com/ibm/wala/analysis/pointers/BasicHeapGraph.java
View File

@ -268,9 +268,7 @@ public class BasicHeapGraph<T extends InstanceKey> extends HeapGraphImpl<T> {
InstanceKey I = (InstanceKey) N;
TypeReference T = I.getConcreteType().getReference();
if (T == null) {
assert T != null : "null concrete type from " + I.getClass();
}
assert T != null : "null concrete type from " + I.getClass();
if (T.isArrayType()) {
PointerKey p = getHeapModel().getPointerKeyForArrayContents(I);
if (p == null || !nodeManager.containsNode(p)) {
@ -280,9 +278,7 @@ public class BasicHeapGraph<T extends InstanceKey> extends HeapGraphImpl<T> {
}
} else {
IClass klass = getHeapModel().getClassHierarchy().lookupClass(T);
if (klass == null) {
assert klass != null : "null klass for type " + T;
}
assert klass != null : "null klass for type " + T;
MutableSparseIntSet result = MutableSparseIntSet.makeEmpty();
for (Iterator<IField> it = klass.getAllInstanceFields().iterator(); it.hasNext();) {
IField f = it.next();

2
com.ibm.wala.core/src/com/ibm/wala/cfg/ShrikeCFG.java
View File

@ -355,7 +355,7 @@ public class ShrikeCFG extends AbstractCFG<IInstruction, ShrikeCFG.BasicBlock> i
if (!exceptionTypes.isEmpty()) {
addExceptionalEdgeTo(b);
exceptionTypes.clear();
caughtException = null;
assert caughtException == null;
}
}
if (caughtException != null) {

4
com.ibm.wala.core/src/com/ibm/wala/ipa/callgraph/propagation/ClassBasedInstanceKeys.java
View File

@ -78,9 +78,7 @@ public class ClassBasedInstanceKeys implements InstanceKeyFactory {
if (DEBUG) {
System.err.println(("type: " + type));
}
if (type == null) {
assert type != null : "null type for " + allocation;
}
assert type != null : "null type for " + allocation;
int i = 0;
while (i <= dim) {
i++;

4
com.ibm.wala.core/src/com/ibm/wala/ipa/cha/ClassHierarchy.java
View File

@ -756,9 +756,7 @@ public class ClassHierarchy implements IClassHierarchy {
return b;
} else {
Node n = map.get(b.getReference());
if (n == null) {
assert n != null : "null n for " + b;
}
assert n != null : "null n for " + b;
Set<IClass> superB;
superB = getSuperclasses(b);
IClass aa = a;

4
com.ibm.wala.core/src/com/ibm/wala/ssa/SSAInvokeInstruction.java
View File

@ -65,9 +65,7 @@ public abstract class SSAInvokeInstruction extends SSAAbstractInvokeInstruction
nExpected += site.getDeclaredTarget().getNumberOfParameters();
if (nExpected > 0) {
if (params == null) {
assert params != null : "null params for " + site;
}
assert params != null : "null params for " + site;
if (params.length != nExpected) {
assert params.length == nExpected : "wrong number of params for " + site + " Expected " + nExpected + " got "
+ params.length;

2
com.ibm.wala.dalvik/.settings/org.eclipse.jdt.core.prefs
View File

@ -80,7 +80,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=error
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=error
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

2
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/classLoader/DexCFG.java
View File

@ -350,7 +350,7 @@ public class DexCFG extends AbstractCFG<Instruction, DexCFG.BasicBlock> implemen
if (!exceptionTypes.isEmpty()) {
addExceptionalEdgeTo(b);
exceptionTypes.clear();
caughtException = null;
assert caughtException == null;
}
}
if (caughtException != null) {

2
com.ibm.wala.dalvik/src/com/ibm/wala/dalvik/ipa/callgraph/propagation/cfa/Intent.java
View File

@ -259,7 +259,7 @@ public class Intent implements ContextItem, Comparable<Intent> {
private static boolean isSystemService(Intent intent) {
assert (intent.action != null);
return (intent != null && intent.action != null && (intent.action.getVal(0) != 'L') && (intent.action.rIndex((byte) '/') < 0) && (intent.action.rIndex((byte) '.') < 0));
return (intent.action.getVal(0) != 'L') && (intent.action.rIndex((byte) '/') < 0) && (intent.action.rIndex((byte) '.') < 0);
}
/**

2
com.ibm.wala.ide/.settings/org.eclipse.jdt.core.prefs
View File

@ -79,7 +79,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=warning
org.eclipse.jdt.core.compiler.problem.rawTypeReference=ignore
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=ignore
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=error
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

2
com.ibm.wala.scandroid/.settings/org.eclipse.jdt.core.prefs
View File

@ -65,7 +65,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=error
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=warning
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

8
com.ibm.wala.scandroid/source/org/scandroid/flow/types/FlowType.java
View File

@ -129,15 +129,15 @@ public abstract class FlowType<E extends ISSABasicBlock> {
@SuppressWarnings("unused")
private boolean compareBlocks(BasicBlockInContext<E> a,
BasicBlockInContext<E> b) {
if (null == a || null == b) {
return false;
}
// delegate to the defined implementation, but only if it's true.
if (a.equals(b)) {
return true;
}
if (null == a || null == b) {
return false;
}
if (a.getNumber() != b.getNumber()) {
return false;
}

10
com.ibm.wala.scandroid/source/org/scandroid/util/EntryPoints.java
View File

@ -305,9 +305,8 @@ public class EntryPoints {
for (String[] intent: ActivityIntentList) {
//method = IntentToMethod(intent[0]);
method = "onCreate(Landroid/os/Bundle;)V";
if (method != null)
im = cha.resolveMethod(StringStuff.makeMethodReference(intent[1]+"."+method));
im = cha.resolveMethod(StringStuff.makeMethodReference(intent[1]+"."+method));
if (im!=null)
entries.add(new DefaultEntrypoint(im,cha));
@ -316,9 +315,8 @@ public class EntryPoints {
//Seems that every broadcast receiver can be an entrypoints?
// method = IntentToMethod(intent[0]);
method = "onReceive(Landroid/content/Context;Landroid/content/Intent;)V";
if (method != null)
im = cha.resolveMethod(StringStuff.makeMethodReference(intent[1]+"."+method));
im = cha.resolveMethod(StringStuff.makeMethodReference(intent[1]+"."+method));
if (im!=null)
entries.add(new DefaultEntrypoint(im,cha));
}

2
com.ibm.wala.shrike/.settings/org.eclipse.jdt.core.prefs
View File

@ -89,7 +89,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=warning
org.eclipse.jdt.core.compiler.problem.rawTypeReference=error
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=error
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

2
com.ibm.wala.shrike/src/com/ibm/wala/shrikeBT/shrikeCT/tools/ClassPrinter.java
View File

@ -194,7 +194,7 @@ public class ClassPrinter {
int count = 0;
for (int j = 0; j < map.length; j++) {
String line2 = " " + j + ": " + map[j];
if (line == null || line2 == null || !line2.substring(line2.indexOf(':')).equals(line.substring(line.indexOf(':')))) {
if (line == null || !line2.substring(line2.indexOf(':')).equals(line.substring(line.indexOf(':')))) {
if (count > 1) {
w.write(" (" + count + " times)\n");
} else if (count > 0) {

2
com.ibm.wala.util/.settings/org.eclipse.jdt.core.prefs
View File

@ -79,7 +79,7 @@ org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=warning
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=error
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.redundantNullCheck=error
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=warning
org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=error
org.eclipse.jdt.core.compiler.problem.reportMethodCanBePotentiallyStatic=ignore

5
com.ibm.wala.util/src/com/ibm/wala/util/intset/OrdinalSet.java
View File

@ -109,8 +109,9 @@ public class OrdinalSet<T> implements Iterable<T> {
if ((a == null && b == null) || a == b || (a.mapping == b.mapping && a.S == b.S)) {
return true;
}
if (a != null && b != null && a.size() == b.size()) {
assert a != null && b != null;
if (a.size() == b.size()) {
if (a.mapping == b.mapping || (a.mapping != null && b.mapping != null && a.mapping.equals(b.mapping))) {
return a.S == b.S || (a.S != null && b.S != null && a.S.sameValue(b.S));
}