MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-19 13:02:30 +07:00
Code Issues Packages Projects Releases Wiki Activity

LambdaGenerationUtil#canBeUncheckedLambda: supports statements; checks control-flow breaks;

Browse Source 
Fix for IDEA-165369 Replace Optional.isPresent() should be disabled if the thenBranch contains non-local control flow
This commit is contained in:
Tagir Valeev
2016-12-14 15:43:50 +07:00
parent 1cc292570c
commit dddfd5d7b6
8 changed files with 226 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

122
java/java-analysis-impl/src/com/intellij/codeInspection/util/LambdaGenerationUtil.java
View File

@@ -17,12 +17,13 @@ package com.intellij.codeInspection.util;
import com.intellij.codeInsight.ExceptionUtil;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiReferenceExpression;
import com.intellij.psi.PsiVariable;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.function.Predicate;
/**
* Utility methods which are helpful to generate new lambda expressions in quick-fixes
@@ -31,25 +32,112 @@ import org.jetbrains.annotations.Contract;
*/
public class LambdaGenerationUtil {
/**
* Tests the expression whether it could be converted to the body of lambda expression
* mapped to functional interface which SAM does not declare any checked exceptions.
* The following things are checked:
* Tests the element (expression or statement) whether it could be converted to the body
* of lambda expression mapped to functional interface which SAM does not declare any
* checked exceptions. The following things are checked:
*
* <p>1. The expression should not throw checked exceptions</p>
* <p>2. The expression should not refer any variables which are not effectively final</p>
* <p>3. No control flow instructions inside which may jump out of the supplied lambdaCandidate</p>
*
* @param lambdaCandidate an expression to test
* @return true if this expression can be converted to lambda
* @param lambdaCandidate an expression or statement to test
* @return true if this expression or statement can be converted to lambda
*/
@Contract("null -> false")
public static boolean canBeUncheckedLambda(PsiExpression lambdaCandidate) {
if(lambdaCandidate == null) return false;
public static boolean canBeUncheckedLambda(@Nullable PsiElement lambdaCandidate) {
return canBeUncheckedLambda(lambdaCandidate, var -> false);
}
/**
* Tests the element (expression or statement) whether it could be converted to the body
* of lambda expression mapped to functional interface which SAM does not declare any
* checked exceptions. The following things are checked:
*
* <p>1. The expression should not throw checked exceptions</p>
* <p>2. The expression should not refer any variables which are not effectively final
* and not allowed by specified predicate</p>
* <p>3. No control flow instructions inside which may jump out of the supplied lambdaCandidate</p>
*
* @param lambdaCandidate an expression or statement to test
* @param variableAllowedPredicate a predicate which returns true if the variable is allowed to be present inside {@code lambdaCandidate}
* even if it's not effectively final (e.g. it will be replaced by something else when moved to lambda)
* @return true if this expression or statement can be converted to lambda
*/
@Contract("null, _ -> false")
public static boolean canBeUncheckedLambda(@Nullable PsiElement lambdaCandidate, @NotNull Predicate<PsiVariable> variableAllowedPredicate) {
if(!(lambdaCandidate instanceof PsiExpression) && !(lambdaCandidate instanceof PsiStatement)) return false;
if(!ExceptionUtil.getThrownCheckedExceptions(lambdaCandidate).isEmpty()) return false;
return PsiTreeUtil.processElements(lambdaCandidate, e -> {
if (!(e instanceof PsiReferenceExpression)) return true;
PsiElement element = ((PsiReferenceExpression)e).resolve();
return !(element instanceof PsiVariable) ||
HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
});
CanBeLambdaBodyVisitor visitor = new CanBeLambdaBodyVisitor(lambdaCandidate, variableAllowedPredicate);
lambdaCandidate.accept(visitor);
return visitor.canBeLambdaBody();
}
private static class CanBeLambdaBodyVisitor extends JavaRecursiveElementWalkingVisitor {
// Throws is not handled here: it's usually not a problem to move "throws <UncheckedException>" inside lambda.
private boolean myCanBeLambdaBody = true;
private final PsiElement myRoot;
private final Predicate<PsiVariable> myVariableAllowedPredicate;
CanBeLambdaBodyVisitor(PsiElement root, Predicate<PsiVariable> variableAllowedPredicate) {
myRoot = root;
myVariableAllowedPredicate = variableAllowedPredicate;
}
@Override
public void visitElement(PsiElement element) {
if(!myCanBeLambdaBody) return;
super.visitElement(element);
}
@Override
public void visitClass(PsiClass aClass) {
// do not go down the local/anonymous classes
}
@Override
public void visitLambdaExpression(PsiLambdaExpression expression) {
// do not go down the nested lambda expressions
}
@Override
public void visitReferenceExpression(PsiReferenceExpression expression) {
if(!myCanBeLambdaBody) return;
super.visitReferenceExpression(expression);
PsiElement element = expression.resolve();
if (element instanceof PsiVariable &&
!(element instanceof PsiField) &&
!myVariableAllowedPredicate.test((PsiVariable)element) &&
!PsiTreeUtil.isAncestor(myRoot, element, true) &&
!HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, myRoot, null)) {
myCanBeLambdaBody = false;
}
}
@Override
public void visitBreakStatement(PsiBreakStatement statement) {
PsiStatement exitedStatement = statement.findExitedStatement();
if(exitedStatement == null || !PsiTreeUtil.isAncestor(myRoot, exitedStatement, false)) {
myCanBeLambdaBody = false;
}
super.visitBreakStatement(statement);
}
@Override
public void visitContinueStatement(PsiContinueStatement statement) {
PsiStatement continuedStatement = statement.findContinuedStatement();
if(continuedStatement == null || !PsiTreeUtil.isAncestor(myRoot, continuedStatement, false)) {
myCanBeLambdaBody = false;
}
super.visitContinueStatement(statement);
}
@Override
public void visitReturnStatement(PsiReturnStatement statement) {
myCanBeLambdaBody = false;
}
public boolean canBeLambdaBody() {
return myCanBeLambdaBody;
}
}
}

