From e6371b208f39135bb64ec1d202f8eaf04714e410 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Thu, 18 Dec 2014 19:07:20 +0100 Subject: [PATCH] lambda <-> anonymous <-> method ref: collapse lambda block when applicable refactored (IDEA-134509) --- .../AnonymousCanBeLambdaInspection.java | 21 ++--- .../RedundantLambdaCodeBlockInspection.java | 90 ++++++++++--------- .../afterValueVoidCompatibleAmbiguity.java | 4 +- .../afterVoidValueTransformation.java | 9 ++ .../beforeVoidValueTransformation.java | 13 +++ .../ReplaceMethodRefWithLambdaIntention.java | 19 ++-- .../NewRefsInference1_after.java | 4 +- 7 files changed, 93 insertions(+), 67 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterVoidValueTransformation.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeVoidValueTransformation.java 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 9a2a4f74c37c..0266b905186a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/AnonymousCanBeLambdaInspection.java @@ -228,8 +228,6 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaBatchLocalInspection LOG.assertTrue(anonymousClass != null); - final boolean voidCompatible = PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(anonymousClass.getBaseClassType())); - ChangeContextUtil.encodeContextInfo(anonymousClass, true); final PsiElement lambdaContext = anonymousClass.getParent().getParent(); boolean validContext = LambdaUtil.isValidLambdaContext(lambdaContext); @@ -263,27 +261,18 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaBatchLocalInspection PsiLambdaExpression lambdaExpression = (PsiLambdaExpression)elementFactory.createExpressionFromText(withoutTypesDeclared, anonymousClass); - final PsiStatement[] statements = body.getStatements(); - PsiElement copy = body.copy(); - if (statements.length == 1) { - if (statements[0] instanceof PsiReturnStatement) { - PsiExpression value = ((PsiReturnStatement)statements[0]).getReturnValue(); - if (value != null) { - copy = value.copy(); - } - } else if (statements[0] instanceof PsiExpressionStatement && !(voidCompatible && lambdaContext instanceof PsiExpressionList)) { - copy = ((PsiExpressionStatement)statements[0]).getExpression().copy(); - } - } - PsiElement lambdaBody = lambdaExpression.getBody(); LOG.assertTrue(lambdaBody != null); - lambdaBody.replace(copy); + lambdaBody.replace(body); giveUniqueNames(project, lambdaContext, elementFactory, lambdaExpression, lambdaExpression.getParameterList().getParameters()); final PsiNewExpression newExpression = (PsiNewExpression)anonymousClass.getParent(); lambdaExpression = (PsiLambdaExpression)newExpression.replace(lambdaExpression); + final PsiExpression singleExpr = RedundantLambdaCodeBlockInspection.isCodeBlockRedundant(lambdaExpression, lambdaExpression.getBody()); + if (singleExpr != null) { + lambdaExpression.getBody().replace(singleExpr); + } ChangeContextUtil.decodeContextInfo(lambdaExpression, null, null); if (!validContext) { final PsiParenthesizedExpression typeCast = diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java index 55f9a254bce9..78e3432b7897 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java @@ -74,54 +74,64 @@ public class RedundantLambdaCodeBlockInspection extends BaseJavaBatchLocalInspec public void visitLambdaExpression(PsiLambdaExpression expression) { super.visitLambdaExpression(expression); final PsiElement body = expression.getBody(); - if (body instanceof PsiCodeBlock) { - PsiExpression psiExpression = LambdaUtil.extractSingleExpressionFromBody(body); - if (psiExpression != null && !findCommentsOutsideExpression(body, psiExpression)) { - if (LambdaUtil.isExpressionStatementExpression(psiExpression)) { - final PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); - if (parent instanceof PsiExpressionList) { - final PsiElement gParent = parent.getParent(); - if (gParent instanceof PsiCallExpression) { - final CandidateInfo[] candidates = PsiResolveHelper.SERVICE.getInstance(gParent.getProject()) - .getReferencedMethodCandidates((PsiCallExpression)gParent, false); - if (candidates.length > 1) { - final List info = new ArrayList(Arrays.asList(candidates)); - final LanguageLevel level = PsiUtil.getLanguageLevel(parent); - final JavaMethodsConflictResolver conflictResolver = new JavaMethodsConflictResolver((PsiExpressionList)parent, level); - final PsiExpressionList argumentList = ((PsiCallExpression)gParent).getArgumentList(); - if (argumentList == null) return; - JavaMethodsConflictResolver.checkParametersNumber(info, argumentList.getExpressions().length, false); - conflictResolver.checkSpecifics(info, MethodCandidateInfo.ApplicabilityLevel.VARARGS, level); - if (info.size() > 1) { - return; - } - } + final PsiExpression psiExpression = isCodeBlockRedundant(expression, body); + if (psiExpression != null) { + final PsiElement errorElement; + final PsiElement parent = psiExpression.getParent(); + if (parent instanceof PsiReturnStatement) { + errorElement = parent.getFirstChild(); + } else { + errorElement = body.getFirstChild(); + } + holder.registerProblem(errorElement, "Statement lambda can be replaced with expression lambda", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ReplaceWithExprFix()); + } + } + }; + } + + public static PsiExpression isCodeBlockRedundant(PsiExpression expression, PsiElement body) { + if (body instanceof PsiCodeBlock) { + PsiExpression psiExpression = LambdaUtil.extractSingleExpressionFromBody(body); + if (psiExpression != null && !findCommentsOutsideExpression(body, psiExpression)) { + if (LambdaUtil.isExpressionStatementExpression(psiExpression)) { + final PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + if (parent instanceof PsiExpressionList) { + final PsiElement gParent = parent.getParent(); + if (gParent instanceof PsiCallExpression) { + final CandidateInfo[] candidates = PsiResolveHelper.SERVICE.getInstance(gParent.getProject()) + .getReferencedMethodCandidates((PsiCallExpression)gParent, false); + if (candidates.length > 1) { + final List info = new ArrayList(Arrays.asList(candidates)); + final LanguageLevel level = PsiUtil.getLanguageLevel(parent); + final JavaMethodsConflictResolver conflictResolver = new JavaMethodsConflictResolver((PsiExpressionList)parent, level); + final PsiExpressionList argumentList = ((PsiCallExpression)gParent).getArgumentList(); + if (argumentList == null) { + return null; + } + JavaMethodsConflictResolver.checkParametersNumber(info, argumentList.getExpressions().length, false); + conflictResolver.checkSpecifics(info, MethodCandidateInfo.ApplicabilityLevel.VARARGS, level); + if (info.size() > 1) { + return null; } } } - final PsiElement errorElement; - final PsiElement parent = psiExpression.getParent(); - if (parent instanceof PsiReturnStatement) { - errorElement = parent.getFirstChild(); - } else { - errorElement = body.getFirstChild(); - } - holder.registerProblem(errorElement, "Statement lambda can be replaced with expression lambda", - ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ReplaceWithExprFix()); } } + return psiExpression; } + } + return null; + } - private boolean findCommentsOutsideExpression(PsiElement body, PsiExpression psiExpression) { - final Collection comments = PsiTreeUtil.findChildrenOfType(body, PsiComment.class); - for (PsiComment comment : comments) { - if (!PsiTreeUtil.isAncestor(psiExpression, comment, true)) { - return true; - } - } - return false; + private static boolean findCommentsOutsideExpression(PsiElement body, PsiExpression psiExpression) { + final Collection comments = PsiTreeUtil.findChildrenOfType(body, PsiComment.class); + for (PsiComment comment : comments) { + if (!PsiTreeUtil.isAncestor(psiExpression, comment, true)) { + return true; } - }; + } + return false; } private static class ReplaceWithExprFix implements LocalQuickFix, HighPriorityAction { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterValueVoidCompatibleAmbiguity.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterValueVoidCompatibleAmbiguity.java index 0a2993ab531c..9181230bf2ed 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterValueVoidCompatibleAmbiguity.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterValueVoidCompatibleAmbiguity.java @@ -5,7 +5,9 @@ import java.util.Set; class Test { public static void main(String[] args) { Set strings = new HashSet<>(); - new Test().query((ResultSet var1) -> strings.add("Col1")); + new Test().query(var1 -> { + return strings.add("Col1"); + }); } public void query(RowCallbackHandler rch){ diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterVoidValueTransformation.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterVoidValueTransformation.java new file mode 100644 index 000000000000..9cbf793206d4 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/afterVoidValueTransformation.java @@ -0,0 +1,9 @@ +// "Replace with lambda" "true" +import javax.swing.*; +class Test { + String c = null; + + public void main(String[] args){ + SwingUtilities.invokeLater(() -> c.substring(0).toString()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeVoidValueTransformation.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeVoidValueTransformation.java new file mode 100644 index 000000000000..a676def55b15 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/anonymous2lambda/beforeVoidValueTransformation.java @@ -0,0 +1,13 @@ +// "Replace with lambda" "true" +import javax.swing.*; +class Test { + String c = null; + + public void main(String[] args){ + SwingUtilities.invokeLater(new Runnable() { + public void run() { + c.substring(0).toString(); + } + }); + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/types/ReplaceMethodRefWithLambdaIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/types/ReplaceMethodRefWithLambdaIntention.java index 726809211bfd..bffe0cd4eb8b 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/types/ReplaceMethodRefWithLambdaIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/types/ReplaceMethodRefWithLambdaIntention.java @@ -15,6 +15,7 @@ */ package com.siyeh.ipp.types; +import com.intellij.codeInspection.RedundantLambdaCodeBlockInspection; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; @@ -93,10 +94,11 @@ public class ReplaceMethodRefWithLambdaIntention extends Intention { final JavaResolveResult resolveResult = referenceExpression.advancedResolve(false); final PsiElement resolveElement = resolveResult.getElement(); if (resolveElement instanceof PsiMember) { - boolean needBraces = interfaceMethod.getReturnType() == PsiType.VOID && !(resolveElement instanceof PsiMethod && ((PsiMethod)resolveElement).getReturnType() == PsiType.VOID); - if (needBraces) { - buf.append("{"); + buf.append("{"); + + if (!PsiType.VOID.equals(interfaceMethod.getReturnType())) { + buf.append("return "); } final PsiElement qualifier = referenceExpression.getQualifier(); PsiClass containingClass = null; @@ -179,9 +181,7 @@ public class ReplaceMethodRefWithLambdaIntention extends Intention { buf.append(")"); } - if (needBraces) { - buf.append(";}"); - } + buf.append(";}"); } @@ -190,7 +190,12 @@ public class ReplaceMethodRefWithLambdaIntention extends Intention { if (RedundantCastUtil.isCastRedundant(typeCastExpression)) { final PsiExpression operand = typeCastExpression.getOperand(); LOG.assertTrue(operand != null); - typeCastExpression.replace(operand); + final PsiLambdaExpression expr = (PsiLambdaExpression)typeCastExpression.replace(operand); + final PsiElement body = expr.getBody(); + final PsiExpression singleExpression = RedundantLambdaCodeBlockInspection.isCodeBlockRedundant(expr, body); + if (singleExpression != null) { + body.replace(singleExpression); + } } } diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/types/methodRefs2lambda/NewRefsInference1_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/types/methodRefs2lambda/NewRefsInference1_after.java index 9ed43c1000fb..82ef69d1c515 100644 --- a/plugins/IntentionPowerPak/test/com/siyeh/ipp/types/methodRefs2lambda/NewRefsInference1_after.java +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/types/methodRefs2lambda/NewRefsInference1_after.java @@ -10,8 +10,6 @@ public class MyTest { static void m(I s) {} static { - m((x) -> { - new Foo(x); - }); + m((x) -> new Foo(x)); } }