Safety analysis detects annotations on superclasses and their interfa…

…ces (#2271)

Safety analysis detects annotations on superclasses and their interfaces

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

@@ -49,6 +49,13 @@ public final class SafetyAnnotations {
     public static final String UNSAFE = "com.palantir.logsafe.Unsafe";
     public static final String DO_NOT_LOG = "com.palantir.logsafe.DoNotLog";
 
+    private static final com.google.errorprone.suppliers.Supplier<Name> safeName =
+            VisitorState.memoize(state -> state.getName(SAFE));
+    private static final com.google.errorprone.suppliers.Supplier<Name> unsafeName =
+            VisitorState.memoize(state -> state.getName(UNSAFE));
+    private static final com.google.errorprone.suppliers.Supplier<Name> doNotLogName =
+            VisitorState.memoize(state -> state.getName(DO_NOT_LOG));
+
     private static final com.google.errorprone.suppliers.Supplier<Type> throwableSupplier =
             Suppliers.typeFromClass(Throwable.class);
 
@@ -83,14 +90,9 @@ public static Safety getSafety(Tree tree, VisitorState state) {
 
     public static Safety getSafety(@Nullable Symbol symbol, VisitorState state) {
         if (symbol != null) {
-            if (ASTHelpers.hasAnnotation(symbol, DO_NOT_LOG, state)) {
-                return Safety.DO_NOT_LOG;
-            }
-            if (ASTHelpers.hasAnnotation(symbol, UNSAFE, state)) {
-                return Safety.UNSAFE;
-            }
-            if (ASTHelpers.hasAnnotation(symbol, SAFE, state)) {
-                return Safety.SAFE;
+            Safety direct = getDirectSafety(symbol, state);
+            if (direct != Safety.UNKNOWN) {
+                return direct;
             }
             // Check super-methods
             if (symbol instanceof MethodSymbol) {
@@ -105,7 +107,7 @@ public static Safety getSafety(@Nullable Symbol symbol, VisitorState state) {
             }
             if (symbol instanceof ClassSymbol) {
                 ClassSymbol classSymbol = (ClassSymbol) symbol;
-                Safety safety = Safety.UNKNOWN;
+                Safety safety = getSafety(classSymbol.getSuperclass().tsym, state);
                 for (Type type : classSymbol.getInterfaces()) {
                     safety = Safety.mergeAssumingUnknownIsSame(safety, getSafety(type.tsym, state));
                 }
@@ -122,6 +124,30 @@ public static Safety getSafety(@Nullable Type type, VisitorState state) {
         return Safety.UNKNOWN;
     }
 
+    public static Safety getDirectSafety(@Nullable Symbol symbol, VisitorState state) {
+        if (symbol != null) {
+            if (containsAttributeNamed(symbol, doNotLogName.get(state))) {
+                return Safety.DO_NOT_LOG;
+            }
+            if (containsAttributeNamed(symbol, unsafeName.get(state))) {
+                return Safety.UNSAFE;
+            }
+            if (containsAttributeNamed(symbol, safeName.get(state))) {
+                return Safety.SAFE;
+            }
+        }
+        return Safety.UNKNOWN;
+    }
+
+    private static boolean containsAttributeNamed(Symbol symbol, Name annotationName) {
+        for (Attribute.Compound compound : symbol.getRawAttributes()) {
+            if (compound.type.tsym.getQualifiedName().equals(annotationName)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static Safety getTypeVariableSymbolSafety(TypeVariableSymbol typeVariableSymbol) {
         for (int i = 0; i < typeVariableSymbol.owner.getTypeParameters().size(); i++) {
             SymbolMetadata metadata = typeVariableSymbol.owner.getMetadata();

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

@@ -1676,6 +1676,66 @@ public void testResultOfInvocationSuperInterfaceAnnotated() {
                 .doTest();
     }
 
+    @Test
+    public void testSuperClassInterfaceAnnotated() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  @Unsafe",
+                        "  interface Iface {",
+                        "  }",
+                        "  static class Sup implements Iface {}",
+                        "  static class Sub extends Sup {}",
+                        "  void f(Sub value) {",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'",
+                        "    fun(value);",
+                        "  }",
+                        "  private static void fun(@Safe Object value) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testMultipleInterfacesDifferentSafety() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  @Unsafe interface UnsafeIface {}",
+                        "  @Safe interface SafeIface {}",
+                        "  @DoNotLog interface DnlIface {}",
+                        "  static class One implements SafeIface, UnsafeIface {}",
+                        "  static class Two implements SafeIface, DnlIface, UnsafeIface {}",
+                        "  void f(One one, Two two) {",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE'",
+                        "    fun(one);",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'",
+                        "    fun(two);",
+                        "  }",
+                        "  private static void fun(@Safe Object value) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testSuperclassLessStrictThanInterfaces() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  @DoNotLog interface DnlIface {}",
+                        "  @Safe static class Sup {}",
+                        "  static class Impl extends Sup implements DnlIface {}",
+                        "  void f(Impl value) {",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG'",
+                        "    fun(value);",
+                        "  }",
+                        "  private static void fun(@Safe Object value) {}",
+                        "}")
+                .doTest();
+    }
+
     private CompilationTestHelper helper() {
         return CompilationTestHelper.newInstance(IllegalSafeLoggingArgument.class, getClass());
     }

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

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Safety analysis detects annotations on superclasses and their interfaces
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2271