MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-05-06 05:10:22 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-191856 "Replace Optional.isPresent() condition with functional style expression" creates bad code when used with Type Promotion

Browse Source
This commit is contained in:
Tagir Valeev
2018-05-24 12:44:33 +07:00
parent 2fe3ee616e
commit 0269e72a79
5 changed files with 70 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -19,10 +19,7 @@ import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.BoolUtils;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
@@ -73,7 +70,7 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection
strippedCondition = BoolUtils.getNegated(condition);
invert = true;
}
PsiReferenceExpression optionalRef = extractOptionalFromIfPresentCheck(strippedCondition);
PsiReferenceExpression optionalRef = extractOptionalFromIsPresentCheck(strippedCondition);
if (optionalRef == null) return;
PsiExpression thenExpression = invert ? expression.getElseExpression() : expression.getThenExpression();
PsiExpression elseExpression = invert ? expression.getThenExpression() : expression.getElseExpression();
@@ -91,7 +88,7 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection
strippedCondition = BoolUtils.getNegated(condition);
invert = true;
}
PsiReferenceExpression optionalRef = extractOptionalFromIfPresentCheck(strippedCondition);
PsiReferenceExpression optionalRef = extractOptionalFromIsPresentCheck(strippedCondition);
if (optionalRef == null) return;
PsiStatement thenStatement = extractThenStatement(statement, invert);
PsiStatement elseStatement = extractElseStatement(statement, invert);
@@ -135,7 +132,7 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection
@Nullable
@Contract("null -> null")
static PsiReferenceExpression extractOptionalFromIfPresentCheck(PsiExpression expression) {
static PsiReferenceExpression extractOptionalFromIsPresentCheck(PsiExpression expression) {
if (!(expression instanceof PsiMethodCallExpression)) return null;
PsiMethodCallExpression call = (PsiMethodCallExpression)expression;
if (!call.getArgumentList().isEmpty()) return null;
@@ -183,13 +180,20 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection
if (!hasNoBadRefs) return ProblemType.NONE;
if (!hasOptionalReference.get() || !(lambdaCandidate instanceof PsiExpression)) return ProblemType.INFO;
PsiExpression expression = (PsiExpression)lambdaCandidate;
if (falseExpression != null &&
!ExpressionUtils.isNullLiteral(falseExpression) &&
NullnessUtil.getExpressionNullness(expression, true) != Nullness.NOT_NULL) {
// falseExpression == null is "consumer" case (to be replaced with ifPresent()),
// in this case we don't care about expression nullness
// if falseExpression is null literal, then semantics is preserved
return ProblemType.INFO;
if (falseExpression != null) {
// falseExpression == null is "consumer" case (to be replaced with ifPresent())
if (!ExpressionUtils.isNullLiteral(falseExpression) && NullnessUtil.getExpressionNullness(expression, true) != Nullness.NOT_NULL) {
// if falseExpression is null literal, then semantics is preserved
return ProblemType.INFO;
}
PsiType falseType = falseExpression.getType();
PsiType trueType = expression.getType();
// like x ? double_expression : integer_expression; support only if integer_expression is simple literal,
// so could be converted explicitly to double
if (falseType instanceof PsiPrimitiveType && trueType instanceof PsiPrimitiveType &&
!falseType.equals(trueType) && JavaPsiMathUtil.getNumberFromLiteral(falseExpression) == null) {
return ProblemType.NONE;
}
}
return ProblemType.WARNING;
}
@@ -266,7 +270,7 @@ public class OptionalIsPresentInspection extends AbstractBaseJavaLocalInspection
condition = BoolUtils.getNegated(condition);
invert = true;
}
PsiReferenceExpression optionalRef = extractOptionalFromIfPresentCheck(condition);
PsiReferenceExpression optionalRef = extractOptionalFromIsPresentCheck(condition);
if (optionalRef == null) return;
PsiElement cond = PsiTreeUtil.getParentOfType(element, PsiIfStatement.class, PsiConditionalExpression.class);
PsiElement thenElement;

22
java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java
View File

@@ -10,10 +10,7 @@ import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.BoolUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.MethodCallUtils;
import com.siyeh.ig.psiutils.StreamApiUtil;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -196,7 +193,22 @@ public class OptionalUtil {
if (useOrElseGet && !ExpressionUtils.isSafelyRecomputableExpression(falseExpression)) {
return qualifier + ".orElseGet(() -> " + falseExpression.getText() + ")";
} else {
return qualifier + ".orElse(" + falseExpression.getText() + ")";
PsiType falseType = falseExpression.getType();
String falseText = falseExpression.getText();
if (falseType instanceof PsiPrimitiveType && targetType instanceof PsiPrimitiveType && !(targetType.equals(falseType))) {
Number falseValue = JavaPsiMathUtil.getNumberFromLiteral(falseExpression);
if (falseValue != null) {
falseText = falseValue.toString();
if (targetType.equals(PsiType.FLOAT)) {
falseText += "F";
} else if(targetType.equals(PsiType.DOUBLE) && !falseText.contains(".")) {
falseText += ".0";
} else if(targetType.equals(PsiType.LONG)) {
falseText += "L";
}
}
}
return qualifier + ".orElse(" + falseText + ")";
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterTernaryPrimitiveSimple.java Normal file
View File

@@ -0,0 +1,11 @@
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
public class Main {
void test(Optional<String> foo) {
double bar = foo.map(s -> s.length() * 1.2).orElse(0.0);
}
}

12
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeTernaryPrimitive.java Normal file
View File

@@ -0,0 +1,12 @@
// "Replace Optional.isPresent() condition with functional style expression" "false"
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
public class Main {
void test(Optional<String> foo) {
int defaultValue = 0;
double bar = foo.isPresent<caret>() ? foo.get().length() * 1.2 : defaultValue;
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeTernaryPrimitiveSimple.java Normal file
View File

@@ -0,0 +1,11 @@
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
public class Main {
void test(Optional<String> foo) {
double bar = foo.isPresent<caret>() ? foo.get().length() * 1.2 : 0;
}
}