IDEA-163764 "Replace Optional.isPresent() checks with functional-style expressions" create uncompilable code

IDEA-163463 Stream API migration: type argument before map appears sometimes when it's unnecessary
This commit is contained in:
Tagir Valeev
2016-11-11 11:28:44 +07:00
parent 67c4dfb483
commit 7e574d661c
16 changed files with 190 additions and 16 deletions

View File

@@ -22,12 +22,15 @@ import com.intellij.codeInspection.util.LambdaGenerationUtil;
import com.intellij.codeInspection.util.OptionalUtil;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Ref;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.codeStyle.SuggestedNameInfo;
import com.intellij.psi.codeStyle.VariableKind;
import com.intellij.psi.impl.PsiDiamondTypeUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.psiutils.BoolUtils;
import com.siyeh.ig.psiutils.ControlFlowUtils;
@@ -164,15 +167,18 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
static boolean isOptionalLambdaCandidate(PsiExpression lambdaCandidate, PsiVariable optionalVariable) {
if (lambdaCandidate == null) return false;
if (!ExceptionUtil.getThrownCheckedExceptions(lambdaCandidate).isEmpty()) return false;
Ref<Boolean> hasOptionalReference = new Ref<>(Boolean.FALSE);
return PsiTreeUtil.processElements(lambdaCandidate, e -> {
if (!(e instanceof PsiReferenceExpression)) return true;
PsiElement element = ((PsiReferenceExpression)e).resolve();
if (!(element instanceof PsiVariable)) return true;
// Check that Optional variable is referenced only in context of get() call and other variables are effectively final
return element == optionalVariable
? isOptionalGetCall(e.getParent().getParent(), optionalVariable)
: HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
});
if (element == optionalVariable) {
hasOptionalReference.set(Boolean.TRUE);
return isOptionalGetCall(e.getParent().getParent(), optionalVariable);
}
return HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
}) && hasOptionalReference.get();
}
@NotNull
@@ -195,14 +201,15 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
static String generateOptionalUnwrap(PsiElementFactory factory,
PsiVariable optionalVariable,
PsiExpression trueValue,
PsiExpression falseValue) {
PsiExpression falseValue,
PsiType targetType) {
String lambdaText = generateOptionalLambda(factory, optionalVariable, trueValue);
PsiLambdaExpression lambda = (PsiLambdaExpression)factory.createExpressionFromText(lambdaText, trueValue);
if(ExpressionUtils.isReferenceTo(falseValue, optionalVariable)) {
falseValue = factory.createExpressionFromText(CommonClassNames.JAVA_UTIL_OPTIONAL+".empty()", falseValue);
}
return OptionalUtil.generateOptionalUnwrap(optionalVariable.getName(), lambda.getParameterList().getParameters()[0],
(PsiExpression)lambda.getBody(), falseValue, falseValue.getType(), true);
(PsiExpression)lambda.getBody(), falseValue, targetType, true);
}
static class OptionalIfPresentFix implements LocalQuickFix {
@@ -260,6 +267,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
factory.createStatementFromText(replacementText, cond);
PsiElement result = cond.replace(replacement);
LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result);
PsiDiamondTypeUtil.removeRedundantTypeArguments(result);
CodeStyleManager.getInstance(project).reformat(result);
}
}
@@ -293,7 +301,9 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
PsiExpression falseValue = ((PsiReturnStatement)falseElement).getReturnValue();
LOG.assertTrue(trueValue != null);
LOG.assertTrue(falseValue != null);
return "return " + generateOptionalUnwrap(factory, optionalVariable, trueValue, falseValue) + ";";
return "return " +
generateOptionalUnwrap(factory, optionalVariable, trueValue, falseValue, PsiTypesUtil.getMethodReturnType(trueElement)) +
";";
}
}
@@ -326,7 +336,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
PsiExpression trueValue = trueAssignment.getRExpression();
PsiExpression falseValue = falseAssignment.getRExpression();
LOG.assertTrue(falseValue != null);
return lValue.getText() + " = " + generateOptionalUnwrap(factory, optionalVariable, trueValue, falseValue) + ";";
return lValue.getText() + " = " + generateOptionalUnwrap(factory, optionalVariable, trueValue, falseValue, lValue.getType()) + ";";
}
}
@@ -344,9 +354,11 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
PsiVariable optionalVariable,
PsiElement trueElement,
PsiElement falseElement) {
PsiExpression ternary = PsiTreeUtil.getParentOfType(trueElement, PsiConditionalExpression.class);
LOG.assertTrue(ternary != null);
PsiExpression trueExpression = (PsiExpression)trueElement;
PsiExpression falseExpression = (PsiExpression)falseElement;
return generateOptionalUnwrap(factory, optionalVariable, trueExpression, falseExpression);
return generateOptionalUnwrap(factory, optionalVariable, trueExpression, falseExpression, ternary.getType());
}
}

