IDEA-166636 A bunch of bugs in TrivialFunctionalExpressionUsageInspection (Method call can be simplified)

This commit is contained in:
Tagir Valeev
2017-01-16 12:50:07 +07:00
parent c8c81860e0
commit 4aaa6a4cc2
19 changed files with 270 additions and 93 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -235,7 +235,7 @@ public class StreamToLoopInspection extends BaseJavaBatchLocalInspectionTool {
PsiMethodCallExpression terminalCall = (PsiMethodCallExpression)element;
if(!isSupportedCodeLocation(terminalCall)) return;
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
terminalCall = ensureCodeBlock(terminalCall, factory);
terminalCall = RefactoringUtil.ensureCodeBlock(terminalCall);
if (terminalCall == null) return;
PsiType resultType = terminalCall.getType();
if (resultType == null) return;
@@ -287,42 +287,6 @@ public class StreamToLoopInspection extends BaseJavaBatchLocalInspectionTool {
CodeStyleManager.getInstance(project).reformat(element);
}
/**
* Ensures that given call is surrounded by {@link PsiCodeBlock} (that is, it has a parent statement
* which is located inside the code block). If not, tries to create a code block.
*
* <p>
* Note that the expression is not necessarily a child of {@link PsiExpressionStatement}; it could be a subexpression,
* {@link PsiIfStatement}, etc.
* </p>
*
* @param expression an expression which should be located inside the code block
* @param factory a factory to use to generate code if necessary
* @return a passed expression if it's already surrounded by code block and no changes are necessary;
* a replacement expression (which is equivalent to the passed expression) if a new code block was created;
* {@code null} if the expression cannot be surrounded with code block.
*/
@Nullable
private static PsiMethodCallExpression ensureCodeBlock(PsiMethodCallExpression expression, PsiElementFactory factory) {
PsiElement parent = RefactoringUtil.getParentStatement(expression, false);
if (parent == null) return null;
if (parent instanceof PsiStatement && parent.getParent() instanceof PsiCodeBlock) return expression;
Object marker = new Object();
PsiTreeUtil.mark(expression, marker);
PsiElement copy = parent.copy();
PsiElement newParent;
if (parent instanceof PsiExpression) {
PsiLambdaExpression lambda = (PsiLambdaExpression)parent.getParent();
String replacement = PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(lambda)) ? "{a;}" : "{return a;}";
PsiElement block = parent.replace(factory.createCodeBlockFromText(replacement, lambda));
newParent = LambdaUtil.extractSingleExpressionFromBody(block).replace(copy);
} else {
PsiBlockStatement blockStatement = (PsiBlockStatement)parent.replace(factory.createStatementFromText("{}", parent));
newParent = blockStatement.getCodeBlock().add(copy);
}
return (PsiMethodCallExpression)PsiTreeUtil.releaseMark(newParent, marker);
}
private static StreamEx<OperationRecord> allOperations(List<OperationRecord> operations) {
return StreamEx.of(operations)
.flatMap(or -> or.myOperation.nestedOperations().append(or));

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -982,6 +982,44 @@ public class RefactoringUtil {
return CodeStyleManager.getInstance(element.getProject()).reformat(lambdaExpression.replace(expressionFromText));
}
/**
* Ensures that given call is surrounded by {@link PsiCodeBlock} (that is, it has a parent statement
* which is located inside the code block). If not, tries to create a code block.
*
* <p>
* Note that the expression is not necessarily a child of {@link PsiExpressionStatement}; it could be a subexpression,
* {@link PsiIfStatement}, etc.
* </p>
*
* @param expression an expression which should be located inside the code block
* @param factory a factory to use to generate code if necessary
* @return a passed expression if it's already surrounded by code block and no changes are necessary;
* a replacement expression (which is equivalent to the passed expression) if a new code block was created;
* {@code null} if the expression cannot be surrounded with code block.
*/
@Nullable
public static <T extends PsiExpression> T ensureCodeBlock(@NotNull T expression) {
PsiElement parent = getParentStatement(expression, false);
if (parent == null) return null;
if (parent instanceof PsiStatement && parent.getParent() instanceof PsiCodeBlock) return expression;
Object marker = new Object();
PsiTreeUtil.mark(expression, marker);
PsiElement copy = parent.copy();
PsiElement newParent;
PsiElementFactory factory = JavaPsiFacade.getElementFactory(expression.getProject());
if (parent instanceof PsiExpression) {
PsiLambdaExpression lambda = (PsiLambdaExpression)parent.getParent();
String replacement = PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(lambda)) ? "{a;}" : "{return a;}";
PsiElement block = parent.replace(factory.createCodeBlockFromText(replacement, lambda));
newParent = LambdaUtil.extractSingleExpressionFromBody(block).replace(copy);
} else {
PsiBlockStatement blockStatement = (PsiBlockStatement)parent.replace(factory.createStatementFromText("{}", parent));
newParent = blockStatement.getCodeBlock().add(copy);
}
//noinspection unchecked
return (T)PsiTreeUtil.releaseMark(newParent, marker);
}
public interface ImplicitConstructorUsageVisitor {
void visitConstructor(PsiMethod constructor, PsiMethod baseConstructor);

View File

@@ -0,0 +1,17 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
/* comment1 */
System.out.println(/* output x */"a"+/* who-hoo */ "x");
// comment2
System.out.println("hello");
/*in return */
String s = "foo" + //inline
"bar";
}
}

