diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java index b2a537249b28..ea1c64b97f10 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java @@ -17,6 +17,7 @@ package com.intellij.codeInspection; import com.intellij.codeInsight.ChangeContextUtil; import com.intellij.codeInsight.daemon.GroupNames; +import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; import com.intellij.codeInsight.intention.HighPriorityAction; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; @@ -25,14 +26,18 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.controlFlow.AnalysisCanceledException; +import com.intellij.psi.controlFlow.ControlFlow; +import com.intellij.psi.controlFlow.ControlFlowUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.Function; -import com.intellij.util.containers.HashSet; +import com.intellij.util.containers.ContainerUtilRt; import com.intellij.util.containers.hash.LinkedHashMap; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -82,68 +87,11 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaBatchLocalInspection if (methods.length == 1 && aClass.getFields().length == 0) { final PsiCodeBlock body = methods[0].getBody(); if (body != null) { - final boolean [] bodyContainsForbiddenRefs = new boolean[1]; - final Set locals = new HashSet(); - body.accept(new JavaRecursiveElementWalkingVisitor() { - @Override - public void visitMethodCallExpression(PsiMethodCallExpression methodCallExpression) { - if (bodyContainsForbiddenRefs[0]) return; - super.visitMethodCallExpression(methodCallExpression); - final PsiMethod psiMethod = methodCallExpression.resolveMethod(); - if (psiMethod == methods[0] || - psiMethod != null && - !methodCallExpression.getMethodExpression().isQualified() && - "getClass".equals(psiMethod.getName()) && - psiMethod.getParameterList().getParametersCount() == 0) { - bodyContainsForbiddenRefs[0] = true; - } - } - - @Override - public void visitThisExpression(PsiThisExpression expression) { - if (bodyContainsForbiddenRefs[0]) return; - if (expression.getQualifier() == null) { - bodyContainsForbiddenRefs[0] = true; - } - } - - @Override - public void visitSuperExpression(PsiSuperExpression expression) { - if (bodyContainsForbiddenRefs[0]) return; - if (expression.getQualifier() == null) { - bodyContainsForbiddenRefs[0] = true; - } - } - - @Override - public void visitLocalVariable(PsiLocalVariable variable) { - if (bodyContainsForbiddenRefs[0]) return; - super.visitLocalVariable(variable); - locals.add(variable); - } - - @Override - public void visitReferenceExpression(PsiReferenceExpression expression) { - if (bodyContainsForbiddenRefs[0]) return; - super.visitReferenceExpression(expression); - if (!(expression.getParent() instanceof PsiMethodCallExpression)) { - final PsiField field = PsiTreeUtil.getParentOfType(expression, PsiField.class); - if (field != null) { - final PsiElement resolved = expression.resolve(); - if (resolved instanceof PsiField && ((PsiField)resolved).getContainingClass() == field.getContainingClass()) { - final PsiExpression initializer = ((PsiField)resolved).getInitializer(); - if (initializer == null || - initializer.getTextOffset() > aClass.getTextOffset() && !((PsiField)resolved).hasModifierProperty(PsiModifier.STATIC)) { - bodyContainsForbiddenRefs[0] = true; - } - } - } - } - } - }); - if (!bodyContainsForbiddenRefs[0]) { + final ForbiddenRefsChecker checker = new ForbiddenRefsChecker(methods[0], aClass); + body.accept(checker); + if (!checker.hasForbiddenRefs()) { PsiResolveHelper helper = PsiResolveHelper.SERVICE.getInstance(body.getProject()); - for (PsiLocalVariable local : locals) { + for (PsiLocalVariable local : checker.getLocals()) { final String localName = local.getName(); if (localName != null && helper.resolveReferencedVariable(localName, aClass) != null) return; } @@ -329,4 +277,110 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaBatchLocalInspection return parameterType + parameterName; } } + + private static class ForbiddenRefsChecker extends JavaRecursiveElementWalkingVisitor { + private boolean myBodyContainsForbiddenRefs; + private final Set myLocals = ContainerUtilRt.newHashSet(5); + + private final PsiMethod myMethod; + private final PsiAnonymousClass myAnonymClass; + + public ForbiddenRefsChecker(PsiMethod method, + PsiAnonymousClass aClass) { + myMethod = method; + myAnonymClass = aClass; + } + + @Override + public void visitMethodCallExpression(PsiMethodCallExpression methodCallExpression) { + if (myBodyContainsForbiddenRefs) return; + + super.visitMethodCallExpression(methodCallExpression); + final PsiMethod psiMethod = methodCallExpression.resolveMethod(); + if (psiMethod == myMethod || + psiMethod != null && + !methodCallExpression.getMethodExpression().isQualified() && + "getClass".equals(psiMethod.getName()) && + psiMethod.getParameterList().getParametersCount() == 0) { + myBodyContainsForbiddenRefs = true; + } + } + + @Override + public void visitThisExpression(PsiThisExpression expression) { + if (myBodyContainsForbiddenRefs) return; + + if (expression.getQualifier() == null) { + myBodyContainsForbiddenRefs = true; + } + } + + @Override + public void visitSuperExpression(PsiSuperExpression expression) { + if (myBodyContainsForbiddenRefs) return; + + if (expression.getQualifier() == null) { + myBodyContainsForbiddenRefs = true; + } + } + + @Override + public void visitLocalVariable(PsiLocalVariable variable) { + if (myBodyContainsForbiddenRefs) return; + + super.visitLocalVariable(variable); + myLocals.add(variable); + } + + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + if (myBodyContainsForbiddenRefs) return; + + super.visitReferenceExpression(expression); + if (!(expression.getParent() instanceof PsiMethodCallExpression)) { + final PsiField field = PsiTreeUtil.getParentOfType(expression, PsiField.class); + if (field != null) { + final PsiElement resolved = expression.resolve(); + if (resolved instanceof PsiField && ((PsiField)resolved).getContainingClass() == field.getContainingClass()) { + final PsiExpression initializer = ((PsiField)resolved).getInitializer(); + if (initializer == null || + initializer.getTextOffset() > myAnonymClass.getTextOffset() && !((PsiField)resolved).hasModifierProperty(PsiModifier.STATIC)) { + myBodyContainsForbiddenRefs = true; + } + } + } else { + final PsiMethod method = PsiTreeUtil.getParentOfType(myAnonymClass, PsiMethod.class); + if (method != null && method.isConstructor()) { + final PsiElement resolved = expression.resolve(); + if (resolved instanceof PsiField && + ((PsiField)resolved).hasModifierProperty(PsiModifier.FINAL) && + ((PsiField)resolved).getContainingClass() == method.getContainingClass()) { + try { + final PsiCodeBlock constructorBody = method.getBody(); + if (constructorBody != null) { + final ControlFlow flow = HighlightControlFlowUtil.getControlFlowNoConstantEvaluate(constructorBody); + final int startOffset = flow.getStartOffset(myAnonymClass); + final Collection writtenVariables = ControlFlowUtil.getWrittenVariables(flow, 0, startOffset, false); + if (!writtenVariables.contains(resolved)) { + myBodyContainsForbiddenRefs = true; + } + } + } + catch (AnalysisCanceledException e) { + myBodyContainsForbiddenRefs = true; + } + } + } + } + } + } + + public boolean hasForbiddenRefs() { + return myBodyContainsForbiddenRefs; + } + + public Set getLocals() { + return myLocals; + } + } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterFinalFieldInitializedBeforeAnonymInConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterFinalFieldInitializedBeforeAnonymInConstructor.java new file mode 100644 index 000000000000..e481f9b597c2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterFinalFieldInitializedBeforeAnonymInConstructor.java @@ -0,0 +1,15 @@ +// "Replace with lambda" "true" +class HelloLambda { + final int x; + + HelloLambda() { + x = 1; + Runnable r = () -> { + System.out.println(x); + + }; + + } + + +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedAfterAnonymInConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedAfterAnonymInConstructor.java new file mode 100644 index 000000000000..8660181f50a6 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedAfterAnonymInConstructor.java @@ -0,0 +1,18 @@ +// "Replace with lambda" "false" +class HelloLambda { + final int x; + + HelloLambda() { + Runnable r = new Runnable() { + @Override + public void run() { + System.out.println(x); + + } + }; + x = 1; + + } + + +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedBeforeAnonymInConstructor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedBeforeAnonymInConstructor.java new file mode 100644 index 000000000000..be7fc0df1207 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeFinalFieldInitializedBeforeAnonymInConstructor.java @@ -0,0 +1,18 @@ +// "Replace with lambda" "true" +class HelloLambda { + final int x; + + HelloLambda() { + x = 1; + Runnable r = new Runnable() { + @Override + public void run() { + System.out.println(x); + + } + }; + + } + + +}