diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java index 376e53cc6..4e5953975 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java @@ -52,6 +52,10 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.OptionalDouble; +import java.util.OptionalInt; +import java.util.OptionalLong; import java.util.Set; import javax.annotation.Nullable; import javax.lang.model.element.VariableElement; @@ -173,9 +177,30 @@ public final class SafetyPropagationTransfer implements ForwardTransferFunction< .namedAnyOf("of", "copyOf"), MethodMatchers.staticMethod().onClass(Arrays.class.getName()).named("asList")); + private static final Matcher<ExpressionTree> OPTIONAL_FACTORIES = Matchers.anyOf( + MethodMatchers.staticMethod().onClass(Optional.class.getName()).namedAnyOf("of", "ofNullable"), + MethodMatchers.staticMethod() + .onClassAny( + OptionalInt.class.getName(), OptionalLong.class.getName(), OptionalDouble.class.getName()) + .named("of")); + // These methods do not take the receiver (generally a static class) into account, only the inputs. private static final Matcher<ExpressionTree> RETURNS_SAFETY_COMBINATION_OF_ARGS = - Matchers.anyOf(STRING_FORMAT, OBJECTS_TO_STRING, IMMUTABLE_COLLECTION_FACTORY); + Matchers.anyOf(STRING_FORMAT, OBJECTS_TO_STRING, IMMUTABLE_COLLECTION_FACTORY, OPTIONAL_FACTORIES); + + private static final Matcher<ExpressionTree> OPTIONAL_ACCESSORS = Matchers.anyOf( + MethodMatchers.instanceMethod() + .onDescendantOf(Optional.class.getName()) + .namedAnyOf("get", "orElseThrow", "stream"), + MethodMatchers.instanceMethod() + .onDescendantOf(OptionalInt.class.getName()) + .namedAnyOf("getAsInt", "orElseThrow"), + MethodMatchers.instanceMethod() + .onDescendantOf(OptionalLong.class.getName()) + .namedAnyOf("getAsLong", "orElseThrow"), + MethodMatchers.instanceMethod() + .onDescendantOf(OptionalDouble.class.getName()) + .namedAnyOf("getAsDouble", "orElseThrow")); // Returns the safety of the receiver, e.g. myString.getBytes() returns the safety of myString. private static final Matcher<ExpressionTree> RETURNS_SAFETY_OF_RECEIVER = Matchers.anyOf( @@ -190,7 +215,8 @@ public final class SafetyPropagationTransfer implements ForwardTransferFunction< .namedAnyOf("toArray", "stream", "parallelStream"), MethodMatchers.instanceMethod() .onDescendantOf(Iterable.class.getName()) - .namedAnyOf("toArray", "iterator", "spliterator")); + .namedAnyOf("toArray", "iterator", "spliterator"), + OPTIONAL_ACCESSORS); private static final Matcher<ExpressionTree> RETURNS_SAFETY_OF_FIRST_ARG = Matchers.anyOf( MethodMatchers.staticMethod().onClass(Objects.class.getName()).named("requireNonNull"), diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java index 6fdec81a0..be355cb68 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java @@ -914,6 +914,44 @@ public void disagreeingSafetyAnnotations() { .doTest(); } + @Test + public void testOptionalUnwrapping() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "import java.util.*;", + "class Test {", + " void f(@Unsafe Optional<String> p) {", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " fun(p);", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " fun(p.get());", + " }", + " void fun(@Safe Object in) {}", + "}") + .doTest(); + } + + @Test + public void testOptionalUnsafeType() { + helper().addSourceLines( + "Test.java", + "import com.palantir.logsafe.*;", + "import java.util.*;", + "class Test {", + " @Unsafe static class UnsafeClass {}", + " void f(Optional<UnsafeClass> p) {", + " // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' " + + "but the parameter requires 'SAFE'.", + " fun(p.get());", + " }", + " void fun(@Safe Object in) {}", + "}") + .doTest(); + } + @Test public void testSafeArgOfUnsafe_recommendsUnsafeArgOf() { refactoringHelper()