View File

@@ -0,0 +1,10 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Predicate;
public class Test {
public static void main(String[] args) {
System.out.println("hello");
Collections.singleton("abc").contains("ab");
}
}

View File

@@ -0,0 +1,12 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Supplier;
public class Test {
public static void main(String[] args) {
Supplier<String> s = () -> {
System.out.println("hello");
return "foo";
};
}
}

View File

@@ -0,0 +1,14 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.*;
import java.util.function.*;
public class Test {
public static void main(String[] args) {
List<String> list = new ArrayList<>();
if(list.isEmpty()) {
System.out.println("foo");
list.add("foo");
}
}
}

View File

@@ -2,5 +2,6 @@
import java.util.function.Supplier;
class Test {
String s = "";
// comment
String s = "";
}

View File

@@ -0,0 +1,11 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
/* bar */
/* who-hoo */
String s = "foo";
}
}

View File

@@ -0,0 +1,10 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
/* bar */
String s = ("a" +/* who-hoo */ "x") + "foo";
}
}

View File

@@ -0,0 +1,18 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
String s = ((Function<String, String>) ((x) -> {
/* comment1 */
System.out.println(/* output x */x);
// comment2
System.out.println("hello");
return /*in return */ "foo" + //inline
"bar";
})).<caret>apply("a"+/* who-hoo */ "x");
}
}

View File

@@ -0,0 +1,12 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Predicate;
public class Test {
public static void main(String[] args) {
((Predicate<String>) ((x) -> {
System.out.println("hello");
return Collections.singleton("abc").contains(x);
})).<caret>test("ab");
}
}

View File

@@ -0,0 +1,12 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Supplier;
public class Test {
public static void main(String[] args) {
Supplier<String> s = () -> ((Supplier<String>)() -> {
System.out.println("hello");
return "foo";
}).<caret>get();
}
}

View File

@@ -0,0 +1,15 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.*;
import java.util.function.*;
public class Test {
public static void main(String[] args) {
List<String> list = new ArrayList<>();
if(list.isEmpty())
((Consumer<String>) x -> {
System.out.println(x);
list.add(x);
}).<caret>accept("foo");
}
}

View File

@@ -0,0 +1,14 @@
// "Replace method call on lambda with lambda body" "false"
import java.util.function.Predicate;
public class Test {
public static void main(String[] args) {
((Predicate<String>)((x) -> {
if (x.length() > 0) {
return x.startsWith("a");
}
throw new IllegalArgumentException();
})).<caret>test("ab");
}
}

View File

@@ -0,0 +1,14 @@
// "Replace method call on lambda with lambda body" "false"
import java.util.function.Predicate;
public class Test {
public static void main(String[] args) {
while(((Predicate<String>) ((x) -> {
System.out.println("hello");
return Collections.singleton("abc").contains(x);
})).<caret>test("ab")) {
System.out.println("hello");
}
}
}

View File

@@ -3,6 +3,7 @@ import java.util.function.Supplier;
class Test {
String s = ((Supplier<String>) () -> {
// comment
return "";
}).g<caret>et();
}

View File

@@ -0,0 +1,9 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
String s = ((Function<String, String>)((x) -> /* bar */ "foo")).a<caret>pply("a" +/* who-hoo */ "x");
}
}

View File

@@ -0,0 +1,9 @@
// "Replace method call on lambda with lambda body" "true"
import java.util.function.Function;
public class Test {
public static void main(String[] args) {
String s = ((Function<String, String>)((x) -> /* bar */ x + "foo")).a<caret>pply("a" +/* who-hoo */ "x");
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -24,6 +24,10 @@ import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.refactoring.util.InlineUtil;
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 org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
@@ -51,22 +55,25 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
return callParent instanceof PsiStatement || callParent instanceof PsiLocalVariable || expression.isValueCompatible();
}
if (((PsiCodeBlock)body).getStatements().length == 1) {
PsiStatement[] statements = ((PsiCodeBlock)body).getStatements();
if (statements.length == 1) {
return callParent instanceof PsiStatement
|| callParent instanceof PsiLocalVariable
|| ((PsiCodeBlock)body).getStatements()[0] instanceof PsiReturnStatement && expression.isValueCompatible();
|| statements[0] instanceof PsiReturnStatement && expression.isValueCompatible();
}
final List<PsiExpression> returnExpressions = LambdaUtil.getReturnExpressions(expression);
if (returnExpressions.size() > 1) {
if (returnExpressions.size() > 1 ||
(returnExpressions.size() == 1 && !(ArrayUtil.getLastElement(statements) instanceof PsiReturnStatement))) {
return false;
}
if (returnExpressions.isEmpty()) {
return callParent instanceof PsiStatement;
if (!returnExpressions.isEmpty() && callParent instanceof PsiLocalVariable) {
return true;
}
return callParent instanceof PsiStatement ||
callParent instanceof PsiLocalVariable;
return (callParent instanceof PsiStatement && !(callParent instanceof PsiLoopStatement)) ||
callParent instanceof PsiLambdaExpression;
});
}
@@ -127,60 +134,48 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
}
private static void replaceWithLambdaBody(PsiMethodCallExpression callExpression, PsiLambdaExpression element) {
inlineCallArguments(callExpression, element);
final PsiElement body = element.getBody();
if (body instanceof PsiExpression) {
callExpression.replace(body);
PsiElement body = element.getBody();
PsiExpression expression = LambdaUtil.extractSingleExpressionFromBody(body);
if (expression != null) {
final CommentTracker ct = new CommentTracker();
inlineCallArguments(callExpression, element, ct);
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(expression));
}
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();
if (parent instanceof PsiStatement) {
final PsiElement gParent = parent.getParent();
restoreComments(gParent, parent, body);
for (PsiStatement statement : ((PsiCodeBlock)body).getStatements()) {
PsiElement toInsert;
if (statement instanceof PsiReturnStatement) {
toInsert = ((PsiReturnStatement)statement).getReturnValue();
}
else {
toInsert = statement;
}
if (toInsert != null) {
gParent.addBefore(toInsert, parent);
}
}
parent.delete();
}
else {
final PsiStatement[] statements = ((PsiCodeBlock)body).getStatements();
if (statements.length > 0) {
final PsiStatement anchor = PsiTreeUtil.getParentOfType(parent, PsiStatement.class);
if (anchor != null) {
final PsiElement gParent = anchor.getParent();
restoreComments(gParent, anchor, body);
for (int i = 0; i < statements.length - 1; i++) {
gParent.addBefore(statements[i], anchor);
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);
}
}
PsiStatement statement = statements[statements.length - 1];
final PsiExpression returnValue = ((PsiReturnStatement)statement).getReturnValue();
if (returnValue != null) {
callExpression.replace(returnValue);
}
}
final PsiExpression returnValue = statement == null ? null : statement.getReturnValue();
if (returnValue != null) {
ct.replaceAndRestoreComments(callExpression, ct.markUnchanged(returnValue));
} else {
ct.deleteAndRestoreComments(callExpression);
}
}
}
}
private static void restoreComments(PsiElement gParent, PsiElement parent, PsiElement body) {
for (PsiElement comment : PsiTreeUtil.findChildrenOfType(body, PsiComment.class)) {
gParent.addBefore(comment, parent);
}
}
private static void inlineCallArguments(PsiMethodCallExpression callExpression, PsiLambdaExpression element) {
private static void inlineCallArguments(PsiMethodCallExpression callExpression,
PsiLambdaExpression element,
CommentTracker ct) {
final PsiExpression[] args = callExpression.getArgumentList().getExpressions();
final PsiParameter[] parameters = element.getParameterList().getParameters();
for (int i = 0; i < parameters.length; i++) {
@@ -189,6 +184,7 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
for (PsiReference reference : ReferencesSearch.search(parameter)) {
final PsiElement referenceElement = reference.getElement();
if (referenceElement instanceof PsiJavaCodeReferenceElement) {
ct.markUnchanged(initializer);
InlineUtil.inlineVariable(parameter, initializer, (PsiJavaCodeReferenceElement)referenceElement);
}
}
@@ -207,7 +203,7 @@ public class TrivialFunctionalExpressionUsageInspection extends BaseJavaBatchLoc
@Override
protected void fixExpression(PsiMethodCallExpression callExpression, PsiExpression qualifierExpression) {
if (qualifierExpression instanceof PsiTypeCastExpression) {
final PsiExpression element = ((PsiTypeCastExpression)qualifierExpression).getOperand();
final PsiExpression element = PsiUtil.skipParenthesizedExprDown(((PsiTypeCastExpression)qualifierExpression).getOperand());
if (element instanceof PsiLambdaExpression) {
replaceWithLambdaBody(callExpression, (PsiLambdaExpression)element);
}