39
java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java
View File

@@ -15,9 +15,7 @@
*/
package com.intellij.codeInspection;
import com.intellij.codeInsight.ExceptionUtil;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInspection.util.LambdaGenerationUtil;
import com.intellij.codeInspection.util.OptionalUtil;
import com.intellij.openapi.diagnostic.Logger;
@@ -47,7 +45,7 @@ import org.jetbrains.annotations.Nullable;
public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionTool {
private static final Logger LOG = Logger.getInstance("#" + OptionalIsPresentInspection.class.getName());
private static final OptionalIfPresentCase[] CASES = {
private static final OptionalIsPresentCase[] CASES = {
new ReturnCase(),
new AssignmentCase(),
new ConsumerCase(),
@@ -57,11 +55,11 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
private enum ProblemType {
WARNING, INFO, NONE;
void registerProblem(ProblemsHolder holder, PsiExpression condition, OptionalIfPresentCase scenario) {
void registerProblem(ProblemsHolder holder, PsiExpression condition, OptionalIsPresentCase scenario) {
if(this != NONE) {
holder.registerProblem(condition, "Can be replaced with single expression in functional style",
this == INFO ? ProblemHighlightType.INFORMATION : ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
new OptionalIfPresentFix(scenario));
new OptionalIsPresentFix(scenario));
}
}
}
@@ -110,7 +108,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
}
void check(PsiExpression condition, PsiVariable optionalVariable, PsiElement thenElement, PsiElement elseElement) {
for (OptionalIfPresentCase scenario : CASES) {
for (OptionalIsPresentCase scenario : CASES) {
scenario.getProblemType(optionalVariable, thenElement, elseElement).registerProblem(holder, condition, scenario);
}
}
@@ -177,18 +175,15 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
((PsiReferenceExpression)lambdaCandidate).isReferenceTo(optionalVariable) && OptionalUtil.isOptionalEmptyCall(falseExpression)) {
return ProblemType.WARNING;
}
if (!ExceptionUtil.getThrownCheckedExceptions(lambdaCandidate).isEmpty()) return ProblemType.NONE;
if (!LambdaGenerationUtil.canBeUncheckedLambda(lambdaCandidate, optionalVariable::equals)) return ProblemType.NONE;
Ref<Boolean> hasOptionalReference = new Ref<>(Boolean.FALSE);
boolean hasNoBadRefs = 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
if (element == optionalVariable) {
hasOptionalReference.set(Boolean.TRUE);
return isOptionalGetCall(e.getParent().getParent(), optionalVariable);
}
return HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)element, lambdaCandidate, null);
if (element != optionalVariable) return true;
// Check that Optional variable is referenced only in context of get() call
hasOptionalReference.set(Boolean.TRUE);
return isOptionalGetCall(e.getParent().getParent(), optionalVariable);
});
if(!hasNoBadRefs) return ProblemType.NONE;
if(hasOptionalReference.get() && lambdaCandidate instanceof PsiExpression) return ProblemType.WARNING;
@@ -241,10 +236,10 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
return ExpressionUtils.isSimpleExpression(expression) || LambdaGenerationUtil.canBeUncheckedLambda(expression);
}
static class OptionalIfPresentFix implements LocalQuickFix {
private final OptionalIfPresentCase myScenario;
static class OptionalIsPresentFix implements LocalQuickFix {
private final OptionalIsPresentCase myScenario;
public OptionalIfPresentFix(OptionalIfPresentCase scenario) {
public OptionalIsPresentFix(OptionalIsPresentCase scenario) {
myScenario = scenario;
}
@@ -290,7 +285,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
}
}
interface OptionalIfPresentCase {
interface OptionalIsPresentCase {
ProblemType getProblemType(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement);
String generateReplacement(PsiElementFactory factory,
@@ -299,7 +294,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
PsiElement falseElement);
}
static class ReturnCase implements OptionalIfPresentCase {
static class ReturnCase implements OptionalIsPresentCase {
@Override
public ProblemType getProblemType(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement) {
if (!(trueElement instanceof PsiReturnStatement) || !(falseElement instanceof PsiReturnStatement)) return ProblemType.NONE;
@@ -324,7 +319,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
}
}
static class AssignmentCase implements OptionalIfPresentCase {
static class AssignmentCase implements OptionalIsPresentCase {
@Override
public ProblemType getProblemType(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement) {
PsiAssignmentExpression trueAssignment = ExpressionUtils.getAssignment(trueElement);
@@ -356,7 +351,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
}
}
static class TernaryCase implements OptionalIfPresentCase {
static class TernaryCase implements OptionalIsPresentCase {
@Override
public ProblemType getProblemType(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement) {
if(!(trueElement instanceof PsiExpression) || !(falseElement instanceof PsiExpression)) return ProblemType.NONE;
@@ -379,7 +374,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
}
}
static class ConsumerCase implements OptionalIfPresentCase {
static class ConsumerCase implements OptionalIsPresentCase {
@Override
public ProblemType getProblemType(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement) {
if (falseElement != null && !(falseElement instanceof PsiEmptyStatement)) return ProblemType.NONE;

1
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumer.java
View File

@@ -4,6 +4,7 @@ import java.util.*;
public class Main {
public void testOptional(Optional<String> str) {
if (str == null) str = Optional.empty();
str.ifPresent(System.out::println);
}
}

27
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterConsumerInternalVarAndControlFlow.java Normal file
View File

@@ -0,0 +1,27 @@
// "Replace Optional.isPresent() condition with functional style expression" "INFORMATION"
import java.util.Optional;
public class Test {
public static void main(String[] args) {
for(String arg : args) {
Optional<String> opt = Optional.of("xyz");
opt.ifPresent(s -> {
System.out.println(s);
System.out.println(s);
Runnable r = () -> {
return;
};
for (int i = 0; i < 10; i++) {
if (i == 3) continue;
System.out.println(arg);
if (i == 5)
break;
}
System.out.println(s);
throw new RuntimeException();
});
}
}
}

1
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumer.java
View File

@@ -4,6 +4,7 @@ import java.util.*;
public class Main {
public void testOptional(Optional<String> str) {
if (str == null) str = Optional.empty();
if (str.isPrese<caret>nt()) {
System.out.println(str.get());
}

25
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerBreak.java Normal file
View File

@@ -0,0 +1,25 @@
// "Replace Optional.isPresent() condition with functional style expression" "false"
import java.util.Optional;
public class Test {
public static void main(String[] args) {
for(String arg : args) {
Optional<String> opt = Optional.of("xyz");
if (opt.isPre<caret>sent()) {
System.out.println(opt.get());
System.out.println(opt.get());
Runnable r = () -> {return;};
for(int i=0; i<10; i++) {
if(i == 3) continue;
System.out.println(arg);
if(i == 5)
break;
}
System.out.println(opt.get());
break;
}
}
}
}

25
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerContinue.java Normal file
View File

@@ -0,0 +1,25 @@
// "Replace Optional.isPresent() condition with functional style expression" "false"
import java.util.Optional;
public class Test {
public static void main(String[] args) {
for(String arg : args) {
Optional<String> opt = Optional.of("xyz");
if (opt.isPre<caret>sent()) {
System.out.println(opt.get());
System.out.println(opt.get());
Runnable r = () -> {return;};
for(int i=0; i<10; i++) {
if(i == 3) continue;
System.out.println(arg);
if(i == 5)
break;
}
System.out.println(opt.get());
continue;
}
}
}
}

25
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeConsumerInternalVarAndControlFlow.java Normal file
View File

@@ -0,0 +1,25 @@
// "Replace Optional.isPresent() condition with functional style expression" "INFORMATION"
import java.util.Optional;
public class Test {
public static void main(String[] args) {
for(String arg : args) {
Optional<String> opt = Optional.of("xyz");
if (opt.isPre<caret>sent()) {
System.out.println(opt.get());
System.out.println(opt.get());
Runnable r = () -> {return;};
for(int i=0; i<10; i++) {
if(i == 3) continue;
System.out.println(arg);
if(i == 5)
break;
}
System.out.println(opt.get());
throw new RuntimeException();
}
}
}
}