Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Optional emptiness handler #278

Merged
merged 6 commits into from Mar 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java
@@ -0,0 +1,33 @@
package com.uber.nullaway;

/** Contains error message string to be displayed and the message type from {@link MessageTypes}. */
public class ErrorMessage {
MessageTypes messageType;
String message;

ErrorMessage(MessageTypes messageType, String message) {
this.messageType = messageType;
this.message = message;
}

public void updateErrorMessage(MessageTypes messageType, String message) {
this.messageType = messageType;
this.message = message;
}

public enum MessageTypes {
DEREFERENCE_NULLABLE,
RETURN_NULLABLE,
PASS_NULLABLE,
ASSIGN_FIELD_NULLABLE,
WRONG_OVERRIDE_RETURN,
WRONG_OVERRIDE_PARAM,
METHOD_NO_INIT,
FIELD_NO_INIT,
UNBOX_NULLABLE,
NONNULL_FIELD_READ_BEFORE_INIT,
ANNOTATION_VALUE_INVALID,
CAST_TO_NONNULL_ARG_NONNULL,
GET_ON_EMPTY_OPTIONAL;
}
}
41 changes: 7 additions & 34 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Expand Up @@ -88,6 +88,7 @@
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.processing.JavacProcessingEnvironment;
import com.sun.tools.javac.tree.JCTree;
import com.uber.nullaway.ErrorMessage.MessageTypes;
import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis;
import com.uber.nullaway.dataflow.EnclosingEnvironmentNullness;
import com.uber.nullaway.handlers.Handler;
Expand Down Expand Up @@ -366,7 +367,7 @@ private void updateEnvironmentMapping(Tree tree, VisitorState state) {
// 2. we keep info on all locals rather than just effectively final ones for simplicity
EnclosingEnvironmentNullness.instance(state.context)
.addEnvironmentMapping(
tree, analysis.getLocalVarInfoBefore(state.getPath(), state, handler));
tree, analysis.getNullnessInfoBeforeNewContext(state.getPath(), state, handler));
}

