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
Changes from 5 commits
fd0fadd
884528b
f639ed2
c7bf3d0
a155e0e
a521ff0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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.context)); | ||
tree, analysis.getNullnessInfoBeforeNewContext(state.getPath(), state, handler)); | ||
} | ||
|
||
private Symbol.MethodSymbol getSymbolOfSuperConstructor( | ||
|
@@ -1933,11 +1934,16 @@ private Description matchDereference( | |
return Description.NO_MATCH; | ||
} | ||
if (mayBeNullExpr(state, baseExpression)) { | ||
String message = "dereferenced expression " + baseExpression.toString() + " is @Nullable"; | ||
final String message = | ||
"dereferenced expression " + baseExpression.toString() + " is @Nullable"; | ||
ErrorMessage errorMessage = new ErrorMessage(MessageTypes.DEREFERENCE_NULLABLE, message); | ||
|
||
handler.onPrepareErrorMessage(baseExpression, state, errorMessage); | ||
|
||
return createErrorDescriptionForNullAssignment( | ||
MessageTypes.DEREFERENCE_NULLABLE, | ||
errorMessage.messageType, | ||
derefExpression, | ||
message, | ||
errorMessage.message, | ||
baseExpression, | ||
state.getPath()); | ||
} | ||
|
@@ -1955,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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
@@ -2325,21 +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; | ||
} | ||
|
||
@AutoValue | ||
abstract static class FieldInitEntities { | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -289,6 +289,7 @@ private static class DefaultLibraryModels implements LibraryModels { | |
"javax.lang.model.util.Elements", | ||
"getDocComment(javax.lang.model.element.Element)"), | ||
0) | ||
.put(methodRef("java.util.Optional", "<T>of(T)"), 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
.put(methodRef("java.util.Deque", "addFirst(E)"), 0) | ||
.put(methodRef("java.util.Deque", "addLast(E)"), 0) | ||
.put(methodRef("java.util.Deque", "offerFirst(E)"), 0) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably change all the
createErrorDescription[...]
methods to just takeErrorMessage
rather than the separate components. But let's leave that for a follow up PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely.