[java] consider java 18 not capturing inner class refs when possible in capturing cleaner inspection

IDEA-283307

GitOrigin-RevId: a1486b6c043cd40fff5da92125bc4c7aa2614891

java/java-impl-inspections/src/com/intellij/codeInspection/CapturingCleanerInspection.java

@@ -92,17 +92,27 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
           if (!lambda.getParameterList().isEmpty()) return null;
           PsiElement lambdaBody = lambda.getBody();
           if (lambdaBody == null) return null;
-          return getLambdaElementCapturingThis(lambdaBody, trackedClass).orElse(null);
+          return getLambdaOrInnerClassElementCapturingThis(lambdaBody, trackedClass);
         }
         if (runnableExpr instanceof PsiNewExpression) {
           PsiNewExpression newExpression = (PsiNewExpression)runnableExpr;
-          if (newExpression.getAnonymousClass() != null) return newExpression;
+          if (newExpression.getAnonymousClass() != null) {
+            if (PsiUtil.isLanguageLevel18OrHigher(trackedClass)) {
+              PsiElement elementCapturingThis = getLambdaOrInnerClassElementCapturingThis(newExpression, trackedClass);
+              if (elementCapturingThis != null) {
+                return elementCapturingThis;
+              }
+            } else {
+              return newExpression;
+            }
+          }
           PsiJavaCodeReferenceElement classReference = newExpression.getClassReference();
           if (classReference == null) return null;
           PsiClass aClass = tryCast(classReference.resolve(), PsiClass.class);
           if (aClass == null) return null;
           if (aClass.getContainingClass() != trackedClass) return null;
           if (aClass.hasModifierProperty(PsiModifier.STATIC)) return null;
+          if (PsiUtil.isLanguageLevel18OrHigher(trackedClass) && getLambdaOrInnerClassElementCapturingThis(newExpression, trackedClass) == null) return null;
           return classReference;
         }
         return null;
@@ -110,16 +120,19 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
     };
   }
 
-  private static Optional<PsiElement> getLambdaElementCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) {
+  @Contract(pure = true)
+  private static @Nullable PsiElement getLambdaOrInnerClassElementCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) {
     return StreamEx.ofTree(lambdaBody, el -> StreamEx.of(el.getChildren()))
-      .findAny(element -> isThisCapturingElement(containingClass, element));
+      .findAny(element -> isThisCapturingElement(containingClass, element))
+      .orElse(null);
   }
 
