TrivialFunctionalExpressionUsage bugfixes & refactoring

Fixes IDEA-176019 Trivial functional expression: do not inline if parameter produces side effect and evaluated not once
Fixes EA-103938 (invalid PSI was used if inlining replaced the whole method)
Fixes inlining of functional expression
This commit is contained in:
Tagir Valeev
2017-07-18 17:37:13 +07:00
parent 86977a23ec
commit 5c33eecb2e
8 changed files with 205 additions and 69 deletions

View File

@@ -0,0 +1,10 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Consumer;
import java.util.function.Predicate;
public class Main {
void test() {
((Predicate<String>) String::isEmpty).test("abc");
}
}

View File

@@ -0,0 +1,12 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.IntBinaryOperator;
public class Main {
void test() {
int z = 0;
int x = z += 2;
z += 3;
int res = x;
}
}

View File

@@ -0,0 +1,9 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.IntBinaryOperator;
public class Main {
void test() {
int res = 5;
}
}

View File

@@ -0,0 +1,10 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Consumer;
import java.util.function.Predicate;
public class Main {
void test() {
((Consumer<Predicate<String>>) (x -> x.test("abc"))).ac<caret>cept(String::isEmpty);
}
}

View File

@@ -0,0 +1,9 @@
// "Replace method call on lambda with lambda body" "false"
import java.util.function.IntUnaryOperator;
public class Main {
void test() {
((IntUnaryOperator)x -> x+=5).app<caret>lyAsInt(10);
}
}

View File

@@ -0,0 +1,10 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.IntBinaryOperator;
public class Main {
void test() {
int z = 0;
int res = ((IntBinaryOperator)(x, y) -> x).applyAs<caret>Int(z+=2, z+=3);
}
}

View File

@@ -0,0 +1,9 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.IntBinaryOperator;
public class Main {
void test() {
int res = ((IntBinaryOperator)(x, y) -> x).appl<caret>yAsInt(5, 6);
}
}

View File

