Skip to content

Commit

Permalink
Merge c485300 into 15b8dca
Browse files Browse the repository at this point in the history
  • Loading branch information
@lazaroclapp
lazaroclapp committed Sep 27, 2018
2 parents 15b8dca + c485300 commit e97b7c1
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 30 deletions.
26 changes: 24 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import com.sun.tools.javac.processing.JavacProcessingEnvironment;
import com.sun.tools.javac.tree.JCTree;
import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis;
import com.uber.nullaway.dataflow.EnclosingEnvironmentNullness;
import com.uber.nullaway.handlers.Handler;
import com.uber.nullaway.handlers.Handlers;
import java.util.ArrayList;
Expand All @@ -105,6 +106,7 @@
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.NestingKind;
import javax.lang.model.type.TypeKind;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.javacutil.AnnotationUtils;
Expand Down Expand Up @@ -322,6 +324,14 @@ public Description matchNewClass(NewClassTree tree, VisitorState state) {
return handleInvocation(tree, state, methodSymbol, actualParams);
}

private void updateEnvironmentMapping(Tree tree, VisitorState state) {
// store environment for use when analyzing methods inside the class / lambda
AccessPathNullnessAnalysis analysis = getNullnessAnalysis(state);
EnclosingEnvironmentNullness.instance(state.context)
.addEnvironmentMapping(
tree, analysis.getLocalVarInfoBefore(state.getPath(), state.context));
}