+  @Contract(pure = true)
   private static boolean isThisCapturingElement(@NotNull PsiClass containingClass, PsiElement element) {
     if (element instanceof PsiThisExpression) {
       return PsiUtil.resolveClassInType(((PsiThisExpression)element).getType()) == containingClass;
     }
-    else if (element instanceof PsiReferenceExpression) {
+    if (element instanceof PsiReferenceExpression) {
       PsiReferenceExpression qualifierReference =
         tryCast(((PsiReferenceExpression)element).getQualifierExpression(), PsiReferenceExpression.class);
       if (qualifierReference != null) return false;
@@ -129,7 +142,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
     return false;
   }
 
-  @Contract("_, null -> false")
+  @Contract(value = "_, null -> false", pure = true)
   private static boolean memberBringsThisRef(@NotNull PsiClass containingClass, PsiMember member) {
     if (member == null) return false;
     PsiClass memberContainingClass = member.getContainingClass();
@@ -141,7 +154,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
     return !member.hasModifierProperty(PsiModifier.STATIC);
   }
 
-  @Contract("_, null -> false")
+  @Contract(value = "_, null -> false", pure = true)
   private static boolean isInnerClassOf(@Nullable PsiClass inner, @Nullable PsiClass outer) {
     if (inner == null || inner.hasModifierProperty(PsiModifier.STATIC)) return false;
     return PsiTreeUtil.isAncestor(outer, inner, false);

java/java-impl/src/inspectionDescriptions/CapturingCleaner.html

@@ -1,10 +1,11 @@
 <html>
 <body>
-Reports <code>Runnable</code> passed to a <code>Cleaner.register()</code> capturing reference that leads to a memory leak.
+Reports <code>Runnable</code> passed to a <code>Cleaner.register()</code> capturing reference being registered.
+If the reference is captured, it will never be phantom reachable and the cleaning action will never be invoked.
 <p>Possible sources of this problem:</p>
 <ul>
   <li>Lambda using non-static methods, fields, or <code>this</code> itself</li>
-  <li>Non-static inner class (anonymous or not) always captures this reference</li>
+  <li>Non-static inner class (anonymous or not) always captures this reference in java up to 18 version</li>
   <li>Instance method reference</li>
   <li>Access to outer class non-static members from non-static inner class</li>
 </ul>

java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java

@@ -1052,6 +1052,10 @@ public final class PsiUtil extends PsiUtilCore {
     return getLanguageLevel(element).isAtLeast(LanguageLevel.JDK_17);
   }
 
+  public static boolean isLanguageLevel18OrHigher(@NotNull PsiElement element) {
+    return getLanguageLevel(element).isAtLeast(LanguageLevel.JDK_18);
+  }
+
   @NotNull
   public static LanguageLevel getLanguageLevel(@NotNull PsiElement element) {
     if (element instanceof PsiDirectory) {

java/java-tests/testData/inspection/cleanerCapturingThis/18/CapturingCleaner.java

@@ -0,0 +1,187 @@
+import java.lang.ref.Cleaner;
+
+class Anonymous {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, new Runnable() {
+    @Override
+    public void run() {
+      System.out.println("adsad");
+    }
+  });
+}
+
+class Inner {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, new MyRunnable());
+
+  private class MyRunnable implements Runnable {
+    @Override
+    public void run() {
+      System.out.println("adsad");
+    }
+  }
+}
+
+class InstanceMethodReference {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">this</warning>::run);
+
+  private void run() {
+    System.out.println("adsad");
+    free(fileDescriptor);
+  }
+}
+
+class LambdaExprBodyInstanceMethod {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">run</warning>());
+
+  private void run() {
+    System.out.println("adsad");
+    free(fileDescriptor);
+  }
+}
+
+class LambdaInstanceField {
+  int fileDescriptor;
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
+    System.out.println("adsad");
+    <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">fileDescriptor</warning> = 0;
+  });
+}
+
+class LambdaInstanceMethod {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
+    System.out.println("adsad");
+    free(<warning descr="Runnable passed to Cleaner.register() captures 'this' reference">fileDescriptor</warning>);
+  });
+}
+
+class Base {
+  int fileDescriptor;
+}
+
+class LambdaInstanceSuperField extends Base {
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
+    System.out.println("adsad");
+    <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">fileDescriptor</warning> = 0;
+  });
+}
+
+class LambdaThis {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
+    LambdaThis o = <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">this</warning>;
+  });
+}
+
+class StaticMethodReference {
+  int fileDescriptor;
+
+  static void free(int descriptor) {}
+
+  Cleaner.Cleanable cleanable = Cleaner.create().register(this, StaticMethodReference::run);
+
+  private static void run() {
+    System.out.println("adsad");
+  }
+}
+
+class ResourceHolder {
+  int resource;
+  public void free(){}
+  public void free(int resource){}
+}
+
+class SafeInstanceMethodReference {
+  Cleaner cleaner = Cleaner.create();
+
+  SafeInstanceMethodReference() {
+    ResourceHolder resource = new ResourceHolder();
+    cleaner.register(this, resource::free);
+  }
+}
+
+
+class FieldMethodReference {
+  Cleaner cleaner = Cleaner.create();
+  ResourceHolder resource = new ResourceHolder();
+
+  FieldMethodReference() {
+    cleaner.register(this, resource::free);
+  }
+}
+
+// Reference as tracking target
+
+class StaticMethodFactory {
+  static Cleaner cleaner = Cleaner.create();
+
+  static ResourceHolder create() {
+    ResourceHolder holder = new ResourceHolder();
+    cleaner.register(holder, <warning descr="Runnable passed to Cleaner.register() captures 'holder' reference">holder</warning>::free);
+    return holder;
+  }
+}
+
+class ConstructorDelegatesToStaticMethod {
+  int resource;
+  static Cleaner cleaner = Cleaner.create();
+
+  ConstructorDelegatesToStaticMethod(int resource) {
+    this.resource = resource;
+    register(this);
+  }
+
+  static void register(ConstructorDelegatesToStaticMethod holder) {
+    cleaner.register(holder, () -> free(<warning descr="Runnable passed to Cleaner.register() captures 'holder' reference">holder</warning>.resource));
+  }
+
+  static void free(int resource){}
+}
+
+
+class InnerAccesInstanceOuterMembers {
+  int resource;
+
+  class Inner {
+    Cleaner cleaner = Cleaner.create();
+
+    public Inner() {
+      cleaner.register(this, () -> <warning descr="Runnable passed to Cleaner.register() captures 'this' reference">resource</warning> = -1);
+    }
+  }
+}
+
+class LambdaUsingAnotherInstanceMember {
+  int fileDescriptor;
+
+  static Cleaner cleaner = Cleaner.create();
+
+  void register() {
+    LambdaUsingAnotherInstanceMember another = new LambdaUsingAnotherInstanceMember();
+    cleaner.register(this, () -> {
+      another.fileDescriptor = 12;
+    });
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleaner18InspectionTest.java

@@ -0,0 +1,37 @@
+// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+
+package com.intellij.java.codeInspection;
+
+import com.intellij.JavaTestUtil;
+import com.intellij.codeInspection.CapturingCleanerInspection;
+import com.intellij.testFramework.LightProjectDescriptor;
+import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
+import org.jetbrains.annotations.NotNull;
+
+public class CapturingCleaner18InspectionTest extends LightJavaCodeInsightFixtureTestCase {
+
+  public void testCapturingCleaner() {doTest();}
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+
+    myFixture.enableInspections(new CapturingCleanerInspection());
+  }
+
+  private void doTest() {
+    myFixture.testHighlighting(true, false, false, getTestName(false) + ".java");
+  }
+
+  @NotNull
+  @Override
+  protected LightProjectDescriptor getProjectDescriptor() {
+    return JAVA_18;
+  }
+
+  @NotNull
+  @Override
+  protected String getTestDataPath() {
+    return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis/18";
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java

@@ -32,6 +32,6 @@ public class CapturingCleanerInspectionTest extends LightJavaCodeInsightFixtureT
   @NotNull
   @Override
   protected String getTestDataPath() {
-    return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis";
+    return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis/before18";
   }
 }