diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml index 55df266eac84..2db366db5a00 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/META-INF/InspectionGadgets.xml @@ -603,6 +603,10 @@ key="conditional.expression.with.identical.branches.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.control.flow.issues" enabledByDefault="false" level="WARNING" implementationClass="com.siyeh.ig.controlflow.ConditionalExpressionWithIdenticalBranchesInspection" cleanupTool="true"/> + diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index f3e7a9f5d6d8..8eb966316faa 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -816,7 +816,10 @@ enumerated.constant.naming.convention.element.description=Enumerated constant final.method.display.name='final' method transient.field.in.non.serializable.class.display.name=Transient field in non-serializable class bad.exception.thrown.display.name=Prohibited exception thrown -conditional.expression.with.identical.branches.display.name=Conditional expression with identical or similar branches +conditional.expression.with.identical.branches.display.name=Conditional expression with identical branches +conditional.can.be.pushed.inside.expression.display.name=Conditional can be pushed inside branch expression +conditional.can.be.pushed.inside.expression.option=Ignore when conditional will be only argument of a method call +conditional.can.be.pushed.inside.expression.quickfix=Push conditional expression inside branch raw.use.of.parameterized.type.display.name=Raw use of parameterized class standard.variable.names.display.name=Standard variable names instance.variable.naming.convention.display.name=Instance field naming convention @@ -858,7 +861,7 @@ switch.statements.without.default.problem.descriptor=#ref statement default.not.last.case.in.switch.problem.descriptor=#ref branch not last case in 'switch' statement #loc loop.statements.that.dont.loop.problem.descriptor=#ref statement does not loop #loc conditional.expression.with.identical.branches.problem.descriptor=Conditional expression #ref with identical branches #loc -conditional.expression.with.similar.branches.problem.descriptor=Conditional expression #ref with similar branches #loc +conditional.can.be.pushed.inside.expression.problem.descriptor=Conditional expression can be pushed inside branch #loc if.statement.with.identical.branches.problem.descriptor=#ref statement with identical branches #loc duplicate.condition.problem.descriptor=Duplicate condition #ref #loc duplicate.condition.ignore.method.calls.option=Ignore method calls in condition @@ -1123,8 +1126,6 @@ standard.variable.names.ignore.override.option=Ignore for parameter names identi static.variable.naming.convention.mutable.option=Check 'static final' fields with a mutable type boolean.method.name.must.start.with.question.table.column.name=Boolean method name prefix conditional.expression.with.identical.branches.collapse.quickfix=Collapse conditional expression -conditional.expression.with.identical.branches.push.inside.quickfix=Push conditional inside expression -conditional.expression.with.identical.branches.collapse.quickfix.family=Conditional expression can be simplified redundant.else.unwrap.quickfix=Remove redundant 'else' constant.conditional.expression.problem.descriptor=#ref can be simplified to ''{0}'' #loc constant.conditional.expression.simplify.quickfix=Simplify diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspection.java new file mode 100644 index 000000000000..56427c51b476 --- /dev/null +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspection.java @@ -0,0 +1,141 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.controlflow; + +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.psiutils.EquivalenceChecker; +import com.siyeh.ig.psiutils.ParenthesesUtils; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; + +/** + * @author Bas Leijdekkers + */ +public class ConditionalCanBePushedInsideExpressionInspection extends BaseInspection { + + public boolean ignoreSingleArgument = true; + + @Nls + @NotNull + @Override + public String getDisplayName() { + return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.display.name"); + } + + @NotNull + @Override + protected String buildErrorString(Object... infos) { + return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.problem.descriptor"); + } + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.option"), + this, "ignoreSingleArgument"); + } + + @Nullable + @Override + protected InspectionGadgetsFix buildFix(Object... infos) { + return new PushConditionalInsideFix(); + } + + private static class PushConditionalInsideFix extends InspectionGadgetsFix { + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionGadgetsBundle.message("conditional.can.be.pushed.inside.expression.quickfix"); + } + + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) { + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)descriptor.getPsiElement(); + final PsiExpression thenExpression = conditionalExpression.getThenExpression(); + if (thenExpression == null) { + return; + } + final EquivalenceChecker.Match match = + EquivalenceChecker.getCanonicalPsiEquivalence().expressionsMatch(thenExpression, conditionalExpression.getElseExpression()); + if (!match.isPartialMatch()) { + return; + } + final PsiElement leftDiff = match.getLeftDiff(); + final PsiElement rightDiff = match.getRightDiff(); + final String expression = "(" + conditionalExpression.getCondition().getText() + " ? " + + leftDiff.getText() + " : " + rightDiff.getText() + ")"; + final PsiExpression newConditionalExpression = + JavaPsiFacade.getElementFactory(project).createExpressionFromText(expression, conditionalExpression); + final PsiElement replacedConditionalExpression = leftDiff.replace(newConditionalExpression); + ParenthesesUtils.removeParentheses((PsiExpression)replacedConditionalExpression, false); + conditionalExpression.replace(thenExpression); + } + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new ConditionalCanBePushedInsideExpressionVisitor(); + } + + private class ConditionalCanBePushedInsideExpressionVisitor extends BaseInspectionVisitor { + + @Override + public void visitConditionalExpression(PsiConditionalExpression expression) { + super.visitConditionalExpression(expression); + final PsiExpression thenExpression = expression.getThenExpression(); + if (thenExpression == null) { + return; + } + final PsiExpression elseExpression = expression.getElseExpression(); + final EquivalenceChecker.Match match = + EquivalenceChecker.getCanonicalPsiEquivalence().expressionsMatch(thenExpression, elseExpression); + if (match.isExactMismatch() || match.isExactMatch()) { + return; + } + registerError(expression, ignoreSingleArgument && isOnlyArgumentOfMethodCall(match.getLeftDiff()) + ? ProblemHighlightType.INFORMATION + : ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + } + + private boolean isOnlyArgumentOfMethodCall(PsiElement element) { + if (element == null) { + return false; + } + final PsiElement parent = element.getParent(); + if (!(parent instanceof PsiExpressionList)) { + return false; + } + final PsiExpressionList expressionList = (PsiExpressionList)parent; + if (expressionList.getExpressions().length != 1) { + return false; + } + final PsiElement grandParent = expressionList.getParent(); + return grandParent instanceof PsiMethodCallExpression; + } + } +} diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalExpressionWithIdenticalBranchesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalExpressionWithIdenticalBranchesInspection.java index fa06c963926a..7c8c7ec237af 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalExpressionWithIdenticalBranchesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/ConditionalExpressionWithIdenticalBranchesInspection.java @@ -17,7 +17,6 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.CleanupLocalInspectionTool; import com.intellij.codeInspection.ProblemDescriptor; -import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.siyeh.InspectionGadgetsBundle; @@ -26,20 +25,9 @@ import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.EquivalenceChecker; -import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -import javax.swing.*; public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseInspection implements CleanupLocalInspectionTool { - public boolean myReportOnlyExactlyIdentical; - - @Nullable - @Override - public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel("Report only exactly identical branches", this, "myReportOnlyExactlyIdentical"); - } @Override @NotNull @@ -50,70 +38,34 @@ public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseIn @Override @NotNull protected String buildErrorString(Object... infos) { - final EquivalenceChecker.Match decision = (EquivalenceChecker.Match)infos[1]; - return InspectionGadgetsBundle.message(decision.isPartialMatch() - ? "conditional.expression.with.similar.branches.problem.descriptor" - : "conditional.expression.with.identical.branches.problem.descriptor"); + return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.problem.descriptor"); } @Override public InspectionGadgetsFix buildFix(Object... infos) { - return new CollapseConditional((PsiConditionalExpression)infos[0]); + return new CollapseConditionalFix(); } - private static class CollapseConditional extends InspectionGadgetsFix { - private final SmartPsiElementPointer myConditionalExpression; - - public CollapseConditional(PsiConditionalExpression expression) { - myConditionalExpression = SmartPointerManager.getInstance(expression.getProject()).createSmartPsiElementPointer(expression); - } - - @Override - @NotNull - public String getName() { - return InspectionGadgetsBundle.message(getEquivalenceDecision().isExactMatch() - ? "conditional.expression.with.identical.branches.collapse.quickfix" - : "conditional.expression.with.identical.branches.push.inside.quickfix"); - } + private static class CollapseConditionalFix extends InspectionGadgetsFix { @Override @NotNull public String getFamilyName() { - return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.collapse.quickfix.family"); - } - - public PsiConditionalExpression getConditionalExpression() { - return myConditionalExpression.getElement(); - } - - private EquivalenceChecker.Match getEquivalenceDecision() { - return EquivalenceChecker.getCanonicalPsiEquivalence() - .expressionsMatch(getConditionalExpression().getThenExpression(), getConditionalExpression().getElseExpression()); + return InspectionGadgetsBundle.message("conditional.expression.with.identical.branches.collapse.quickfix"); } @Override public void doFix(Project project, ProblemDescriptor descriptor) { - final EquivalenceChecker.Match decision = getEquivalenceDecision(); - final PsiConditionalExpression conditionalExpression = getConditionalExpression(); + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)descriptor.getPsiElement(); final PsiExpression thenExpression = conditionalExpression.getThenExpression(); - assert thenExpression != null; - if (decision.isExactMatch()) { - final PsiConditionalExpression expression = (PsiConditionalExpression)descriptor.getPsiElement(); - final String bodyText = thenExpression.getText(); - PsiReplacementUtil.replaceExpression(expression, bodyText); - } else if (!decision.isExactMismatch()) { - final PsiElement leftDiff = decision.getLeftDiff(); - final PsiElement rightDiff = decision.getRightDiff(); - - final String expression = "(" + conditionalExpression.getCondition().getText() + " ? " + leftDiff.getText() + " : " + rightDiff.getText() + ")"; - final PsiExpression newConditionalExpression = - JavaPsiFacade.getElementFactory(project).createExpressionFromText(expression, conditionalExpression); - - final PsiElement replacedConditionalExpression = leftDiff.replace(newConditionalExpression); - ParenthesesUtils.removeParentheses((PsiExpression)replacedConditionalExpression, false); - conditionalExpression.replace(thenExpression); + if (thenExpression == null) { + return; } - } + final PsiExpression elseExpression = conditionalExpression.getElseExpression(); + if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, elseExpression)) { + PsiReplacementUtil.replaceExpression(conditionalExpression, thenExpression.getText()); + } + } } @Override @@ -121,17 +73,18 @@ public class ConditionalExpressionWithIdenticalBranchesInspection extends BaseIn return new ConditionalExpressionWithIdenticalBranchesVisitor(); } - private class ConditionalExpressionWithIdenticalBranchesVisitor extends BaseInspectionVisitor { + private static class ConditionalExpressionWithIdenticalBranchesVisitor extends BaseInspectionVisitor { @Override public void visitConditionalExpression(PsiConditionalExpression expression) { super.visitConditionalExpression(expression); final PsiExpression thenExpression = expression.getThenExpression(); + if (thenExpression == null) { + return; + } final PsiExpression elseExpression = expression.getElseExpression(); - final EquivalenceChecker.Match decision = EquivalenceChecker.getCanonicalPsiEquivalence() - .expressionsMatch(thenExpression, elseExpression); - if (thenExpression != null && (myReportOnlyExactlyIdentical ? decision.isExactMatch() : !decision.isExactMismatch())) { - registerError(expression, expression, decision); + if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, elseExpression)) { + registerError(expression); } } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalCanBePushedInsideExpression.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalCanBePushedInsideExpression.html new file mode 100644 index 000000000000..40df1a5589af --- /dev/null +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalCanBePushedInsideExpression.html @@ -0,0 +1,12 @@ + + +Reports conditional expressions with then and else branches so similar that the conditional expression can be pushed inside, thereby shortening the code. +

