diff --git a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java b/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java index ade0791b7..3455252dd 100644 --- a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java @@ -68,6 +68,8 @@ public abstract class AbstractConfig implements Config { protected boolean isAcknowledgeRestrictive; + protected boolean checkOptionalEmptiness; + protected boolean assertsEnabled; /** @@ -182,6 +184,11 @@ public boolean acknowledgeRestrictiveAnnotations() { return isAcknowledgeRestrictive; } + @Override + public boolean checkOptionalEmptiness() { + return checkOptionalEmptiness; + } + @Override public boolean assertsEnabled() { return assertsEnabled; diff --git a/nullaway/src/main/java/com/uber/nullaway/Config.java b/nullaway/src/main/java/com/uber/nullaway/Config.java index 575bb9be4..be7ae6662 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Config.java +++ b/nullaway/src/main/java/com/uber/nullaway/Config.java @@ -107,6 +107,14 @@ public interface Config { */ boolean acknowledgeRestrictiveAnnotations(); + /** + * @return true if Optional Emptiness Handler is to be used. When Optional.get() method is called + * on an empty optional, program will crash with an exception. This handler warns on possible + * cases where Optional.get() call is made on an empty optional. Nullaway determines if an + * optional is non-empty based on Optional.isPresent() call. + */ + boolean checkOptionalEmptiness(); + /** * @return the fully qualified name of a method which will take a @Nullable version of a value and * return an @NonNull copy (likely through an unsafe downcast, but performing runtime checking diff --git a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java index 5d6baab5d..ceeb09117 100644 --- a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java @@ -112,6 +112,11 @@ public boolean acknowledgeRestrictiveAnnotations() { throw new IllegalStateException(error_msg); } + @Override + public boolean checkOptionalEmptiness() { + throw new IllegalStateException(error_msg); + } + @Override @Nullable public String getCastToNonNullMethod() { diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java new file mode 100644 index 000000000..17e6c0f7d --- /dev/null +++ b/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; + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java index 23549901d..e782f9b12 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -55,6 +55,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig { static final String FL_UNANNOTATED_CLASSES = EP_FL_NAMESPACE + ":UnannotatedClasses"; static final String FL_ACKNOWLEDGE_RESTRICTIVE = EP_FL_NAMESPACE + ":AcknowledgeRestrictiveAnnotations"; + static final String FL_CHECK_OPTIONAL_EMPTINESS = EP_FL_NAMESPACE + ":CheckOptionalEmptiness"; static final String FL_SUPPRESS_COMMENT = EP_FL_NAMESPACE + ":AutoFixSuppressionComment"; /** --- JarInfer configs --- */ static final String FL_JI_ENABLED = EP_FL_NAMESPACE + ":JarInferEnabled"; @@ -132,6 +133,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig { isExhaustiveOverride = flags.getBoolean(FL_EXHAUSTIVE_OVERRIDE).orElse(false); isSuggestSuppressions = flags.getBoolean(FL_SUGGEST_SUPPRESSIONS).orElse(false); isAcknowledgeRestrictive = flags.getBoolean(FL_ACKNOWLEDGE_RESTRICTIVE).orElse(false); + checkOptionalEmptiness = flags.getBoolean(FL_CHECK_OPTIONAL_EMPTINESS).orElse(false); treatGeneratedAsUnannotated = flags.getBoolean(FL_GENERATED_UNANNOTATED).orElse(false); assertsEnabled = flags.getBoolean(FL_ASSERTS_ENABLED).orElse(false); fieldAnnotPattern = diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 3fc6e60de..6019081dd 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -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()); } @@ -2325,21 +2331,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 { diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 86fa69e12..1f047413f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -311,7 +311,7 @@ public static boolean isMapPut(Symbol.MethodSymbol symbol, Types types) { * root of an access path; either a variable {@link javax.lang.model.element.Element} or * this (enclosing method receiver) */ - static final class Root { + public static final class Root { /** does this represent the receiver? */ private final boolean isMethodReceiver; diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java index ec1351616..40e20e902 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java @@ -19,6 +19,7 @@ package com.uber.nullaway.dataflow; import com.google.common.collect.ImmutableList; +import com.google.errorprone.VisitorState; import com.sun.source.util.TreePath; import com.sun.tools.javac.util.Context; import com.uber.nullaway.Config; @@ -47,6 +48,8 @@ public final class AccessPathNullnessAnalysis { private final DataFlow dataFlow; + private static String OPTIONAL_PATH = "java.util.Optional"; + // Use #instance to instantiate private AccessPathNullnessAnalysis( Predicate methodReturnsNonNull, @@ -151,11 +154,12 @@ public Set getNonnullStaticFieldsBefore(TreePath path, Context context) * Get nullness info for local variables before some node * * @param path tree path to some AST node within a method / lambda / initializer - * @param context Javac context + * @param state visitor state * @return nullness info for local variables just before the node */ - public NullnessStore getLocalVarInfoBefore(TreePath path, Context context) { - NullnessStore store = dataFlow.resultBefore(path, context, nullnessPropagation); + public NullnessStore getNullnessInfoBeforeNewContext( + TreePath path, VisitorState state, Handler handler) { + NullnessStore store = dataFlow.resultBefore(path, state.context, nullnessPropagation); if (store == null) { return NullnessStore.empty(); } @@ -169,7 +173,8 @@ public NullnessStore getLocalVarInfoBefore(TreePath path, Context context) { || e.getKind().equals(ElementKind.LOCAL_VARIABLE); } } - return false; + + return handler.includeApInfoInSavedContext(ap, state); }); } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java index 0c8756024..784446919 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java @@ -34,7 +34,9 @@ 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; import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; @@ -166,4 +168,15 @@ public void onDataflowVisitLambdaResultExpression( ExpressionTree tree, NullnessStore thenStore, NullnessStore elseStore) { // NoOp } + + @Override + public void onPrepareErrorMessage( + ExpressionTree expr, VisitorState state, ErrorMessage errorMessage) { + // NoOp + } + + @Override + public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { + return false; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java index cc5f41e31..8112b4a4a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -35,7 +35,9 @@ 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; import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; @@ -207,4 +209,21 @@ public void onDataflowVisitLambdaResultExpression( h.onDataflowVisitLambdaResultExpression(tree, thenStore, elseStore); } } + + @Override + public void onPrepareErrorMessage( + ExpressionTree expr, VisitorState state, ErrorMessage errorMessage) { + for (Handler h : handlers) { + h.onPrepareErrorMessage(expr, state, errorMessage); + } + } + + @Override + public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { + boolean shouldFilter = false; + for (Handler h : handlers) { + shouldFilter |= h.includeApInfoInSavedContext(accessPath, state); + } + return shouldFilter; + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java index 527f78a00..4a1c4aefc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java @@ -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; @@ -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)); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java index 823e67936..e86e6307d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -34,8 +34,10 @@ 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; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; @@ -270,6 +272,27 @@ NullnessHint onDataflowVisitMethodInvocation( void onDataflowVisitLambdaResultExpression( ExpressionTree tree, NullnessStore thenStore, NullnessStore elseStore); + /** + * Called while creating the error message on a possible null/empty optional deference. + * + * @param expr The AST node for the expression being matched. + * @param state The current visitor state. + * @param errorMessage error message string and type of the error wrapped in {@link + * com.uber.nullaway.NullAway.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 nullability information for this accesspath should be treated as part of + * the surrounding context when processing a lambda expression or anonymous class declaration. + */ + boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state); + /** * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate * their knowledge of the method return nullability to the the rest of NullAway. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java index a1a9f0b16..797c0a45a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java @@ -50,6 +50,9 @@ public static Handler buildDefault(Config config) { handlerListBuilder.add(new RxNullabilityPropagator()); handlerListBuilder.add(new ContractHandler()); handlerListBuilder.add(new ApacheThriftIsSetHandler()); + if (config.checkOptionalEmptiness()) { + handlerListBuilder.add(new OptionalEmptinessHandler()); + } return new CompositeHandler(handlerListBuilder.build()); } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 026694026..3f54d12d7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -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", "of(T)"), 0) .put(methodRef("java.util.Deque", "addFirst(E)"), 0) .put(methodRef("java.util.Deque", "addLast(E)"), 0) .put(methodRef("java.util.Deque", "offerFirst(E)"), 0) diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java new file mode 100644 index 000000000..454c940e4 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2018 Uber Technologies, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package com.uber.nullaway.handlers; + +import com.google.common.base.Preconditions; +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.Symbol; +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; +import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import java.util.Optional; +import javax.annotation.Nullable; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import org.checkerframework.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.dataflow.cfg.node.Node; + +/** + * Handler to better handle {@code isPresent()} methods in code generated for Optionals. With this + * handler, we learn appropriate Emptiness facts about the relevant property from these calls. + */ +public class OptionalEmptinessHandler extends BaseNoOpHandler { + + private static String OPTIONAL_PATH = "java.util.Optional"; + + @Nullable private Optional optionalType; + + @Override + public boolean onOverrideMayBeNullExpr( + NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) { + if (expr.getKind() == Tree.Kind.METHOD_INVOCATION + && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) { + return true; + } + return exprMayBeNull; + } + + @Override + public void onMatchTopLevelClass( + NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) { + if (optionalType == null) { + optionalType = + Optional.ofNullable(state.getTypeFromString(OPTIONAL_PATH)) + .map(state.getTypes()::erasure); + } + } + + @Override + public NullnessHint onDataflowVisitMethodInvocation( + MethodInvocationNode node, + Types types, + Context context, + AccessPathNullnessPropagation.SubNodeValues inputs, + AccessPathNullnessPropagation.Updates thenUpdates, + AccessPathNullnessPropagation.Updates elseUpdates, + AccessPathNullnessPropagation.Updates bothUpdates) { + Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(node.getTree()); + + if (optionalIsGetCall(symbol, types)) { + return NullnessHint.HINT_NULLABLE; + } else if (optionalIsPresentCall(symbol, types)) { + Element getter = null; + Node base = node.getTarget().getReceiver(); + for (Symbol elem : symbol.owner.getEnclosedElements()) { + if (elem.getKind().equals(ElementKind.METHOD) + && elem.getSimpleName().toString().equals("get")) { + getter = elem; + } + } + updateNonNullAPsForElement(thenUpdates, getter, base); + } + return NullnessHint.UNKNOWN; + } + + @Override + 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(); + // Name of the optional is extracted from the expression + final String message = + "Optional " + + expr.toString().substring(0, exprStringSize - 6) + + " can be empty, dereferenced get() call on it"; + errorMessage.updateErrorMessage(ErrorMessage.MessageTypes.GET_ON_EMPTY_OPTIONAL, message); + } + } + + @Override + public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { + + if (accessPath.getElements().size() == 1) { + AccessPath.Root root = accessPath.getRoot(); + if (!root.isReceiver() && (accessPath.getElements().get(0) instanceof Symbol.MethodSymbol)) { + final Element e = root.getVarElement(); + final Symbol.MethodSymbol g = (Symbol.MethodSymbol) accessPath.getElements().get(0); + return e.getKind().equals(ElementKind.LOCAL_VARIABLE) + && optionalIsGetCall(g, state.getTypes()); + } + } + return false; + } + + private void updateNonNullAPsForElement( + AccessPathNullnessPropagation.Updates updates, @Nullable Element elem, Node base) { + if (elem != null) { + AccessPath ap = AccessPath.fromBaseAndElement(base, elem); + if (ap != null) { + updates.set(ap, Nullness.NONNULL); + } + } + } + + private boolean optionalIsPresentCall(Symbol.MethodSymbol symbol, Types types) { + Preconditions.checkNotNull(optionalType); + // noinspection ConstantConditions + return optionalType.isPresent() + && symbol.getSimpleName().toString().equals("isPresent") + && symbol.getParameters().length() == 0 + && types.isSubtype(symbol.owner.type, optionalType.get()); + } + + private boolean optionalIsGetCall(Symbol.MethodSymbol symbol, Types types) { + Preconditions.checkNotNull(optionalType); + // noinspection ConstantConditions + return optionalType.isPresent() + && symbol.getSimpleName().toString().equals("get") + && symbol.getParameters().length() == 0 + && types.isSubtype(symbol.owner.type, optionalType.get()); + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java index 646014b7e..dc41f53c7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java @@ -1175,6 +1175,99 @@ public void lambdaPlusRestrictiveNegative() { .doTest(); } + @Test + public void OptionalEmptinessHandlerTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated", + "-XepOpt:NullAway:CheckOptionalEmptiness=true")) + .addSourceLines( + "TestNegative.java", + "package com.uber;", + "import java.util.Optional;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Function;", + "public class TestNegative {", + " void foo() {", + " Optional a = Optional.empty();", + " // no error since a.isPresent() is called", + " if(a.isPresent()){", + " a.get().toString();", + " }", + " }", + " public void lambdaConsumer(Function a){", + " return;", + " }", + " void bar() {", + " Optional b = Optional.empty();", + " if(b.isPresent()){", + " lambdaConsumer(v -> b.get().toString());", + " }", + " }", + "}") + .addSourceLines( + "TestPositive.java", + "package com.uber;", + "import java.util.Optional;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Function;", + "public class TestPositive {", + " void foo() {", + " Optional a = Optional.empty();", + " // BUG: Diagnostic contains: Optional a can be empty", + " a.get().toString();", + " }", + " public void lambdaConsumer(Function a){", + " return;", + " }", + " void bar() {", + " Optional b = Optional.empty();", + " // BUG: Diagnostic contains: Optional b can be empty", + " lambdaConsumer(v -> b.get().toString());", + " }", + "}") + .doTest(); + } + + @Test + public void OptionalEmptinessUncheckedTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated")) + .addSourceLines( + "TestNegative.java", + "package com.uber;", + "import java.util.Optional;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Function;", + "public class TestNegative {", + " void foo() {", + " Optional a = Optional.empty();", + " // no error since the handler is not attached", + " a.get().toString();", + " }", + " public void lambdaConsumer(Function a){", + " return;", + " }", + " void bar() {", + " Optional b = Optional.empty();", + " if(b.isPresent()){", + " // no error since the handler is not attached", + " lambdaConsumer(v -> b.get().toString());", + " }", + " }", + "}") + .doTest(); + } + @Test public void testCastToNonNull() { compilationHelper diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayNativeModels.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayNativeModels.java index 01c6adeda..9e11212d4 100644 --- a/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayNativeModels.java +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayNativeModels.java @@ -37,6 +37,7 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; import javax.lang.model.element.Element; @@ -229,6 +230,8 @@ static void nonNullParameters() { File f = new File(s); // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required URLClassLoader.newInstance(null, NullAwayNativeModels.class.getClassLoader()); + // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required + Optional op = Optional.of(null); } static void elementStuff(Element e, Elements elems) {