private Symbol.MethodSymbol getSymbolOfSuperConstructor(
Symbol.MethodSymbol anonClassConstructorSymbol, VisitorState state) {
// get the statements in the body of the anonymous class constructor
Expand Down Expand Up @@ -568,8 +578,12 @@ private Description checkReturnExpression(

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
if (!matchWithinClass) {
return Description.NO_MATCH;
}
Symbol.MethodSymbol funcInterfaceMethod =
NullabilityUtil.getFunctionalInterfaceMethod(tree, state.getTypes());
updateEnvironmentMapping(tree, state);
handler.onMatchLambdaExpression(this, tree, state, funcInterfaceMethod);
if (NullabilityUtil.isUnannotated(funcInterfaceMethod, config)) {
return Description.NO_MATCH;
Expand Down Expand Up @@ -600,6 +614,9 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState
*/
@Override
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
if (!matchWithinClass) {
return Description.NO_MATCH;
}
Symbol.MethodSymbol referencedMethod = ASTHelpers.getSymbol(tree);
Symbol.MethodSymbol funcInterfaceSymbol =
NullabilityUtil.getFunctionalInterfaceMethod(tree, state.getTypes());
Expand Down Expand Up @@ -1051,7 +1068,8 @@ public Description matchClass(ClassTree tree, VisitorState state) {
// we don't want to update the flag for nested classes.
// ideally we would keep a stack of flags to handle nested types,
// but this is not easy within the Error Prone APIs
if (!classSymbol.getNestingKind().isNested()) {
NestingKind nestingKind = classSymbol.getNestingKind();
if (!nestingKind.isNested()) {
matchWithinClass = !isExcludedClass(classSymbol, state);
// since we are processing a new top-level class, invalidate any cached
// results for previous classes
Expand All @@ -1061,9 +1079,13 @@ public Description matchClass(ClassTree tree, VisitorState state) {
class2Entities.clear();
class2ConstructorUninit.clear();
computedNullnessMap.clear();
EnclosingEnvironmentNullness.instance(state.context).clear();
}
if (matchWithinClass) {
checkFieldInitialization(tree, state);
if (nestingKind.equals(NestingKind.LOCAL) || nestingKind.equals(NestingKind.ANONYMOUS)) {
updateEnvironmentMapping(tree, state);
}
}
return Description.NO_MATCH;
}
Expand Down Expand Up @@ -1801,7 +1823,7 @@ public boolean nullnessFromDataflow(VisitorState state, ExpressionTree expr) {

public AccessPathNullnessAnalysis getNullnessAnalysis(VisitorState state) {
return AccessPathNullnessAnalysis.instance(
state.context, nonAnnotatedMethod, state.getTypes(), config, this.handler);
state.context, nonAnnotatedMethod, config, this.handler);
}

private boolean mayBeNullFieldAccess(VisitorState state, ExpressionTree expr, Symbol exprSymbol) {
Expand Down
22 changes: 12 additions & 10 deletions nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,25 @@ public static Symbol.ClassSymbol getOutermostClassSymbol(Symbol symbol) {
*/
@Nullable
public static TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
while (path != null) {
if (path.getLeaf() instanceof MethodTree || path.getLeaf() instanceof LambdaExpressionTree) {
return path;
TreePath curPath = path.getParentPath();
while (curPath != null) {
if (curPath.getLeaf() instanceof MethodTree
|| curPath.getLeaf() instanceof LambdaExpressionTree) {
return curPath;
}
TreePath parent = path.getParentPath();
TreePath parent = curPath.getParentPath();
if (parent != null && parent.getLeaf() instanceof ClassTree) {
if (path.getLeaf() instanceof BlockTree) {
if (curPath.getLeaf() instanceof BlockTree) {
// found initializer block
return path;
return curPath;
}
if (path.getLeaf() instanceof VariableTree
&& ((VariableTree) path.getLeaf()).getInitializer() != null) {
if (curPath.getLeaf() instanceof VariableTree
&& ((VariableTree) curPath.getLeaf()).getInitializer() != null) {
// found field with an inline initializer
return path;
return curPath;
}
}
path = parent;
curPath = parent;
}
return null;
}
Expand Down
28 changes: 22 additions & 6 deletions nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import com.google.common.collect.ImmutableList;
import com.sun.source.util.TreePath;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.Config;
import com.uber.nullaway.Nullness;
Expand Down Expand Up @@ -51,32 +50,30 @@ public final class AccessPathNullnessAnalysis {
// Use #instance to instantiate
private AccessPathNullnessAnalysis(
Predicate<MethodInvocationNode> methodReturnsNonNull,
Types types,
Context context,
Config config,
Handler handler) {
this.nullnessPropagation =
new AccessPathNullnessPropagation(
Nullness.NONNULL, methodReturnsNonNull, types, config, handler);
Nullness.NONNULL, methodReturnsNonNull, context, config, handler);
this.dataFlow = new DataFlow();
}

/**
* @param context Javac context
* @param methodReturnsNonNull predicate determining whether a method is assumed to return NonNull
* value
* @param types javac Types data structure
* @param config analysis config
* @return instance of the analysis
*/
public static AccessPathNullnessAnalysis instance(
Context context,
Predicate<MethodInvocationNode> methodReturnsNonNull,
Types types,
Config config,
Handler handler) {
AccessPathNullnessAnalysis instance = context.get(FIELD_NULLNESS_ANALYSIS_KEY);
if (instance == null) {
instance = new AccessPathNullnessAnalysis(methodReturnsNonNull, types, config, handler);
instance = new AccessPathNullnessAnalysis(methodReturnsNonNull, context, config, handler);
context.put(FIELD_NULLNESS_ANALYSIS_KEY, instance);
}
return instance;
Expand Down Expand Up @@ -150,6 +147,25 @@ public Set<Element> getNonnullStaticFieldsBefore(TreePath path, Context context)
return getNonnullStaticFields(store);
}

public NullnessStore getLocalVarInfoBefore(TreePath path, Context context) {
NullnessStore store = dataFlow.resultBefore(path, context, nullnessPropagation);
if (store == null) {
return NullnessStore.empty();
}
return store.filterAccessPaths(
(ap) -> {
if (ap.getElements().size() == 0) {
AccessPath.Root root = ap.getRoot();
if (!root.isReceiver()) {
Element e = root.getVarElement();
return e.getKind().equals(ElementKind.PARAMETER)
|| e.getKind().equals(ElementKind.LOCAL_VARIABLE);
}
}
return false;
});
}

/**
* @param path tree path of static method, or initializer block
* @param context Javac context
Expand Down
63 changes: 54 additions & 9 deletions nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@

import com.google.common.base.Preconditions;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.Trees;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.processing.JavacProcessingEnvironment;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.Config;
import com.uber.nullaway.NullabilityUtil;
import com.uber.nullaway.Nullness;
Expand All @@ -37,10 +41,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;
import javax.annotation.CheckReturnValue;
import javax.annotation.Nullable;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.NestingKind;
import javax.lang.model.element.VariableElement;
import org.checkerframework.dataflow.analysis.ConditionalTransferResult;
import org.checkerframework.dataflow.analysis.RegularTransferResult;
Expand Down Expand Up @@ -135,6 +142,8 @@ public class AccessPathNullnessPropagation implements TransferFunction<Nullness,

private final Predicate<MethodInvocationNode> methodReturnsNonNull;

private final Context context;

private final Types types;

private final Config config;
Expand All @@ -144,12 +153,13 @@ public class AccessPathNullnessPropagation implements TransferFunction<Nullness,
AccessPathNullnessPropagation(
Nullness defaultAssumption,
Predicate<MethodInvocationNode> methodReturnsNonNull,
Types types,
Context context,
Config config,
Handler handler) {
this.defaultAssumption = defaultAssumption;
this.methodReturnsNonNull = methodReturnsNonNull;
this.types = types;
this.context = context;
this.types = Types.instance(context);
this.config = config;
this.handler = handler;
}
Expand Down Expand Up @@ -177,21 +187,27 @@ private static Node unwrapAssignExpr(Node node) {
public NullnessStore initialStore(
UnderlyingAST underlyingAST, List<LocalVariableNode> parameters) {
if (parameters == null) {
// Documentation of this method states, "parameters is only set if the underlying AST is a
// method"
return NullnessStore.empty();
// not a method
UnderlyingAST.CFGStatement ast = (UnderlyingAST.CFGStatement) underlyingAST;
return getEnvNullnessStoreForClass(ast.getClassTree());
}
boolean isLambda = underlyingAST.getKind().equals(UnderlyingAST.Kind.LAMBDA);
if (isLambda) {
return lambdaInitialStore((UnderlyingAST.CFGLambda) underlyingAST, parameters);
} else {
return methodInitialStore(underlyingAST, parameters);
return methodInitialStore((UnderlyingAST.CFGMethod) underlyingAST, parameters);
}
}

private NullnessStore lambdaInitialStore(
UnderlyingAST.CFGLambda underlyingAST, List<LocalVariableNode> parameters) {
NullnessStore.Builder result = NullnessStore.empty().toBuilder();
// start from environment
EnclosingEnvironmentNullness environmentNullness =
EnclosingEnvironmentNullness.instance(context);
NullnessStore environmentMapping =
Objects.requireNonNull(
environmentNullness.getEnvironmentMapping(underlyingAST.getLambdaTree()));
NullnessStore.Builder result = environmentMapping.toBuilder();
LambdaExpressionTree code = underlyingAST.getLambdaTree();
// need to check annotation for i'th parameter of functional interface declaration
Symbol.MethodSymbol fiMethodSymbol = NullabilityUtil.getFunctionalInterfaceMethod(code, types);
Expand Down Expand Up @@ -225,8 +241,10 @@ private NullnessStore lambdaInitialStore(
}

private NullnessStore methodInitialStore(
UnderlyingAST underlyingAST, List<LocalVariableNode> parameters) {
NullnessStore.Builder result = NullnessStore.empty().toBuilder();
UnderlyingAST.CFGMethod underlyingAST, List<LocalVariableNode> parameters) {
ClassTree classTree = underlyingAST.getClassTree();
NullnessStore envStore = getEnvNullnessStoreForClass(classTree);
NullnessStore.Builder result = envStore.toBuilder();
for (LocalVariableNode param : parameters) {
Element element = param.getElement();
Nullness assumed = Nullness.hasNullableAnnotation((Symbol) element) ? NULLABLE : NONNULL;
Expand All @@ -236,6 +254,33 @@ private NullnessStore methodInitialStore(
return result.build();
}

private NullnessStore getEnvNullnessStoreForClass(ClassTree classTree) {
NullnessStore envStore = NullnessStore.empty();
ClassTree enclosingLocalOrAnonymous = findEnclosingLocalOrAnonymousClass(classTree);
if (enclosingLocalOrAnonymous != null) {
EnclosingEnvironmentNullness environmentNullness =
EnclosingEnvironmentNullness.instance(context);
envStore =
Objects.requireNonNull(
environmentNullness.getEnvironmentMapping(enclosingLocalOrAnonymous));
}
return envStore;
}

@Nullable
private ClassTree findEnclosingLocalOrAnonymousClass(ClassTree classTree) {
Symbol.ClassSymbol symbol = ASTHelpers.getSymbol(classTree);
while (symbol.getNestingKind().isNested()) {
if (symbol.getNestingKind().equals(NestingKind.ANONYMOUS)
|| symbol.getNestingKind().equals(NestingKind.LOCAL)) {
return Trees.instance(JavacProcessingEnvironment.instance(context)).getTree(symbol);
} else {
symbol = symbol.owner.enclClass();
}
}
return null;
}

@Override
public TransferResult<Nullness, NullnessStore> visitShortLiteral(
ShortLiteralNode shortLiteralNode, TransferInput<Nullness, NullnessStore> input) {
Expand Down
22 changes: 20 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/dataflow/DataFlow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
Expand Down Expand Up @@ -98,7 +99,8 @@ public ControlFlowGraph load(CfgParams key) {
bodyPath = new TreePath(codePath, lambdaExpressionTree.getBody());
} else if (codePath.getLeaf() instanceof MethodTree) {
MethodTree method = (MethodTree) codePath.getLeaf();
ast = new UnderlyingAST.CFGMethod(method, /*classTree*/ null);
ClassTree enclClass = ASTHelpers.findEnclosingNode(codePath, ClassTree.class);
ast = new UnderlyingAST.CFGMethod(method, enclClass);
BlockTree body = method.getBody();
if (body == null) {
throw new IllegalStateException(
Expand Down Expand Up @@ -195,6 +197,18 @@ S resultBeforeExpr(TreePath exprPath, Context context, T transfer) {
return analysisResult == null ? null : analysisResult.getStoreBefore(exprPath.getLeaf());
}

/**
* like {@link #resultBeforeExpr(TreePath, Context, TransferFunction)} but for an arbitrary Tree
* in a method. A bit riskier to use since we don't check that there is a corresponding CFG node
* to the Tree; use with care.
*/
@Nullable
public <A extends AbstractValue<A>, S extends Store<S>, T extends TransferFunction<A, S>>
S resultBefore(TreePath exprPath, Context context, T transfer) {
AnalysisResult<A, S> analysisResult = resultFor(exprPath, context, transfer);
return analysisResult == null ? null : analysisResult.getStoreBefore(exprPath.getLeaf());
}

@Nullable
private <A extends AbstractValue<A>, S extends Store<S>, T extends TransferFunction<A, S>>
AnalysisResult<A, S> resultForExpr(TreePath exprPath, Context context, T transfer) {
Expand All @@ -204,7 +218,11 @@ AnalysisResult<A, S> resultForExpr(TreePath exprPath, Context context, T transfe
"Leaf of exprPath must be of type ExpressionTree, but was %s",
leaf.getClass().getName());

final ExpressionTree expr = (ExpressionTree) leaf;
return resultFor(exprPath, context, transfer);
}

private <A extends AbstractValue<A>, S extends Store<S>, T extends TransferFunction<A, S>>
AnalysisResult<A, S> resultFor(TreePath exprPath, Context context, T transfer) {
final TreePath enclosingPath =
NullabilityUtil.findEnclosingMethodOrLambdaOrInitializer(exprPath);
if (enclosingPath == null) {
Expand Down
Loading

0 comments on commit e97b7c1

Please sign in to comment.