View File

@@ -20,6 +20,7 @@ import com.intellij.codeInspection.util.OptionalUtil;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiTypesUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
import org.jetbrains.annotations.NotNull;
@@ -50,7 +51,7 @@ class ReplaceWithFindFirstFix extends MigrateToStreamFix {
if (nextReturnStatement == null) return null;
PsiExpression orElseExpression = nextReturnStatement.getReturnValue();
if (!ExpressionUtils.isSimpleExpression(orElseExpression)) return null;
stream = generateOptionalUnwrap(stream, tb, value, orElseExpression, null);
stream = generateOptionalUnwrap(stream, tb, value, orElseExpression, PsiTypesUtil.getMethodReturnType(returnStatement));
restoreComments(loopStatement, body);
boolean sibling = nextReturnStatement.getParent() == loopStatement.getParent();
PsiElement replacement = loopStatement.replace(elementFactory.createStatementFromText("return " + stream + ";", loopStatement));

View File

@@ -23,6 +23,7 @@ import com.intellij.codeInsight.daemon.HighlightDisplayKey;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInspection.*;
import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
import com.intellij.codeInspection.util.OptionalUtil;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.TextRange;
@@ -997,10 +998,8 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo
operationName = "mapToObj";
}
PsiExpression expression = myType == null ? myExpression : RefactoringUtil.convertInitializerToNormalExpression(myExpression, myType);
if(myType != null && !(myType instanceof PsiPrimitiveType) && !(myType instanceof PsiCapturedWildcardType)) {
operationName = "<"+myType.getCanonicalText()+">"+operationName;
}
return "." + operationName + "(" + LambdaUtil.createLambda(myVariable, expression) + ")";
return "." + OptionalUtil.getMapTypeArgument(expression, myType) + operationName +
"(" + LambdaUtil.createLambda(myVariable, expression) + ")";
}
@Override

View File

@@ -18,6 +18,7 @@ package com.intellij.codeInspection.util;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.MethodCallUtils;
@@ -127,7 +128,8 @@ public class OptionalUtil {
}
trueExpression =
targetType == null ? trueExpression : RefactoringUtil.convertInitializerToNormalExpression(trueExpression, targetType);
qualifier += ".map(" + LambdaUtil.createLambda(var, trueExpression) + ")";
String typeArg = getMapTypeArgument(trueExpression, targetType);
qualifier += "." + typeArg + "map(" + LambdaUtil.createLambda(var, trueExpression) + ")";
}
if (useOrElseGet && !ExpressionUtils.isSimpleExpression(falseExpression)) {
return qualifier + ".orElseGet(() -> " + falseExpression.getText() + ")";
@@ -135,4 +137,16 @@ public class OptionalUtil {
return qualifier + ".orElse(" + falseExpression.getText() + ")";
}
}
@NotNull
public static String getMapTypeArgument(PsiExpression expression, PsiType type) {
if (!(type instanceof PsiClassType)) return "";
if (((PsiClassType)type).getParameterCount() == 0) {
PsiExpression copy =
JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(expression.getText(), expression);
PsiType exprType = copy.getType();
if (exprType != null && !LambdaUtil.notInferredType(exprType) && TypeConversionUtil.isAssignable(type, exprType)) return "";
}
return "<" + type.getCanonicalText() + ">";
}
}

View File

@@ -0,0 +1,15 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.util.Optional;
public class Main<T> {
public static <A extends Annotation> Optional<A> findAnnotation(Optional<? extends AnnotatedElement> element) {
return element.<Optional<A>>map(annotatedElement -> annotatedElement.getAnnotations().length == 0 ? Optional.empty() : null).orElseGet(() -> findAnnotation((AnnotatedElement) null));
}
private static <A extends Annotation> Optional<A> findAnnotation(AnnotatedElement element) {
return Optional.empty();
}
}

