diff --git a/java/java-tests/testData/inspection/commonIfParts/afterLastStatementIsCallInfo.java b/java/java-tests/testData/inspection/commonIfParts/afterLastStatementIsCallInfo.java new file mode 100644 index 000000000000..558547c8996b --- /dev/null +++ b/java/java-tests/testData/inspection/commonIfParts/afterLastStatementIsCallInfo.java @@ -0,0 +1,17 @@ +// "Extract common part from 'if'" "INFORMATION" + +class X { + void foo() { + if (true) { + another(1); + } + else { + another(2); + } + bar(); + } + + void another(int i) {} + + void bar() {} +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/commonIfParts/beforeLastStatementIsCallInfo.java b/java/java-tests/testData/inspection/commonIfParts/beforeLastStatementIsCallInfo.java new file mode 100644 index 000000000000..aead33389ec1 --- /dev/null +++ b/java/java-tests/testData/inspection/commonIfParts/beforeLastStatementIsCallInfo.java @@ -0,0 +1,18 @@ +// "Extract common part from 'if'" "INFORMATION" + +class X { + void foo() { + if (true) { + another(1); + bar(); + } + else { + another(2); + bar(); + } + } + + void another(int i) {} + + void bar() {} +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspection.java index 539a35ffc037..65e0328e34fa 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspection.java @@ -17,6 +17,7 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInsight.BlockUtils; import com.intellij.codeInspection.*; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; @@ -29,6 +30,7 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import javax.swing.*; import java.util.*; import java.util.stream.Collectors; @@ -36,12 +38,21 @@ import static com.intellij.util.ObjectUtils.tryCast; // Not really with identical branches, but also common parts public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJavaLocalInspectionTool { + public boolean myHighlightWhenLastStatementIsCall = true; + private static final List ourInspectors = new ArrayList<>(Arrays.asList( ImplicitElse::inspect, ThenElse::inspect, ElseIf::inspect )); + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel("Highlight when last common statement is call", this, "myHighlightWhenLastStatementIsCall"); + } + @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { @@ -51,7 +62,7 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava PsiStatement[] thenStatements = unwrap(ifStatement.getThenBranch()); PsiStatement[] elseStatements = unwrap(ifStatement.getElseBranch()); for (IfStatementInspector inspector : ourInspectors) { - IfInspectionResult result = inspector.inspect(ifStatement, thenStatements, elseStatements, isOnTheFly); + IfInspectionResult result = inspector.inspect(ifStatement, thenStatements, elseStatements, isOnTheFly, myHighlightWhenLastStatementIsCall); if (result != null) { ProblemHighlightType highlightType; if (result.myIsWarning) { @@ -73,7 +84,8 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava @Nullable IfInspectionResult inspect(@NotNull PsiIfStatement ifStatement, @NotNull PsiStatement[] thenBranch, @NotNull PsiStatement[] elseBranch, - boolean isOnTheFly); + boolean isOnTheFly, + boolean highlightWhenLastStatementIsCall); } private static class IfInspectionResult { @@ -175,7 +187,10 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava private final boolean myMayChangeSemantics; private final boolean myIsOnTheFly; - private ExtractCommonIfPartsFix(CommonPartType type, boolean mayChangeSemantics, boolean isOnTheFly) { + + private ExtractCommonIfPartsFix(CommonPartType type, + boolean mayChangeSemantics, + boolean isOnTheFly) { myType = type; myMayChangeSemantics = mayChangeSemantics; myIsOnTheFly = isOnTheFly; @@ -631,7 +646,8 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava @Nullable static IfInspectionResult inspect(@NotNull PsiIfStatement ifStatement, @NotNull PsiStatement[] thenBranch, @NotNull PsiStatement[] elseBranch, - boolean isOnTheFly) { + boolean isOnTheFly, + boolean highlightWhenLastStatementIsCall) { ImplicitElse implicitElse = from(thenBranch, elseBranch, ifStatement); if (implicitElse == null) return null; CommonPartType type = implicitElse.getType(); @@ -807,21 +823,36 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava return new ThenElse(headCommonParts, tailCommonParts, mayChangeSemantics, type, substitutionTable); } + private static boolean isSingleCallTail(List tail) { + if (tail.size() != 1) return false; + PsiExpressionStatement expressionStatement = tryCast(tail.get(0), PsiExpressionStatement.class); + if (expressionStatement == null) return false; + PsiMethodCallExpression call = tryCast(expressionStatement.getExpression(), PsiMethodCallExpression.class); + return call != null; + } + @Nullable static IfInspectionResult inspect(@NotNull PsiIfStatement ifStatement, @NotNull PsiStatement[] thenBranch, @NotNull PsiStatement[] elseBranch, - boolean isOnTheFly) { + boolean isOnTheFly, + boolean highlightWhenLastStatementIsCall) { ThenElse thenElse = from(ifStatement, thenBranch, elseBranch, isOnTheFly); if (thenElse == null) return null; boolean isNotInCodeBlock = !(ifStatement.getParent() instanceof PsiCodeBlock); boolean mayChangeSemantics = thenElse.myMayChangeSemantics; CommonPartType type = thenElse.myCommonPartType; ExtractCommonIfPartsFix fix = new ExtractCommonIfPartsFix(type, mayChangeSemantics, isOnTheFly); - boolean isInfoLevel = mayChangeSemantics || isNotInCodeBlock; + boolean tailStatementIsSingleCall = !highlightWhenLastStatementIsCall + && isSingleCallTail(thenElse.myTailStatementsOfThen) + && thenElse.myHeadUnitsOfThen.isEmpty(); + boolean isInfoLevel = mayChangeSemantics + || isNotInCodeBlock + || type == CommonPartType.WITH_VARIABLES_EXTRACT + || tailStatementIsSingleCall; PsiElement elementToHighlight = isInfoLevel ? ifStatement : ifStatement.getFirstChild(); if (type == CommonPartType.VARIABLES_ONLY && !isOnTheFly) return null; - return new IfInspectionResult(elementToHighlight, type != CommonPartType.WITH_VARIABLES_EXTRACT && !isInfoLevel, fix, + return new IfInspectionResult(elementToHighlight, !isInfoLevel, fix, type.getDescriptionMessage(mayChangeSemantics)); } @@ -1038,7 +1069,8 @@ public class IfStatementWithIdenticalBranchesInspection extends AbstractBaseJava @Nullable static IfInspectionResult inspect(@NotNull PsiIfStatement ifStatement, @NotNull PsiStatement[] thenBranch, @NotNull PsiStatement[] elseBranch, - boolean isOnTheFly) { + boolean isOnTheFly, + boolean highlightWhenLastStatementIsCall) { ElseIf elseIf = from(ifStatement, thenBranch); if (elseIf == null) return null; String message = InspectionsBundle.message("inspection.common.if.parts.family.else.if.description"); diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspectionTest.java index 7796706f224d..fa03660411f4 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/IfStatementWithIdenticalBranchesInspectionTest.java @@ -10,6 +10,7 @@ public class IfStatementWithIdenticalBranchesInspectionTest extends LightQuickFi @Override protected LocalInspectionTool[] configureLocalInspectionTools() { IfStatementWithIdenticalBranchesInspection inspection = new IfStatementWithIdenticalBranchesInspection(); + inspection.myHighlightWhenLastStatementIsCall = !getTestName(false).equals("LastStatementIsCallInfo.java"); return new LocalInspectionTool[]{inspection}; }