@@ -16,7 +16,6 @@
package com.intellij.codeInspection;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Condition;
import com.intellij.psi.*;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.MethodSignatureUtil;
@@ -27,11 +26,16 @@ import com.intellij.refactoring.util.LambdaRefactoringUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.Stream;
public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLocalInspectionTool {
@NotNull
@@ -41,15 +45,17 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
@Override
public void visitMethodReferenceExpression(final PsiMethodReferenceExpression expression) {
doCheckMethodCallOnFunctionalExpression(expression, element -> expression.resolve() != null);
doCheckMethodCallOnFunctionalExpression(expression, call -> expression.resolve() != null);
}
@Override
public void visitLambdaExpression(final PsiLambdaExpression expression) {
doCheckMethodCallOnFunctionalExpression(expression, ggParent -> {
final PsiElement callParent = ggParent.getParent();
final PsiElement body = expression.getBody();
if (body == null) return;
Predicate<PsiMethodCallExpression> checkBody = call -> {
final PsiElement callParent = call.getParent();
final PsiElement body = expression.getBody();
if (!(body instanceof PsiCodeBlock)) {
return callParent instanceof PsiStatement || callParent instanceof PsiLocalVariable || expression.isValueCompatible();
}
@@ -87,21 +93,26 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
return (callParent instanceof PsiStatement && !(callParent instanceof PsiLoopStatement)) ||
callParent instanceof PsiLambdaExpression;
});
};
Predicate<PsiMethodCallExpression> checkWrites = call ->
Arrays.stream(expression.getParameterList().getParameters())
.noneMatch(parameter -> VariableAccessUtils.variableIsAssigned(parameter, body));
doCheckMethodCallOnFunctionalExpression(expression, checkBody.and(checkWrites));
}
@Override
public void visitAnonymousClass(final PsiAnonymousClass aClass) {
if (AnonymousCanBeLambdaInspection.canBeConvertedToLambda(aClass, false, Collections.emptySet())) {
final PsiElement newExpression = aClass.getParent();
doCheckMethodCallOnFunctionalExpression(ggParent -> {
final PsiNewExpression newExpression = ObjectUtils.tryCast(aClass.getParent(), PsiNewExpression.class);
doCheckMethodCallOnFunctionalExpression(call -> {
final PsiMethod method = aClass.getMethods()[0];
final PsiCodeBlock body = method.getBody();
final PsiReturnStatement[] returnStatements = PsiUtil.findReturnStatements(body);
if (returnStatements.length > 1) {
return false;
}
final PsiElement callParent = ggParent.getParent();
final PsiElement callParent = call.getParent();
return callParent instanceof PsiStatement ||
callParent instanceof PsiLocalVariable;
}, newExpression, aClass.getBaseClassType(), new ReplaceAnonymousWithLambdaBodyFix());
@@ -109,81 +120,137 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
}
private void doCheckMethodCallOnFunctionalExpression(PsiElement expression,
Condition<PsiElement> elementContainerCondition) {
final PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
if (parent instanceof PsiTypeCastExpression) {
final PsiType interfaceType = ((PsiTypeCastExpression)parent).getType();
doCheckMethodCallOnFunctionalExpression(elementContainerCondition, parent, interfaceType,
Predicate<PsiMethodCallExpression> elementContainerPredicate) {
final PsiTypeCastExpression parent =
ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(expression.getParent()), PsiTypeCastExpression.class);
if (parent != null) {
final PsiType interfaceType = parent.getType();
doCheckMethodCallOnFunctionalExpression(elementContainerPredicate, parent, interfaceType,
expression instanceof PsiLambdaExpression ? new ReplaceWithLambdaBodyFix()
: new ReplaceWithMethodReferenceFix());
}
}
private void doCheckMethodCallOnFunctionalExpression(Condition<PsiElement> elementContainerCondition,
PsiElement parent,
PsiType interfaceType,
private void doCheckMethodCallOnFunctionalExpression(Predicate<PsiMethodCallExpression> elementContainerPredicate,
PsiExpression qualifier,
PsiType interfaceType,
LocalQuickFix fix) {
final PsiElement gParent = PsiUtil.skipParenthesizedExprUp(parent.getParent());
if (gParent instanceof PsiReferenceExpression) {
final PsiElement ggParent = gParent.getParent();
if (ggParent instanceof PsiMethodCallExpression) {
final PsiMethod resolveMethod = ((PsiMethodCallExpression)ggParent).resolveMethod();
final PsiElement referenceNameElement = ((PsiMethodCallExpression)ggParent).getMethodExpression().getReferenceNameElement();
if (resolveMethod != null &&
!resolveMethod.isVarArgs() &&
((PsiMethodCallExpression)ggParent).getArgumentList().getExpressions().length == resolveMethod.getParameterList().getParametersCount() &&
referenceNameElement != null &&
elementContainerCondition.value(ggParent)) {
final PsiMethod interfaceMethod = LambdaUtil.getFunctionalInterfaceMethod(interfaceType);
if (resolveMethod == interfaceMethod ||
interfaceMethod != null && MethodSignatureUtil.isSuperMethod(interfaceMethod, resolveMethod)) {
holder.registerProblem(referenceNameElement, "Method call can be simplified", fix);
}
}
}
final PsiMethodCallExpression call = ExpressionUtils.getCallForQualifier(qualifier);
if (call == null) return;
final PsiMethod method = call.resolveMethod();
final PsiElement referenceNameElement = call.getMethodExpression().getReferenceNameElement();
boolean suitableMethod = method != null &&
referenceNameElement != null &&
!method.isVarArgs() &&
call.getArgumentList().getExpressions().length == method.getParameterList().getParametersCount() &&
elementContainerPredicate.test(call);
if (!suitableMethod) return;
final PsiMethod interfaceMethod = LambdaUtil.getFunctionalInterfaceMethod(interfaceType);
if (method == interfaceMethod || interfaceMethod != null && MethodSignatureUtil.isSuperMethod(interfaceMethod, method)) {
holder.registerProblem(referenceNameElement, "Method call can be simplified", fix);
}
}
};
}
private static void replaceWithLambdaBody(PsiMethodCallExpression callExpression, PsiLambdaExpression element) {
PsiElement body = element.getBody();
private static void replaceWithLambdaBody(PsiLambdaExpression lambda) {
lambda = extractSideEffects(lambda);
if (lambda == null) return;
PsiMethodCallExpression callExpression = PsiTreeUtil.getParentOfType(lambda, PsiMethodCallExpression.class);
if (callExpression == null) return;
PsiElement body = lambda.getBody();
PsiExpression expression = LambdaUtil.extractSingleExpressionFromBody(body);
if (expression != null) {
final CommentTracker ct = new CommentTracker();
inlineCallArguments(callExpression, element, ct);
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(expression));
replaceExpression(callExpression, lambda);
}
else if (body instanceof PsiCodeBlock) {
element = RefactoringUtil.ensureCodeBlock(element);
if(element == null) return;
body = element.getBody();
if(!(body instanceof PsiCodeBlock)) return;
callExpression = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class);
if(callExpression == null) return;
final CommentTracker ct = new CommentTracker();
inlineCallArguments(callExpression, element, ct);
final PsiElement parent = callExpression.getParent();
final PsiStatement[] statements = ((PsiCodeBlock)body).getStatements();
if (statements.length > 0) {
final PsiStatement anchor = PsiTreeUtil.getParentOfType(parent, PsiStatement.class, false);
PsiReturnStatement statement = ObjectUtils.tryCast(statements[statements.length - 1], PsiReturnStatement.class);
if (anchor != null) {
final PsiElement gParent = anchor.getParent();
for (PsiElement child : body.getChildren()) {
if (child != statement && !(child instanceof PsiJavaToken)) {
gParent.addBefore(ct.markUnchanged(child), anchor);
}
}
replaceCodeBlock(lambda);
}
}
private static PsiLambdaExpression extractSideEffects(PsiLambdaExpression lambda) {
PsiMethodCallExpression callExpression = PsiTreeUtil.getParentOfType(lambda, PsiMethodCallExpression.class);
if (callExpression == null) return lambda;
PsiExpression[] arguments = callExpression.getArgumentList().getExpressions();
if (Stream.of(arguments).noneMatch(SideEffectChecker::mayHaveSideEffects)) return lambda;
lambda = RefactoringUtil.ensureCodeBlock(lambda);
if (lambda == null) return null;
callExpression = PsiTreeUtil.getParentOfType(lambda, PsiMethodCallExpression.class);
if (callExpression == null) return lambda;
arguments = callExpression.getArgumentList().getExpressions();
PsiParameter[] parameters = lambda.getParameterList().getParameters();
if (arguments.length != parameters.length) return lambda;
PsiStatement anchor = PsiTreeUtil.getParentOfType(lambda, PsiStatement.class, false);
if (anchor == null) return lambda;
List<PsiStatement> statements = new ArrayList<>();
PsiElementFactory factory = JavaPsiFacade.getElementFactory(lambda.getProject());
for (int i = 0; i < arguments.length; i++) {
PsiExpression argument = arguments[i];
PsiParameter parameter = parameters[i];
List<PsiExpression> sideEffects = SideEffectChecker.extractSideEffectExpressions(argument);
if (!sideEffects.isEmpty()) {
boolean used = VariableAccessUtils.variableIsUsed(parameter, lambda.getBody());
if (used) {
String name = parameter.getName();
if (name == null) continue;
statements.add(
factory.createStatementFromText(parameter.getType().getCanonicalText() + " " + name + "=" + argument.getText()+";", lambda));
argument.replace(factory.createExpressionFromText(name, parameter));
}
final PsiExpression returnValue = statement == null ? null : statement.getReturnValue();
if (returnValue != null) {
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(returnValue));
} else {
ct.deleteAndRestoreComments(callExpression);
else {
Collections.addAll(statements, StatementExtractor.generateStatements(sideEffects, argument));
}
}
}
BlockUtils.addBefore(anchor, statements.toArray(PsiStatement.EMPTY_ARRAY));
return lambda;
}
private static void replaceExpression(PsiMethodCallExpression callExpression, PsiLambdaExpression element) {
PsiExpression expression;
final CommentTracker ct = new CommentTracker();
LambdaRefactoringUtil.specifyLambdaParameterTypes(element);
inlineCallArguments(callExpression, element, ct);
// body could be invalidated after inlining
expression = LambdaUtil.extractSingleExpressionFromBody(element.getBody());
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(expression));
}
private static void replaceCodeBlock(PsiLambdaExpression element) {
element = RefactoringUtil.ensureCodeBlock(element);
if (element == null) return;
PsiElement body = element.getBody();
if (!(body instanceof PsiCodeBlock)) return;
PsiMethodCallExpression callExpression = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class);
if (callExpression == null) return;
final CommentTracker ct = new CommentTracker();
LambdaRefactoringUtil.specifyLambdaParameterTypes(element);
inlineCallArguments(callExpression, element, ct);
body = element.getBody();
final PsiElement parent = callExpression.getParent();
final PsiStatement[] statements = ((PsiCodeBlock)body).getStatements();
if (statements.length == 0) return;
final PsiStatement anchor = PsiTreeUtil.getParentOfType(parent, PsiStatement.class, false);
PsiReturnStatement statement = ObjectUtils.tryCast(statements[statements.length - 1], PsiReturnStatement.class);
if (anchor != null) {
final PsiElement gParent = anchor.getParent();
for (PsiElement child : body.getChildren()) {
if (child != statement && !(child instanceof PsiJavaToken)) {
gParent.addBefore(ct.markUnchanged(child), anchor);
}
}
}
final PsiExpression returnValue = statement == null ? null : statement.getReturnValue();
if (returnValue != null) {
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(returnValue));
}
else {
ct.deleteAndRestoreComments(callExpression);
}
}
private static void inlineCallArguments(PsiMethodCallExpression callExpression,
@@ -218,7 +285,7 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
if (qualifierExpression instanceof PsiTypeCastExpression) {
final PsiExpression element = PsiUtil.skipParenthesizedExprDown(((PsiTypeCastExpression)qualifierExpression).getOperand());
if (element instanceof PsiLambdaExpression) {
replaceWithLambdaBody(callExpression, (PsiLambdaExpression)element);
replaceWithLambdaBody((PsiLambdaExpression)element);
}
}
}
@@ -240,7 +307,7 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
final PsiLambdaExpression lambdaExpression =
LambdaRefactoringUtil.convertMethodReferenceToLambda((PsiMethodReferenceExpression)element, false, true);
if (lambdaExpression != null) {
replaceWithLambdaBody(callExpression, lambdaExpression);
replaceWithLambdaBody(lambdaExpression);
}
}
}
@@ -262,7 +329,7 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
if (cast instanceof PsiTypeCastExpression) {
final PsiExpression lambdaExpression = ((PsiTypeCastExpression)cast).getOperand();
if (lambdaExpression instanceof PsiLambdaExpression) {
replaceWithLambdaBody(callExpression, (PsiLambdaExpression)lambdaExpression);
replaceWithLambdaBody((PsiLambdaExpression)lambdaExpression);
}
}
}