View File

@@ -0,0 +1,9 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
import java.util.*;
public class Main {
public static Runnable get(Optional<String> s) {
return s.<Runnable>map(s1 -> s1::trim).orElse(null);
}
}

View File

@@ -0,0 +1,18 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.util.Optional;
public class Main<T> {
public static <A extends Annotation> Optional<A> findAnnotation(Optional<? extends AnnotatedElement> element) {
if (element.isPre<caret>sent()) {
return element.get().getAnnotations().length == 0 ? Optional.empty() : null;
}
return findAnnotation((AnnotatedElement)null);
}
private static <A extends Annotation> Optional<A> findAnnotation(AnnotatedElement element) {
return Optional.empty();
}
}

View File

@@ -0,0 +1,18 @@
// "Replace Optional.isPresent() condition with functional style expression" "false"
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.util.Optional;
public class Main<T> {
public static <A extends Annotation> Optional<A> findAnnotation(Optional<? extends AnnotatedElement> element) {
if (element.isPre<caret>sent()) {
return Optional.empty();
}
return findAnnotation((AnnotatedElement)null);
}
private static <A extends Annotation> Optional<A> findAnnotation(AnnotatedElement element) {
return Optional.empty();
}
}

View File

@@ -0,0 +1,12 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
import java.util.*;
public class Main {
public static Runnable get(Optional<String> s) {
if(s.isPres<caret>ent()) {
return s.get()::trim;
}
return null;
}
}

View File

@@ -5,7 +5,7 @@ import java.util.stream.Collectors;
public class Main {
public List<Runnable> test(List<String> list) {
List<Runnable> result = list.stream().<Runnable>map(s -> new Runnable() {
List<Runnable> result = list.stream().map(s -> new Runnable() {
@Override
public void run() {
String str = s;

View File

@@ -0,0 +1,11 @@
// "Replace with collect" "true"
import java.util.*;
import java.util.stream.Collectors;
public class Main {
public List<CharSequence> getListCharSequence(List<String> input) {
List<CharSequence> result = input.stream().filter(s -> !s.isEmpty()).map(String::trim).collect(Collectors.toList());
return result;
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with findFirst()" "true"
import java.util.*;
public class Main {
public List<String> getErrors(List<String> data) {
List<String> def = Collections.singletonList("Not found");
return data.stream().filter(s -> s.startsWith("xyz")).findFirst().<List<String>>map(s -> s.length() < 10 ? Collections.emptyList() : Arrays.asList()).orElse(def);
}
}

View File

@@ -0,0 +1,10 @@
// "Replace with findFirst()" "true"
import java.util.*;
public class Main {
public Runnable getRunnable(List<String> data) {
Runnable def = () -> {};
return data.stream().filter(s -> s.startsWith("xyz")).findFirst().<Runnable>map(s -> s.length() > 2 ? s::trim : System.out::println).orElse(def);
}
}

View File

@@ -0,0 +1,15 @@
// "Replace with collect" "true"
import java.util.*;
public class Main {
public List<CharSequence> getListCharSequence(List<String> input) {
List<CharSequence> result = new ArrayList<>();
for(String s : in<caret>put) {
if(!s.isEmpty()) {
result.add(s.trim());
}
}
return result;
}
}

View File

@@ -0,0 +1,15 @@
// "Replace with findFirst()" "true"
import java.util.*;
public class Main {
public List<String> getErrors(List<String> data) {
List<String> def = Collections.singletonList("Not found");
for(String s : dat<caret>a) {
if(s.startsWith("xyz")) {
return s.length() < 10 ? Collections.emptyList() : Arrays.asList();
}
}
return def;
}
}

View File

@@ -0,0 +1,15 @@
// "Replace with findFirst()" "true"
import java.util.*;
public class Main {
public Runnable getRunnable(List<String> data) {
Runnable def = () -> {};
for(String s : dat<caret>a) {
if(s.startsWith("xyz")) {
return s.length() > 2 ? s::trim : System.out::println;
}
}
return def;
}
}