MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

CapturingCleanerInspection: fix review issues

Browse Source
This commit is contained in:
Roman Ivanov
2018-01-25 18:38:04 +07:00
parent 80e3c4aef6
commit c2484042bc
5 changed files with 20 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/META-INF/JavaPlugin.xml
+1 -1
View File
@@ -627,7 +627,7 @@
groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.CapturingCleanerInspection"
bundle="messages.InspectionsBundle"
key="inspection.cleaner.capturing.this.description"/>
key="inspection.capturing.cleaner.description"/>
<localInspection groupPath="Java" language="JAVA" shortName="OverwrittenKey"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
java/java-impl/src/com/intellij/codeInspection/CapturingCleanerInspection.java
+5 -23
View File
@@ -1,7 +1,6 @@
// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection;
import com.intellij.lang.jvm.JvmModifier;
import com.intellij.psi.*;
import com.intellij.psi.util.InheritanceUtil;
import com.intellij.psi.util.PsiUtil;
@@ -38,7 +37,6 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
PsiExpression runnableExpression = ExpressionUtils.resolveExpression(expressions[1]);
if (trackedObject == null || runnableExpression == null) return;
final String referenceName;
final PsiElement highlightingElement;
if (trackedObject instanceof PsiThisExpression) {
PsiClassType classType = tryCast(trackedObject.getType(), PsiClassType.class);
@@ -47,7 +45,6 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
if (trackedClass == null) return;
PsiElement elementCapturingThis = getElementCapturingThis(runnableExpression, trackedClass);
if (elementCapturingThis == null) return;
referenceName = "this";
highlightingElement = elementCapturingThis;
}
else if (trackedObject instanceof PsiReferenceExpression) {
@@ -57,17 +54,16 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
if (!VariableAccessUtils.variableIsUsed(variable, runnableExpression)) return;
Optional<PsiElement> referenceExpression = StreamEx.ofTree(((PsiElement)runnableExpression), el -> StreamEx.of(el.getChildren()))
.findAny(el -> el instanceof PsiReferenceExpression &&
tryCast(((PsiReferenceExpression)el).resolve(), PsiVariable.class) == variable);
((PsiReferenceExpression)el).isReferenceTo(variable));
if (!referenceExpression.isPresent()) return;
String variableName = variable.getName();
if (variableName == null) return;
referenceName = variableName;
highlightingElement = referenceExpression.get();
}
else {
return;
}
holder.registerProblem(highlightingElement, InspectionsBundle.message("inspection.cleaner.capturing.this", referenceName));
holder.registerProblem(highlightingElement, InspectionsBundle.message("inspection.capturing.cleaner"));
}
private PsiElement getElementCapturingThis(PsiExpression runnableExpr, PsiClass trackedClass) {
@@ -77,15 +73,10 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
PsiElement qualifier = methodReference.getQualifier();
if (qualifier instanceof PsiThisExpression) {
PsiClass thisClass = resolveThis((PsiThisExpression)qualifier);
PsiClass thisClass = PsiUtil.resolveClassInType(((PsiThisExpression)qualifier).getType());
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;
}
if (runnableExpr instanceof PsiLambdaExpression) {
@@ -103,7 +94,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
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;
if (aClass.hasModifierProperty(PsiModifier.STATIC)) return null;
return classReference;
}
return null;
@@ -111,15 +102,6 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
};
}
@Contract("null -> null")
@Nullable
static PsiClass resolveThis(@Nullable PsiThisExpression thisExpression) {
if (thisExpression == null) return null;
PsiClassType classType = tryCast(thisExpression.getType(), PsiClassType.class);
if (classType == null) return null;
return classType.resolve();
}
private static Optional<PsiElement> getElementLambdaCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) {
return StreamEx.ofTree(lambdaBody, el -> StreamEx.of(el.getChildren()))
.findAny(element -> isThisCapturingElement(containingClass, element));
@@ -127,7 +109,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT
private static boolean isThisCapturingElement(@NotNull PsiClass containingClass, PsiElement element) {
if (element instanceof PsiThisExpression) {
return resolveThis((PsiThisExpression)element) == containingClass;
return PsiUtil.resolveClassInType(((PsiThisExpression)element).getType()) == containingClass;
}
else if (element instanceof PsiReferenceExpression) {
PsiMember member = tryCast(((PsiReferenceExpression)element).resolve(), PsiMember.class);
java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleanerInspection.java → java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleaner.java
+11 -11
View File
@@ -5,7 +5,7 @@ class Anonymous {
static void free(int descriptor) {}
Cleaner.Cleanable cleanable = Cleaner.create().register(this, <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">new Runnable() {
Cleaner.Cleanable cleanable = Cleaner.create().register(this, <warning descr="Cleaner capture object reference">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 <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">MyRunnable</warning>());
Cleaner.Cleanable cleanable = Cleaner.create().register(this, new <warning descr="Cleaner capture object reference">MyRunnable</warning>());
private class MyRunnable implements Runnable {
@Override
@@ -33,7 +33,7 @@ class InstanceMethodReference {
static void free(int descriptor) {}
Cleaner.Cleanable cleanable = Cleaner.create().register(this, <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">this</warning>::run);
Cleaner.Cleanable cleanable = Cleaner.create().register(this, <warning descr="Cleaner capture object reference">this</warning>::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, () -> <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">run</warning>());
Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> <warning descr="Cleaner capture object reference">run</warning>());
private void run() {
System.out.println("adsad");
@@ -59,7 +59,7 @@ class LambdaInstanceField {
Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
System.out.println("adsad");
<warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">fileDescriptor</warning> = 0;
<warning descr="Cleaner capture object reference">fileDescriptor</warning> = 0;
});
}
@@ -70,7 +70,7 @@ class LambdaInstanceMethod {
Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
System.out.println("adsad");
free(<warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">fileDescriptor</warning>);
free(<warning descr="Cleaner capture object reference">fileDescriptor</warning>);
});
}
@@ -81,7 +81,7 @@ class Base {
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 that leads to memory leak">fileDescriptor</warning> = 0;
<warning descr="Cleaner capture object reference">fileDescriptor</warning> = 0;
});
}
@@ -91,7 +91,7 @@ class LambdaThis {
static void free(int descriptor) {}
Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> {
LambdaThis o = <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">this</warning>;
LambdaThis o = <warning descr="Cleaner capture object reference">this</warning>;
});
}
@@ -139,7 +139,7 @@ class StaticMethodFactory {
static ResourceHolder create() {
ResourceHolder holder = new ResourceHolder();
cleaner.register(holder, <warning descr="Runnable passed to Cleaner.register() captures 'holder' reference that leads to memory leak">holder</warning>::free);
cleaner.register(holder, <warning descr="Cleaner capture object reference">holder</warning>::free);
return holder;
}
}
@@ -154,7 +154,7 @@ class ConstructorDelegatesToStaticMethod {
}
static void register(ConstructorDelegatesToStaticMethod holder) {
cleaner.register(holder, () -> free(<warning descr="Runnable passed to Cleaner.register() captures 'holder' reference that leads to memory leak">holder</warning>.resource));
cleaner.register(holder, () -> free(<warning descr="Cleaner capture object reference">holder</warning>.resource));
}
static void free(int resource){}
@@ -168,7 +168,7 @@ class InnerAccesInstanceOuterMembers {
Cleaner cleaner = Cleaner.create();
public Inner() {
cleaner.register(this, () -> <warning descr="Runnable passed to Cleaner.register() captures 'this' reference that leads to memory leak">resource</warning> = -1);
cleaner.register(this, () -> <warning descr="Cleaner capture object reference">resource</warning> = -1);
}
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java
+1 -1
View File
@@ -10,7 +10,7 @@ import org.jetbrains.annotations.NotNull;
public class CapturingCleanerInspectionTest extends LightCodeInsightFixtureTestCase {
public void testCapturingCleanerInspection() {doTest();}
public void testCapturingCleaner() {doTest();}
@Override
protected void setUp() throws Exception {
platform/platform-resources-en/src/messages/InspectionsBundle.properties
+2 -2
View File
@@ -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 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
inspection.capturing.cleaner=Cleaner capture object reference
inspection.capturing.cleaner.description=Cleaner capture object reference