Support subclass safety overrides

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgument.java

@@ -30,6 +30,7 @@
 import com.palantir.baseline.errorprone.safety.SafetyAnalysis;
 import com.palantir.baseline.errorprone.safety.SafetyAnnotations;
 import com.sun.source.tree.AssignmentTree;
+import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.CompoundAssignmentTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
@@ -72,7 +73,8 @@ public final class IllegalSafeLoggingArgument extends BugChecker
                 BugChecker.CompoundAssignmentTreeMatcher,
                 BugChecker.MethodTreeMatcher,
                 BugChecker.VariableTreeMatcher,
-                BugChecker.NewClassTreeMatcher {
+                BugChecker.NewClassTreeMatcher,
+                BugChecker.ClassTreeMatcher {
 
     private static final String UNSAFE_ARG = "com.palantir.logsafe.UnsafeArg";
     private static final Matcher<ExpressionTree> SAFE_ARG_OF_METHOD_MATCHER = MethodMatchers.staticMethod()
@@ -339,4 +341,21 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
                         variableTypeSafety, parameterSafetyAnnotation))
                 .build();
     }
+
+    @Override
+    public Description matchClass(ClassTree tree, VisitorState state) {
+        Safety directSafety = SafetyAnnotations.getDirectSafety(ASTHelpers.getSymbol(tree), state);
+        if (directSafety.allowsAll()) {
+            return Description.NO_MATCH;
+        }
+        Safety ancestorSafety = SafetyAnnotations.getTypeSafetyFromAncestors(tree, state);
+        if (directSafety.allowsValueWith(ancestorSafety)) {
+            return Description.NO_MATCH;
+        }
+        return buildDescription(tree)
+                .setMessage(String.format(
+                        "Dangerous type: annotated '%s' but ancestors declare '%s'.",
+                        directSafety, SafetyAnnotations.getSafety(tree, state)))
+                .build();
+    }
 }

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/MoreASTHelpers.java

@@ -22,6 +22,8 @@
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.CatchTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.Tree;
 import com.sun.source.tree.TryTree;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
@@ -32,7 +34,7 @@
 
 /** Utility functionality that does not exist in {@link com.google.errorprone.util.ASTHelpers}. */
 @SuppressWarnings("checkstyle:AbbreviationAsWordInName")
