From 58e361e48eab04b596de1021700ccd6d834baccb Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 11 Jul 2018 17:46:41 +0700 Subject: [PATCH] OptionalUtil#generateOptionalUnwrap: nullity problem fix (EA-124380 - NPE: OptionalUtil.generateOptionalUnwrap) --- .../ConditionalCanBeOptionalInspection.java | 6 +++-- .../OptionalIsPresentInspection.java | 7 ++++-- .../SimplifyOptionalCallChainsInspection.java | 4 ++-- .../codeInspection/util/OptionalUtil.java | 22 ++++++++++--------- .../beforeConditionalIncomplete.java | 8 +++++++ 5 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeConditionalIncomplete.java diff --git a/java/java-impl/src/com/intellij/codeInspection/ConditionalCanBeOptionalInspection.java b/java/java-impl/src/com/intellij/codeInspection/ConditionalCanBeOptionalInspection.java index d48358ee285e..0c7e4356eccf 100644 --- a/java/java-impl/src/com/intellij/codeInspection/ConditionalCanBeOptionalInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/ConditionalCanBeOptionalInspection.java @@ -21,6 +21,8 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Objects; + public class ConditionalCanBeOptionalInspection extends AbstractBaseJavaLocalInspectionTool { @NotNull @Override @@ -116,7 +118,7 @@ public class ConditionalCanBeOptionalInspection extends AbstractBaseJavaLocalIns (PsiLambdaExpression)factory.createExpressionFromText("(" + variable.getType().getCanonicalText() + " " + inLambdaName + ")->" + ct.text(notNullBranch), ternary); PsiParameter lambdaParameter = trueLambda.getParameterList().getParameters()[0]; - PsiExpression trueBody = (PsiExpression)trueLambda.getBody(); + PsiExpression trueBody = Objects.requireNonNull((PsiExpression)trueLambda.getBody()); String replacement = OptionalUtil.generateOptionalUnwrap(CommonClassNames.JAVA_UTIL_OPTIONAL + ".ofNullable(" + name + ")", lambdaParameter, trueBody, ct.markUnchanged(nullBranch), ternary.getType(), !ExpressionUtils.isSafelyRecomputableExpression(nullBranch)); @@ -131,7 +133,7 @@ public class ConditionalCanBeOptionalInspection extends AbstractBaseJavaLocalIns final PsiExpression myNullBranch; final PsiExpression myNotNullBranch; - public TernaryNullCheck(PsiVariable variable, PsiExpression nullBranch, PsiExpression notNullBranch) { + private TernaryNullCheck(PsiVariable variable, PsiExpression nullBranch, PsiExpression notNullBranch) { myVariable = variable; myNullBranch = nullBranch; myNotNullBranch = notNullBranch; diff --git a/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java b/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java index 73697c277f8b..6d3e8708c779 100644 --- a/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java @@ -25,6 +25,8 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Objects; + import static com.intellij.codeInsight.PsiEquivalenceUtil.areElementsEquivalent; public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspectionTool { @@ -239,8 +241,9 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection } String lambdaText = generateOptionalLambda(factory, ct, optionalRef, trueValue); PsiLambdaExpression lambda = (PsiLambdaExpression)factory.createExpressionFromText(lambdaText, trueValue); + PsiExpression body = Objects.requireNonNull((PsiExpression)lambda.getBody()); return OptionalUtil.generateOptionalUnwrap(optionalRef.getText(), lambda.getParameterList().getParameters()[0], - (PsiExpression)lambda.getBody(), ct.markUnchanged(falseValue), targetType, true); + body, ct.markUnchanged(falseValue), targetType, true); } static boolean isSimpleOrUnchecked(PsiExpression expression) { @@ -250,7 +253,7 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection static class OptionalIsPresentFix implements LocalQuickFix { private final OptionalIsPresentCase myScenario; - public OptionalIsPresentFix(OptionalIsPresentCase scenario) { + OptionalIsPresentFix(OptionalIsPresentCase scenario) { myScenario = scenario; } diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java index bbd11b86dea7..ba8ab217f64d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java @@ -465,7 +465,7 @@ public class SimplifyOptionalCallChainsInspection extends AbstractBaseJavaLocalI tryCast(PsiTreeUtil.skipWhitespacesForward(returnVar.getParent()), PsiStatement.class); if (nextStatement == null) return null; PsiExpression lambdaExpr = extractMappingExpression(nextStatement, returnVar); - if (lambdaExpr == null || !LambdaGenerationUtil.canBeUncheckedLambda(lambdaExpr)) return null; + if (!LambdaGenerationUtil.canBeUncheckedLambda(lambdaExpr)) return null; if(!ReferencesSearch.search(returnVar).forEach(reference -> PsiTreeUtil.isAncestor(statement, reference.getElement(), false) || PsiTreeUtil.isAncestor(nextStatement, reference.getElement(), false))) return null; @@ -503,7 +503,7 @@ public class SimplifyOptionalCallChainsInspection extends AbstractBaseJavaLocalI private final String myMessage; private final String myDescription; - public SimplifyOptionalChainFix(String replacement, String message, String description) { + private SimplifyOptionalChainFix(String replacement, String message, String description) { myReplacement = replacement; myMessage = message; myDescription = description; diff --git a/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java b/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java index bd5ee3460c43..6b1f5ace8a97 100644 --- a/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java +++ b/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java @@ -91,7 +91,7 @@ public class OptionalUtil { * @return an expression text which will unwrap an {@code Optional}. */ public static String generateOptionalUnwrap(String qualifier, PsiVariable var, - PsiExpression trueExpression, PsiExpression falseExpression, + @NotNull PsiExpression trueExpression, PsiExpression falseExpression, @Nullable PsiType targetType, boolean useOrElseGet) { PsiExpression stripped = PsiUtil.skipParenthesizedExprDown(trueExpression); PsiType trueType = trueExpression.getType(); @@ -120,15 +120,17 @@ public class OptionalUtil { PsiConditionalExpression condition = (PsiConditionalExpression)stripped; PsiExpression thenExpression = condition.getThenExpression(); PsiExpression elseExpression = condition.getElseExpression(); - if (elseExpression != null && PsiEquivalenceUtil.areElementsEquivalent(falseExpression, elseExpression)) { - return generateOptionalUnwrap( - qualifier + ".filter(" + LambdaUtil.createLambda(var, condition.getCondition()) + ")", var, - condition.getThenExpression(), falseExpression, targetType, useOrElseGet); - } - if (thenExpression != null && PsiEquivalenceUtil.areElementsEquivalent(falseExpression, thenExpression)) { - return generateOptionalUnwrap( - qualifier + ".filter(" + var.getName() + " -> " + BoolUtils.getNegatedExpressionText(condition.getCondition()) + ")", var, - condition.getElseExpression(), falseExpression, targetType, useOrElseGet); + if (thenExpression != null && elseExpression != null) { + if (PsiEquivalenceUtil.areElementsEquivalent(falseExpression, elseExpression)) { + return generateOptionalUnwrap( + qualifier + ".filter(" + LambdaUtil.createLambda(var, condition.getCondition()) + ")", var, + thenExpression, falseExpression, targetType, useOrElseGet); + } + if (PsiEquivalenceUtil.areElementsEquivalent(falseExpression, thenExpression)) { + return generateOptionalUnwrap( + qualifier + ".filter(" + var.getName() + " -> " + BoolUtils.getNegatedExpressionText(condition.getCondition()) + ")", var, + elseExpression, falseExpression, targetType, useOrElseGet); + } } } String suffix = null; diff --git a/java/java-tests/testData/inspection/optionalChains/beforeConditionalIncomplete.java b/java/java-tests/testData/inspection/optionalChains/beforeConditionalIncomplete.java new file mode 100644 index 000000000000..041302708ef9 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeConditionalIncomplete.java @@ -0,0 +1,8 @@ +// "Fix all 'Simplify Optional call chains' problems in file" "false" +import java.util.Optional; + +public class Test { + String test(Optional opt) { + return opt.map(x -> x.isEmpty() ? null : ).orElse(null); + } +} \ No newline at end of file