For example the following conditional expression: + 

condition ? message("value: " + 1) : message("value: " + 2)
+Can be pushed inside and transformed into: + 
message("value: " + (condition ? 1 : 2))
+ +

+ New in 2017.2 + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalExpressionWithIdenticalBranches.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalExpressionWithIdenticalBranches.html index 84661085ecef..9ce40e84fa0f 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalExpressionWithIdenticalBranches.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ConditionalExpressionWithIdenticalBranches.html @@ -1,8 +1,7 @@ -Reports conditional expressions -with identical or similar "then" and "else" branches. Such expressions are almost certainly -programmer error. +Reports conditional expressions with identical then and else branches. +Such expressions are almost certainly a mistake.

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_can_be_pushed_inside_expression/ConditionalCanBePushedInsideExpression.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_can_be_pushed_inside_expression/ConditionalCanBePushedInsideExpression.java new file mode 100644 index 000000000000..d866ee29c11c --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_can_be_pushed_inside_expression/ConditionalCanBePushedInsideExpression.java @@ -0,0 +1,56 @@ +import java.util.Random; + +class ConditionalCanBePushedInsideExpression { + + void fuzzy() { + String someString = new Random().nextBoolean() ? "2" + "q" + "1" : "2" + "qwe" + "1"; + } + + void fuzzy2() { + Object someString = new Random().nextBoolean() ? (Object) "1" : (Object) "2"; + } + + void fuzzy3() { + Object someString = new Random().nextBoolean() ? "21" + (Object) "1" : "21" + (Object) "2"; + } + + void fuzzy4(int[] ints) { + int i = new Random().nextBoolean() ? ints[3] : ints[4]; + } + + void fuzzy5(String[] strings) { + String s = new Random().nextBoolean()? "asd" + strings[2] : "qwe" + strings[2]; + } + + void fuzzy6() { + int j = new Random().nextBoolean() ? 6 + someMethod("123", "") : 6 + someMethod("321", ""); + } + + void fuzzy7(int k) { + int i = k == 10 ? singleParameterMethod("one") : singleParameterMethod("two"); + } + + int someMethod(String s, String s2) { + return s.length(); + } + + int singleParameterMethod(String s) { + return s.length(); + } + + class Item { + Item(String name) { + } + + Item(int value) { + } + + void v() { + int i = 1; + Item item = (i == 1 ? new Item("1") : new Item(i)); // warning here + + Item item1 = (i == 1 ? new Item("1") : new Item("2")); // warning here + } + } + +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_expression_with_identical_branches/ConditionalExpressionWithIdenticalBranches.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_expression_with_identical_branches/ConditionalExpressionWithIdenticalBranches.java index b25ba92e5d3f..545a4aa3bb57 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_expression_with_identical_branches/ConditionalExpressionWithIdenticalBranches.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/conditional_expression_with_identical_branches/ConditionalExpressionWithIdenticalBranches.java @@ -1,7 +1,5 @@ package com.siyeh.igtest.controlflow.conditional_expression_with_identical_branches; -import java.util.Random; - class ConditionalExpressionWithIdenticalBranches { int one(boolean b) { @@ -20,49 +18,10 @@ class ConditionalExpressionWithIdenticalBranches { return b? } - void fuzzy() { - String someString = new Random().nextBoolean() ? "2" + "q" + "1" : "2" + "qwe" + "1"; - } - - void fuzzy2() { - Object someString = new Random().nextBoolean() ? (Object) "1" : (Object) "2"; - } - - void fuzzy3() { - Object someString = new Random().nextBoolean() ? "21" + (Object) "1" : "21" + (Object) "2"; - } - - void fuzzy4(int[] ints) { - int i = new Random().nextBoolean() ? ints[3] : ints[4]; - } - - void fuzzy5(String[] strings) { - String s = new Random().nextBoolean()? "asd" + strings[2] : "qwe" + strings[2]; - } - - void fuzzy6() { - int j = new Random().nextBoolean() ? 6 + someMethod("123", "") : 6 + someMethod("321", ""); - } - int someMethod(String s, String s2) { return s.length(); } - class Item { - Item(String name) { - } - - Item(int value) { - } - - void v() { - int i = 1; - Item item = (i == 1 ? new Item("1") : new Item(i)); // warning here - - Item item1 = (i == 1 ? new Item("1") : new Item("2")); // warning here - } - } - class A { private String test(String... s) { return ""; diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspectionTest.java new file mode 100644 index 000000000000..ca5761c8f837 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/ConditionalCanBePushedInsideExpressionInspectionTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.controlflow; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.Nullable; + +/** + * @author Bas Leijdekkers + */ +public class ConditionalCanBePushedInsideExpressionInspectionTest extends LightInspectionTestCase { + + public void testConditionalCanBePushedInsideExpression() { + doTest(); + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + return new ConditionalCanBePushedInsideExpressionInspection(); + } +}