diff --git a/java/java-impl/src/META-INF/JavaPlugin.xml b/java/java-impl/src/META-INF/JavaPlugin.xml index ae8368ce52d2..0b31b1733ac7 100644 --- a/java/java-impl/src/META-INF/JavaPlugin.xml +++ b/java/java-impl/src/META-INF/JavaPlugin.xml @@ -622,10 +622,10 @@ groupKey="group.names.performance.issues" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.CollectionAddAllCanBeReplacedWithConstructorInspection" displayName="Redundant 'Collection.addAll()' call"/> - referenceExpression = StreamEx.ofTree(((PsiElement)runnableExpression), el -> StreamEx.of(el.getChildren())) + .findAny(el -> el instanceof PsiReferenceExpression && + tryCast(((PsiReferenceExpression)el).resolve(), PsiVariable.class) == variable); + if (!referenceExpression.isPresent()) return; String variableName = variable.getName(); if (variableName == null) return; referenceName = variableName; + highlightingElement = referenceExpression.get(); } else { return; } - holder.registerProblem(runnableExpr, InspectionsBundle.message("inspection.cleaner.capturing.this", referenceName)); + holder.registerProblem(highlightingElement, InspectionsBundle.message("inspection.cleaner.capturing.this", referenceName)); } - private boolean capturesThis(PsiExpression runnableExpr, PsiClass trackedClass) { + private PsiElement getElementCapturingThis(PsiExpression runnableExpr, PsiClass trackedClass) { if (runnableExpr instanceof PsiMethodReferenceExpression) { PsiMethodReferenceExpression methodReference = (PsiMethodReferenceExpression)runnableExpr; - if (PsiMethodReferenceUtil.isStaticallyReferenced(methodReference)) return false; + if (PsiMethodReferenceUtil.isStaticallyReferenced(methodReference)) return null; - PsiThisExpression thisExpression = tryCast(methodReference.getQualifier(), PsiThisExpression.class); - if (thisExpression == null) return false; - PsiClass thisClass = resolveThis(thisExpression); - if (thisClass != trackedClass) return false; - } - else if (runnableExpr instanceof PsiLambdaExpression) { - PsiLambdaExpression lambda = (PsiLambdaExpression)runnableExpr; - if (lambda.getParameterList().getParametersCount() != 0) return false; - PsiElement lambdaBody = lambda.getBody(); - if (lambdaBody == null) return false; - if (!lambdaCapturesThis(lambdaBody, trackedClass)) return false; - } - else if (runnableExpr instanceof PsiNewExpression) { - PsiNewExpression newExpression = (PsiNewExpression)runnableExpr; - if (newExpression.getAnonymousClass() == null) { - PsiJavaCodeReferenceElement classReference = newExpression.getClassReference(); - if (classReference == null) return false; - PsiClass aClass = tryCast(classReference.resolve(), PsiClass.class); - if (aClass == null) return false; - if (aClass.getContainingClass() != trackedClass) return false; - if (aClass.hasModifier(JvmModifier.STATIC)) return false; + PsiElement qualifier = methodReference.getQualifier(); + if (qualifier instanceof PsiThisExpression) { + PsiClass thisClass = resolveThis((PsiThisExpression)qualifier); + if (thisClass != trackedClass) return null; + return qualifier; } + //if (qualifier instanceof PsiReferenceExpression) { + // PsiField field = tryCast(((PsiReferenceExpression)qualifier).resolve(), PsiField.class); + // if (!memberBringsThisRef(trackedClass, field)) return null; + // return qualifier; + //} + return null; } - return true; + if (runnableExpr instanceof PsiLambdaExpression) { + PsiLambdaExpression lambda = (PsiLambdaExpression)runnableExpr; + if (lambda.getParameterList().getParametersCount() != 0) return null; + PsiElement lambdaBody = lambda.getBody(); + if (lambdaBody == null) return null; + return getElementLambdaCapturingThis(lambdaBody, trackedClass).orElse(null); + } + if (runnableExpr instanceof PsiNewExpression) { + PsiNewExpression newExpression = (PsiNewExpression)runnableExpr; + if (newExpression.getAnonymousClass() != null) 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.hasModifier(JvmModifier.STATIC)) return null; + return classReference; + } + return null; } }; } @@ -102,9 +120,9 @@ public class CleanerCapturingThisInspection extends AbstractBaseJavaLocalInspect return classType.resolve(); } - private static boolean lambdaCapturesThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) { + private static Optional getElementLambdaCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) { return StreamEx.ofTree(lambdaBody, el -> StreamEx.of(el.getChildren())) - .anyMatch(element -> isThisCapturingElement(containingClass, element)); + .findAny(element -> isThisCapturingElement(containingClass, element)); } private static boolean isThisCapturingElement(@NotNull PsiClass containingClass, PsiElement element) { @@ -113,18 +131,23 @@ public class CleanerCapturingThisInspection extends AbstractBaseJavaLocalInspect } else if (element instanceof PsiReferenceExpression) { PsiMember member = tryCast(((PsiReferenceExpression)element).resolve(), PsiMember.class); - if (member == null) return false; - PsiClass memberContainingClass = member.getContainingClass(); - if (memberContainingClass == null) return false; - if (!InheritanceUtil.isInheritorOrSelf(containingClass, memberContainingClass, true) && - !isInnerClassOf(containingClass, memberContainingClass)) { - return false; - } - return !member.hasModifierProperty(PsiModifier.STATIC); + return memberBringsThisRef(containingClass, member); } return false; } + @Contract("_, null -> false") + private static boolean memberBringsThisRef(@NotNull PsiClass containingClass, PsiMember member) { + if (member == null) return false; + PsiClass memberContainingClass = member.getContainingClass(); + if (memberContainingClass == null) return false; + if (!InheritanceUtil.isInheritorOrSelf(containingClass, memberContainingClass, true) && + !isInnerClassOf(containingClass, memberContainingClass)) { + return false; + } + return !member.hasModifierProperty(PsiModifier.STATIC); + } + @Contract("_, null -> false") private static boolean isInnerClassOf(@Nullable PsiClass inner, @Nullable PsiClass outer) { if (outer == null) return false; diff --git a/java/java-impl/src/inspectionDescriptions/CleanerCapturingThis.html b/java/java-impl/src/inspectionDescriptions/CapturingCleaner.html similarity index 100% rename from java/java-impl/src/inspectionDescriptions/CleanerCapturingThis.html rename to java/java-impl/src/inspectionDescriptions/CapturingCleaner.html diff --git a/java/java-tests/testData/inspection/cleanerCapturingThis/CleanerCapturingThisInspection.java b/java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleanerInspection.java similarity index 63% rename from java/java-tests/testData/inspection/cleanerCapturingThis/CleanerCapturingThisInspection.java rename to java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleanerInspection.java index 2f16f184560b..a47ef7d82b79 100644 --- a/java/java-tests/testData/inspection/cleanerCapturingThis/CleanerCapturingThisInspection.java +++ b/java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleanerInspection.java @@ -5,7 +5,7 @@ class Anonymous { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, new Runnable() { + Cleaner.Cleanable cleanable = Cleaner.create().register(this, new Runnable() { @Override public void run() { System.out.println("adsad"); @@ -18,7 +18,7 @@ class Inner { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, new MyRunnable()); + Cleaner.Cleanable cleanable = Cleaner.create().register(this, new MyRunnable()); private class MyRunnable implements Runnable { @Override @@ -33,7 +33,7 @@ class InstanceMethodReference { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, this::run); + Cleaner.Cleanable cleanable = Cleaner.create().register(this, this::run); private void run() { System.out.println("adsad"); @@ -46,7 +46,7 @@ class LambdaExprBodyInstanceMethod { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> run()); + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> run()); private void run() { System.out.println("adsad"); @@ -57,10 +57,10 @@ class LambdaExprBodyInstanceMethod { class LambdaInstanceField { int fileDescriptor; - Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { System.out.println("adsad"); - fileDescriptor = 0; - }); + fileDescriptor = 0; + }); } class LambdaInstanceMethod { @@ -68,10 +68,10 @@ class LambdaInstanceMethod { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { System.out.println("adsad"); - free(fileDescriptor); - }); + free(fileDescriptor); + }); } class Base { @@ -79,10 +79,10 @@ class Base { } class LambdaInstanceSuperField extends Base { - Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { System.out.println("adsad"); - fileDescriptor = 0; - }); + fileDescriptor = 0; + }); } class LambdaThis { @@ -90,9 +90,9 @@ class LambdaThis { static void free(int descriptor) {} - Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { - LambdaThis o = this; - }); + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + LambdaThis o = this; + }); } class StaticMethodReference { @@ -122,6 +122,16 @@ class SafeInstanceMethodReference { } } + +class FieldMethodReference { + Cleaner cleaner = Cleaner.create(); + ResourceHolder resource = new ResourceHolder(); + + FieldMethodReference() { + cleaner.register(this, resource::free); + } +} + // Reference as tracking target class StaticMethodFactory { @@ -129,7 +139,7 @@ class StaticMethodFactory { static ResourceHolder create() { ResourceHolder holder = new ResourceHolder(); - cleaner.register(holder, holder::free); + cleaner.register(holder, holder::free); return holder; } } @@ -144,7 +154,7 @@ class ConstructorDelegatesToStaticMethod { } static void register(ConstructorDelegatesToStaticMethod holder) { - cleaner.register(holder, () -> free(holder.resource)); + cleaner.register(holder, () -> free(holder.resource)); } static void free(int resource){} @@ -158,7 +168,7 @@ class InnerAccesInstanceOuterMembers { Cleaner cleaner = Cleaner.create(); public Inner() { - cleaner.register(this, () -> resource = -1); + cleaner.register(this, () -> resource = -1); } } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/CleanerCapturingThisInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java similarity index 74% rename from java/java-tests/testSrc/com/intellij/java/codeInspection/CleanerCapturingThisInspectionTest.java rename to java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java index 743478d7bd0d..c365975343e1 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/CleanerCapturingThisInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java @@ -3,20 +3,20 @@ package com.intellij.java.codeInspection; import com.intellij.JavaTestUtil; -import com.intellij.codeInspection.CleanerCapturingThisInspection; +import com.intellij.codeInspection.CapturingCleanerInspection; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; import org.jetbrains.annotations.NotNull; -public class CleanerCapturingThisInspectionTest extends LightCodeInsightFixtureTestCase { +public class CapturingCleanerInspectionTest extends LightCodeInsightFixtureTestCase { - public void testCleanerCapturingThisInspection() {doTest();} + public void testCapturingCleanerInspection() {doTest();} @Override protected void setUp() throws Exception { super.setUp(); - myFixture.enableInspections(new CleanerCapturingThisInspection()); + myFixture.enableInspections(new CapturingCleanerInspection()); } private void doTest() { diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index b3434b8df72a..1d6dab93f834 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -940,5 +940,5 @@ inspection.conditional.break.in.infinite.loop.description=Conditional break insi inspection.endless.stream.description=Non-short-circuit operation consumes the infinite stream -inspection.cleaner.capturing.this=Runnable passed to register() captures ''{0}'' reference that leads to memory leak +inspection.cleaner.capturing.this=Runnable passed to Cleaner.register() captures ''{0}'' reference that leads to memory leak inspection.cleaner.capturing.this.description=Runnable passed to Cleaner.register() captures reference that leads to memory leak \ No newline at end of file