From 04df6bad09ca472d7d493ce738bcb7b07546a7d1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 29 Jun 2018 15:27:16 +0700 Subject: [PATCH] ConstantConditionalExpressionInspection: add cast if necessary when type changes Fixes IDEA-194648 Simpler expression auto-fix results in different code --- .../constantConditional/afterNullReturn.java | 7 ++++ .../constantConditional/afterSimple.java | 6 +++ .../afterTypeConversion.java | 7 ++++ .../constantConditional/beforeNullReturn.java | 6 +++ .../constantConditional/beforeSimple.java | 6 +++ .../beforeTypeConversion.java | 7 ++++ ...nstantConditionalExpressionInspection.java | 38 +++++++++++++------ ...onditionalExpressionInspectionFixTest.java | 19 ++++++++++ 8 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterNullReturn.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterTypeConversion.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeNullReturn.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeTypeConversion.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspectionFixTest.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterNullReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterNullReturn.java new file mode 100644 index 000000000000..4d124895c4db --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterNullReturn.java @@ -0,0 +1,7 @@ +// "Simplify" "true" +class Test { + String test(String foo) { + /*always false?*/ + return null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterSimple.java new file mode 100644 index 000000000000..0e94f43f8c79 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterSimple.java @@ -0,0 +1,6 @@ +// "Simplify" "true" +class Test { + int test(int a, int b) { + return a; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterTypeConversion.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterTypeConversion.java new file mode 100644 index 000000000000..a2670033c6f8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/afterTypeConversion.java @@ -0,0 +1,7 @@ +// "Simplify" "true" +class Test { + public static void main(String[] args) { + int i = 0; + System.out.println((int) 'x'); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeNullReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeNullReturn.java new file mode 100644 index 000000000000..9a3c66917d00 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeNullReturn.java @@ -0,0 +1,6 @@ +// "Simplify" "true" +class Test { + String test(String foo) { + return (false/*always false?*/) ? foo : null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeSimple.java new file mode 100644 index 000000000000..603fcc53abc5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeSimple.java @@ -0,0 +1,6 @@ +// "Simplify" "true" +class Test { + int test(int a, int b) { + return true?a:b; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeTypeConversion.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeTypeConversion.java new file mode 100644 index 000000000000..fc67833c1ff6 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional/beforeTypeConversion.java @@ -0,0 +1,7 @@ +// "Simplify" "true" +class Test { + public static void main(String[] args) { + int i = 0; + System.out.println(false ? i : 'x'); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspection.java index eb1ae2d19353..0c43060b2320 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspection.java @@ -19,11 +19,14 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiConditionalExpression; import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiType; +import com.intellij.psi.PsiTypeCastExpression; +import com.intellij.psi.util.PsiTypesUtil; +import com.intellij.psi.util.RedundantCastUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; -import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.BoolUtils; import com.siyeh.ig.psiutils.CommentTracker; import org.jetbrains.annotations.NotNull; @@ -46,20 +49,19 @@ public class ConstantConditionalExpressionInspection @Override @NotNull public String buildErrorString(Object... infos) { - final PsiConditionalExpression expression = - (PsiConditionalExpression)infos[0]; - return InspectionGadgetsBundle.message( - "constant.conditional.expression.problem.descriptor", - calculateReplacementExpression(expression, new CommentTracker())); + final PsiConditionalExpression expression = (PsiConditionalExpression)infos[0]; + return InspectionGadgetsBundle.message("constant.conditional.expression.problem.descriptor", + calculateReplacementExpression(expression).getText()); } - static String calculateReplacementExpression(PsiConditionalExpression exp, CommentTracker commentTracker) { + @NotNull + static PsiExpression calculateReplacementExpression(@NotNull PsiConditionalExpression exp) { final PsiExpression thenExpression = exp.getThenExpression(); final PsiExpression elseExpression = exp.getElseExpression(); final PsiExpression condition = exp.getCondition(); assert thenExpression != null; assert elseExpression != null; - return BoolUtils.isTrue(condition) ? commentTracker.text(thenExpression) : commentTracker.text(elseExpression); + return BoolUtils.isTrue(condition) ? thenExpression : elseExpression; } @Override @@ -79,9 +81,23 @@ public class ConstantConditionalExpressionInspection @Override public void doFix(Project project, ProblemDescriptor descriptor) { final PsiConditionalExpression expression = (PsiConditionalExpression)descriptor.getPsiElement(); - CommentTracker commentTracker = new CommentTracker(); - final String newExpression = calculateReplacementExpression(expression, commentTracker); - PsiReplacementUtil.replaceExpression(expression, newExpression, commentTracker); + CommentTracker ct = new CommentTracker(); + final PsiExpression replacement = calculateReplacementExpression(expression); + PsiType type = replacement.getType(); + PsiType expressionType = expression.getType(); + if (type != null && + expressionType != null && + !type.equals(expressionType) && + PsiTypesUtil.isDenotableType(expressionType, expression)) { + PsiTypeCastExpression castExpression = (PsiTypeCastExpression)ct + .replaceAndRestoreComments(expression, "(" + expressionType.getCanonicalText() + ")" + ct.text(replacement)); + if (RedundantCastUtil.isCastRedundant(castExpression)) { + RedundantCastUtil.removeCast(castExpression); + } + } + else { + ct.replaceAndRestoreComments(expression, replacement); + } } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspectionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspectionFixTest.java new file mode 100644 index 000000000000..16bfe57baf52 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConstantConditionalExpressionInspectionFixTest.java @@ -0,0 +1,19 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.siyeh.ig.controlflow; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; + +public class ConstantConditionalExpressionInspectionFixTest extends LightQuickFixParameterizedTestCase { + @Override + protected void setUp() throws Exception { + super.setUp(); + enableInspectionTool(new ConstantConditionalExpressionInspection()); + } + + public void test() { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/constantConditional"; + } +}