private Symbol.MethodSymbol getSymbolOfSuperConstructor(
Expand Down Expand Up @@ -1937,7 +1938,7 @@ private Description matchDereference(
"dereferenced expression " + baseExpression.toString() + " is @Nullable";
ErrorMessage errorMessage = new ErrorMessage(MessageTypes.DEREFERENCE_NULLABLE, message);

handler.getErrorMessage(baseExpression, state, errorMessage);
handler.onPrepareErrorMessage(baseExpression, state, errorMessage);

return createErrorDescriptionForNullAssignment(
errorMessage.messageType,
Expand All @@ -1960,7 +1961,10 @@ private Description matchDereference(
* @return the error description
*/
private Description createErrorDescription(
MessageTypes errorType, Tree errorLocTree, String message, TreePath path) {
com.uber.nullaway.ErrorMessage.MessageTypes errorType,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ErrorMessage.MessageTypes, no? Rather than the FQN.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

Tree errorLocTree,
String message,
TreePath path) {
Tree enclosingSuppressTree = suppressibleNode(path);
return createErrorDescription(errorType, errorLocTree, message, enclosingSuppressTree);
}
Expand Down Expand Up @@ -2330,37 +2334,6 @@ public void setComputedNullness(ExpressionTree e, Nullness nullness) {
computedNullnessMap.put(e, nullness);
}

public enum MessageTypes {
DEREFERENCE_NULLABLE,
RETURN_NULLABLE,
PASS_NULLABLE,
ASSIGN_FIELD_NULLABLE,
WRONG_OVERRIDE_RETURN,
WRONG_OVERRIDE_PARAM,
METHOD_NO_INIT,
FIELD_NO_INIT,
UNBOX_NULLABLE,
NONNULL_FIELD_READ_BEFORE_INIT,
ANNOTATION_VALUE_INVALID,
CAST_TO_NONNULL_ARG_NONNULL,
GET_ON_EMPTY_OPTIONAL;
}

public static class ErrorMessage {
MessageTypes messageType;
String message;

ErrorMessage(MessageTypes messageType, String message) {
this.messageType = messageType;
this.message = message;
}

public void updateErrorMessage(MessageTypes messageType, String message) {
this.messageType = messageType;
this.message = message;
}
}

@AutoValue
abstract static class FieldInitEntities {

Expand Down
Expand Up @@ -157,7 +157,8 @@ public Set<Element> getNonnullStaticFieldsBefore(TreePath path, Context context)
* @param state visitor state
* @return nullness info for local variables just before the node
*/
public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state, Handler handler) {
public NullnessStore getNullnessInfoBeforeNewContext(
TreePath path, VisitorState state, Handler handler) {
NullnessStore store = dataFlow.resultBefore(path, state.context, nullnessPropagation);
if (store == null) {
return NullnessStore.empty();
Expand All @@ -173,14 +174,14 @@ public NullnessStore getLocalVarInfoBefore(TreePath path, VisitorState state, Ha
}
}

return handler.filterApForLocalVarInfoBefore(ap, state);
return handler.includeApInfoInSavedContext(ap, state);
});
}

/**
* @param path tree path of static method, or initializer block
* @param context Javac context
* @return fields guaranteed to be nonnull at exit of static method (or initialize r block)
* @return fields guaranteed to be nonnull at exit of static method (or initializer block)
*/
public Set<Element> getNonnullStaticFieldsAtExit(TreePath path, Context context) {
NullnessStore nullnessResult = dataFlow.finalResult(path, context, nullnessPropagation);
Expand Down
Expand Up @@ -34,6 +34,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.dataflow.AccessPath;
import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
Expand Down Expand Up @@ -169,13 +170,13 @@ public void onDataflowVisitLambdaResultExpression(
}

@Override
public void getErrorMessage(
ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
public void onPrepareErrorMessage(
ExpressionTree expr, VisitorState state, ErrorMessage errorMessage) {
// NoOp
}

@Override
public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) {
return false;
}
}
Expand Up @@ -35,6 +35,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.dataflow.AccessPath;
import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
Expand Down Expand Up @@ -210,18 +211,18 @@ public void onDataflowVisitLambdaResultExpression(
}

@Override
public void getErrorMessage(
ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
public void onPrepareErrorMessage(
ExpressionTree expr, VisitorState state, ErrorMessage errorMessage) {
for (Handler h : handlers) {
h.getErrorMessage(expr, state, errorMessage);
h.onPrepareErrorMessage(expr, state, errorMessage);
}
}

@Override
public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) {
boolean shouldFilter = false;
for (Handler h : handlers) {
shouldFilter |= h.filterApForLocalVarInfoBefore(accessPath, state);
shouldFilter |= h.includeApInfoInSavedContext(accessPath, state);
}
return shouldFilter;
}
Expand Down
Expand Up @@ -30,6 +30,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.Nullness;
import com.uber.nullaway.dataflow.AccessPath;
Expand Down Expand Up @@ -221,7 +222,10 @@ private void reportMatch(Tree errorLocTree, String message) {
if (this.analysis != null && this.state != null) {
this.state.reportMatch(
this.analysis.createErrorDescription(
NullAway.MessageTypes.ANNOTATION_VALUE_INVALID, errorLocTree, message, errorLocTree));
ErrorMessage.MessageTypes.ANNOTATION_VALUE_INVALID,
errorLocTree,
message,
errorLocTree));
}
}

Expand Down
Expand Up @@ -34,6 +34,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.Nullness;
import com.uber.nullaway.dataflow.AccessPath;
Expand Down Expand Up @@ -279,18 +280,18 @@ void onDataflowVisitLambdaResultExpression(
* @param errorMessage error message string and type of the error wrapped in {@link
* com.uber.nullaway.NullAway.ErrorMessage}.
*/
void getErrorMessage(ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage);
void onPrepareErrorMessage(ExpressionTree expr, VisitorState state, ErrorMessage errorMessage);

/**
* Called when the store access paths are filtered for local variable information before an
* expression.
*
* @param accessPath The access path that needs to be checked if filtered.
* @param state The current visitor state.
* @return true if the accesspath should be filtered to be included in the local variable
* information.
* @return true if the nullability information for this accesspath should be treated as part of
* the surrounding context when processing a lambda expression or anonymous class declaration.
*/
boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state);
boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state);

/**
* A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate
Expand Down
Expand Up @@ -31,6 +31,7 @@
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Context;
import com.uber.nullaway.ErrorMessage;
import com.uber.nullaway.NullAway;
import com.uber.nullaway.Nullness;
import com.uber.nullaway.dataflow.AccessPath;
Expand Down Expand Up @@ -100,8 +101,8 @@ public NullnessHint onDataflowVisitMethodInvocation(
}

@Override
public void getErrorMessage(
ExpressionTree expr, VisitorState state, NullAway.ErrorMessage errorMessage) {
public void onPrepareErrorMessage(
ExpressionTree expr, VisitorState state, ErrorMessage errorMessage) {
if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
&& optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) {
final int exprStringSize = expr.toString().length();
Expand All @@ -110,12 +111,12 @@ && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.get
"Optional "
+ expr.toString().substring(0, exprStringSize - 6)
+ " can be empty, dereferenced get() call on it";
errorMessage.updateErrorMessage(NullAway.MessageTypes.GET_ON_EMPTY_OPTIONAL, message);
errorMessage.updateErrorMessage(ErrorMessage.MessageTypes.GET_ON_EMPTY_OPTIONAL, message);
}
}

@Override
public boolean filterApForLocalVarInfoBefore(AccessPath accessPath, VisitorState state) {
public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) {

if (accessPath.getElements().size() == 1) {
AccessPath.Root root = accessPath.getRoot();
Expand Down