From 30b914be2860788448552941284eba50ce9af0b1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 16 Jul 2019 18:27:10 +0700 Subject: [PATCH] IDEA-216810 Unexpected behaviour with "Transform Method to Single Exit Point" / Inline refactoring GitOrigin-RevId: 31a77ebb86f3da40caeda5e8ca60ccec51ebfb13 --- .../impl/singlereturn/FinishMarker.java | 31 +++++++++++-------- .../afterOneBranchDefault.java | 10 ++++++ .../afterTwoBranchesDefault.java | 12 +++++++ .../beforeOneBranchDefault.java | 9 ++++++ .../beforeTwoBranchesDefault.java | 11 +++++++ 5 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterOneBranchDefault.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTwoBranchesDefault.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeOneBranchDefault.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTwoBranchesDefault.java diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java index 0cd11f47634d..dfbf2adf0acb 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/FinishMarker.java @@ -93,19 +93,23 @@ public class FinishMarker { currentContext = loopOrSwitch; } else { - while (true) { + while (currentContext instanceof PsiIfStatement) { PsiElement ifParent = currentContext.getParent(); - if (!(ifParent instanceof PsiCodeBlock)) break; - if (!(ifParent.getParent() instanceof PsiStatement)) { - return ifParent != block; + if (ifParent instanceof PsiIfStatement) { + currentContext = (PsiStatement)ifParent; } - currentContext = (PsiStatement)ifParent.getParent(); - if (!(currentContext instanceof PsiBlockStatement) || - !(currentContext.getParent() instanceof PsiIfStatement) || - ControlFlowUtils.codeBlockMayCompleteNormally((PsiCodeBlock)ifParent)) { - break; + else if (ifParent instanceof PsiCodeBlock) { + if (!(ifParent.getParent() instanceof PsiStatement)) { + return ifParent != block; + } + currentContext = (PsiStatement)ifParent.getParent(); + if (!(currentContext instanceof PsiBlockStatement) || + !(currentContext.getParent() instanceof PsiIfStatement) || + ControlFlowUtils.codeBlockMayCompleteNormally((PsiCodeBlock)ifParent)) { + break; + } + currentContext = (PsiStatement)currentContext.getParent(); } - currentContext = (PsiStatement)currentContext.getParent(); } } while (true) { @@ -144,10 +148,11 @@ public class FinishMarker { .map(val -> val instanceof PsiLiteralExpression ? ((PsiLiteralExpression)val).getValue() : NULL) .toSet(); if (!mayNeedMarker) { - if (nonTerminalReturnValues.size() == 1 && nonTerminalReturnValues.iterator().next() != NULL) { - return new FinishMarker(FinishMarkerType.SEPARATE_VAR, nonTerminalReturns.iterator().next()); + PsiExpression initValue = findBestExpression(terminalReturn, nonTerminalReturns, mayNeedMarker); + if (initValue == null && nonTerminalReturnValues.size() == 1 && nonTerminalReturnValues.iterator().next() != NULL) { + initValue = nonTerminalReturns.iterator().next(); } - return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns, mayNeedMarker)); + return new FinishMarker(FinishMarkerType.SEPARATE_VAR, initValue); } if (PsiType.BOOLEAN.equals(returnType)) { if (nonTerminalReturnValues.size() == 1) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterOneBranchDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterOneBranchDefault.java new file mode 100644 index 000000000000..1483ce3f3a91 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterOneBranchDefault.java @@ -0,0 +1,10 @@ +// "Transform body to single exit-point form" "true" +class Test { + private String nameByIndex(int colourIndex) { + String result = "Blue"; + if (colourIndex == 1) { + result = "Red"; + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTwoBranchesDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTwoBranchesDefault.java new file mode 100644 index 000000000000..edf425dd94c8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterTwoBranchesDefault.java @@ -0,0 +1,12 @@ +// "Transform body to single exit-point form" "true" +class Test { + private String nameByIndex(int colourIndex) { + String result = "Blue"; + if (colourIndex == 1) { + result = "Red"; + } else if (colourIndex == 2) { + result = "Green"; + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeOneBranchDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeOneBranchDefault.java new file mode 100644 index 000000000000..44f0c6c6503c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeOneBranchDefault.java @@ -0,0 +1,9 @@ +// "Transform body to single exit-point form" "true" +class Test { + private String nameByIndex(int colourIndex) { + if (colourIndex == 1) { + return "Red"; + } + return "Blue"; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTwoBranchesDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTwoBranchesDefault.java new file mode 100644 index 000000000000..efe1ff61cde2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeTwoBranchesDefault.java @@ -0,0 +1,11 @@ +// "Transform body to single exit-point form" "true" +class Test { + private String nameByIndex(int colourIndex) { + if (colourIndex == 1) { + return "Red"; + } else if (colourIndex == 2) { + return "Green"; + } + return "Blue"; + } +} \ No newline at end of file