Prevent Unnecessary Propagation When Using Array Containers (#2709)

palantir · Jan 30, 2024 · 11f6015 · 11f6015
1 parent 356477c
commit 11f6015
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 0 deletions.
diff --git a/...-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java b/...-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyAnnotations.java
@@ -205,6 +205,12 @@ private static Safety getSafetyInternal(Type type, VisitorState state, Set<Strin
             }
         }
         Safety typeArgumentCombination = SAFETY_IS_COMBINATION_OF_TYPE_ARGUMENTS.getSafety(type, state, dejaVu);
+        // Arrays are difficult to pipe through the above combiner without a large refactor, since the AST
+        // is not quite a tree it stores its type information adjacent to the node.
+        if (type instanceof Type.ArrayType) {
+            typeArgumentCombination = Safety.mergeAssumingUnknownIsSame(
+                    typeArgumentCombination, getSafety(((Type.ArrayType) type).elemtype.tsym, state));
+        }
         return ASTHelpers.isSubtype(type, throwableSupplier.get(state), state)
                 ? Safety.UNSAFE.leastUpperBound(typeArgumentCombination)
                 : typeArgumentCombination;

diff --git a/...rror-prone/src/test/java/com/palantir/baseline/errorprone/SafeLoggingPropagationTest.java b/...rror-prone/src/test/java/com/palantir/baseline/errorprone/SafeLoggingPropagationTest.java
@@ -1037,6 +1037,49 @@ void testIgnoresOutOfScopeReturns() {
                 .doTest();
     }
 
+    @Test
+    void testSafetyAnnotatedReturnTypeDoesNotAnnotateMethod() {
+        fix().addInputLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "public final class Test {",
+                        "  @Unsafe",
+                        "  private static class UnsafeType {}",
+                        "  public UnsafeType getType() { return new UnsafeType(); }",
+                        "}")
+                .expectUnchanged()
+                .doTest();
+    }
+
+    @Test
+    void testSafetyAnnotatedArrayTypeDoesNotAnnotateMethod() {
+        fix().addInputLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "public final class Test {",
+                        "  @Unsafe",
+                        "  private static class UnsafeType {}",
+                        "  public UnsafeType[] getType() { return new UnsafeType[]{ new UnsafeType() }; }",
+                        "}")
+                .expectUnchanged()
+                .doTest();
+    }
+
+    @Test
+    void testSafetyAnnotatedCollectionTypeDoesNotAnnotateMethod() {
+        fix().addInputLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "import java.util.*;",
+                        "public final class Test {",
+                        "  @Unsafe",
+                        "  private static class UnsafeType {}",
+                        "  public List<UnsafeType> getType() { return new ArrayList<UnsafeType>(); }",
+                        "}")
+                .expectUnchanged()
+                .doTest();
+    }
+
     private RefactoringValidator fix(String... args) {
         return RefactoringValidator.of(SafeLoggingPropagation.class, getClass(), args);
     }

diff --git a/changelog/@unreleased/pr-2709.v2.yml b/changelog/@unreleased/pr-2709.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Prevent Unnecessary Propagation When Using Array Containers
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2709