-final class MoreASTHelpers {
+public final class MoreASTHelpers {
 
     /** Removes any type that is a subtype of another type in the set. */
     @SuppressWarnings("ReferenceEquality")
@@ -115,5 +117,11 @@ static ImmutableList<Type> expandUnion(@Nullable Type type) {
         return ImmutableList.of(type);
     }
 
+    public static Type getResultType(Tree tree) {
+        return tree instanceof ExpressionTree
+                ? ASTHelpers.getResultType((ExpressionTree) tree)
+                : ASTHelpers.getType(tree);
+    }
+
     private MoreASTHelpers() {}
 }

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/SafeLoggingPropagation.java

@@ -183,7 +183,7 @@ private static List<VarSymbol> getRecordComponents(ClassSymbol classSymbol) {
 
     private Description matchRecord(ClassTree classTree, ClassSymbol classSymbol, VisitorState state) {
         Safety existingClassSafety = SafetyAnnotations.getSafety(classTree, state);
-        Safety safety = getTypeSafetyFromAncestors(classTree, state);
+        Safety safety = SafetyAnnotations.getTypeSafetyFromAncestors(classTree, state);
         for (VarSymbol recordComponent : getRecordComponents(classSymbol)) {
             Safety symbolSafety = SafetyAnnotations.getSafety(recordComponent, state);
             Safety typeSymSafety = SafetyAnnotations.getSafety(recordComponent.type.tsym, state);
@@ -203,7 +203,7 @@ private Description matchClassOrInterface(ClassTree classTree, ClassSymbol class
     @SuppressWarnings("checkstyle:CyclomaticComplexity")
     private Description matchImmutables(ClassTree classTree, ClassSymbol classSymbol, VisitorState state) {
         Safety existingClassSafety = SafetyAnnotations.getSafety(classTree, state);
-        Safety safety = getTypeSafetyFromAncestors(classTree, state);
+        Safety safety = SafetyAnnotations.getTypeSafetyFromAncestors(classTree, state);
         boolean hasKnownGetter = false;
         boolean isJson = hasJacksonAnnotation(classSymbol, state);
         for (Tree member : classTree.getMembers()) {
@@ -251,14 +251,6 @@ private Description matchBasedOnToString(ClassTree classTree, ClassSymbol classS
         return handleSafety(classTree, classTree.getModifiers(), state, existingClassSafety, symbolSafety);
     }
 
-    private static Safety getTypeSafetyFromAncestors(ClassTree classTree, VisitorState state) {
-        Safety safety = SafetyAnnotations.getSafety(classTree.getExtendsClause(), state);
-        for (Tree implemented : classTree.getImplementsClause()) {
-            safety = safety.leastUpperBound(SafetyAnnotations.getSafety(implemented, state));
-        }
-        return safety;
-    }
-
     private Description handleSafety(
             Tree tree, ModifiersTree treeModifiers, VisitorState state, Safety existingSafety, Safety computedSafety) {
         if (existingSafety != Safety.UNKNOWN && existingSafety.allowsValueWith(computedSafety)) {

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java

@@ -20,6 +20,7 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.suppliers.Suppliers;
 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.Attribute;
@@ -124,6 +125,14 @@ public static Safety getSafety(@Nullable Type type, VisitorState state) {
         return Safety.UNKNOWN;
     }
 
+    public static Safety getTypeSafetyFromAncestors(ClassTree classTree, VisitorState state) {
+        Safety safety = SafetyAnnotations.getSafety(classTree.getExtendsClause(), state);
+        for (Tree implemented : classTree.getImplementsClause()) {
+            safety = safety.leastUpperBound(SafetyAnnotations.getSafety(implemented, state));
+        }
+        return safety;
+    }
+
     public static Safety getDirectSafety(@Nullable Symbol symbol, VisitorState state) {
         if (symbol != null) {
             if (containsAttributeNamed(symbol, doNotLogName.get(state))) {

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java

@@ -33,6 +33,7 @@
 import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.matchers.method.MethodMatchers;
 import com.google.errorprone.util.ASTHelpers;
+import com.palantir.baseline.errorprone.MoreASTHelpers;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.LambdaExpressionTree;
@@ -1083,9 +1084,20 @@ public TransferResult<Safety, AccessPathStore<Safety>> visitTypeCast(
     /** Handles type changes (widen, narrow, and cast). */
     private TransferResult<Safety, AccessPathStore<Safety>> handleTypeConversion(
             Tree newType, Node original, TransferInput<Safety, AccessPathStore<Safety>> input) {
-        Safety valueSafety = getValueOfSubNode(input, original);
-        Safety narrowTargetSafety = SafetyAnnotations.getSafety(newType, state);
-        Safety resultSafety = Safety.mergeAssumingUnknownIsSame(valueSafety, narrowTargetSafety);
+        Safety inputSafety = getValueOfSubNode(input, original);
+        Safety targetSafety = SafetyAnnotations.getSafety(newType, state);
+        Safety resultSafety;
+        if (!targetSafety.allowsValueWith(inputSafety)
+                && ASTHelpers.isSubtype(
+                        MoreASTHelpers.getResultType(newType),
+                        MoreASTHelpers.getResultType(original.getTree()),
+                        state)) {
+            // In the case that the cast target is more specific than the input, propagate the safety of the cast
+            // target. Validation occurs when the type itself is constructed, the cast exposes previous validation.
+            resultSafety = targetSafety;
+        } else {
+            resultSafety = Safety.mergeAssumingUnknownIsSame(inputSafety, targetSafety);
+        }
         return noStoreChanges(resultSafety, input);
     }
 

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java

@@ -1736,6 +1736,44 @@ public void testSuperclassLessStrictThanInterfaces() {
                 .doTest();
     }
 
+    @Test
+    public void testCastSafety() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  @DoNotLog class DoNotLogClass {}",
+                        "  @Safe interface SafeClass {}",
+                        "  @Unsafe interface UnsafeClass {}",
+                        "  void f(Object object, UnsafeClass unsafeObject, @Unsafe Object unsafeAnnotatedObject) {",
+                        "    fun(object);",
+                        "    fun((SafeClass) object);",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'",
+                        "    fun((UnsafeClass) object);",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'",
+                        "    fun((DoNotLogClass) object);",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'",
+                        "    fun((SafeClass) unsafeObject);",
+                        "    fun((SafeClass) unsafeAnnotatedObject);",
+                        "  }",
+                        "  private static void fun(@Safe Object obj) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testSubclassWithLenientSafety() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  @Unsafe interface UnsafeClass {}",
+                        "  // BUG: Diagnostic contains: Dangerous type: annotated 'SAFE' but ancestors declare 'SAFE'.",
+                        "  @Safe interface SafeSubclass extends UnsafeClass {}",
+                        "}")
+                .doTest();
+    }
+
     private CompilationTestHelper helper() {
         return CompilationTestHelper.newInstance(IllegalSafeLoggingArgument.class, getClass());
     }

changelog/@unreleased/pr-2289.v2.yml

@@ -0,0 +1,6 @@
+type: improvement
+improvement:
+  description: Trust type safety on cast results, based on validation that occurred
+    when the type was created.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2289