From 723c05545541a669284d152ed8a64476eb5f1e7b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 19 Apr 2019 13:26:43 +0700 Subject: [PATCH] FinishMarker: use the first movable value when mayNeedMarker is true (may generate shorter code) --- .../impl/singlereturn/ExitContext.java | 4 ++-- .../impl/singlereturn/FinishMarker.java | 19 ++++++++++++------- .../afterChangedParameter.java | 3 +-- .../afterDefaultValue.java | 18 ++++++++++++++++++ .../beforeDefaultValue.java | 10 ++++++++++ 5 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterDefaultValue.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeDefaultValue.java diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java index 599027194ab6..6addfaa8e65e 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/singlereturn/ExitContext.java @@ -27,8 +27,8 @@ class ExitContext { private final @NotNull PsiCodeBlock myBlock; private final @NotNull String myReturnVariable; private final @NotNull PsiElementFactory myFactory; - boolean myReturnVariableUsed = false; - final PsiExpression myReturnVariableDefaultValue; + private boolean myReturnVariableUsed = false; + private final PsiExpression myReturnVariableDefaultValue; ExitContext(@NotNull PsiCodeBlock block, @NotNull PsiType returnType, @NotNull FinishMarker marker) { myBlock = block; 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 d192bda7f1b1..edc357a7a377 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 @@ -147,7 +147,7 @@ public class FinishMarker { if (nonTerminalReturnValues.size() == 1 && nonTerminalReturnValues.iterator().next() != NULL) { return new FinishMarker(FinishMarkerType.SEPARATE_VAR, nonTerminalReturns.iterator().next()); } - return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns)); + return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns, mayNeedMarker)); } if (PsiType.BOOLEAN.equals(returnType)) { if (nonTerminalReturnValues.size() == 1) { @@ -160,7 +160,7 @@ public class FinishMarker { } } if (PsiType.INT.equals(returnType) || PsiType.LONG.equals(returnType)) { - return getMarkerForIntegral(nonTerminalReturns, terminalReturn, returnType, factory); + return getMarkerForIntegral(nonTerminalReturns, terminalReturn, mayNeedMarker, returnType, factory); } if (!(returnType instanceof PsiPrimitiveType)) { if (StreamEx.of(nonTerminalReturns).map(ret -> NullabilityUtil.getExpressionNullability(ret, true)) @@ -174,11 +174,13 @@ public class FinishMarker { return new FinishMarker(FinishMarkerType.SEPARATE_VAR, value); } } - return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns)); + return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns, mayNeedMarker)); } @Nullable - private static PsiExpression findBestExpression(PsiReturnStatement terminalReturn, List nonTerminalReturns) { + private static PsiExpression findBestExpression(PsiReturnStatement terminalReturn, + List nonTerminalReturns, + boolean mayNeedMarker) { List bestGroup = StreamEx.of(nonTerminalReturns) .filter(FinishMarker::canMoveToStart) .groupingBy(PsiExpression::getText, LinkedHashMap::new, Collectors.toList()) @@ -192,13 +194,16 @@ public class FinishMarker { if (terminalReturn != null && canMoveToStart(terminalReturn.getReturnValue())) { return terminalReturn.getReturnValue(); } + if (mayNeedMarker && !bestGroup.isEmpty()) { + return bestGroup.get(0); + } return null; } @NotNull private static FinishMarker getMarkerForIntegral(List nonTerminalReturns, PsiReturnStatement terminalReturn, - PsiType returnType, PsiElementFactory factory) { + boolean mayNeedMarker, PsiType returnType, PsiElementFactory factory) { boolean isLong = PsiType.LONG.equals(returnType); LongRangeSet fullSet = requireNonNull(LongRangeSet.fromType(returnType)); LongRangeSet set = nonTerminalReturns.stream() @@ -236,11 +241,11 @@ public class FinishMarker { return new FinishMarker(FinishMarkerType.VALUE_NON_EQUAL, factory.createExpressionFromText(text, null)); } } - return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns)); + return new FinishMarker(FinishMarkerType.SEPARATE_VAR, findBestExpression(terminalReturn, nonTerminalReturns, mayNeedMarker)); } @Contract("null -> false") - static boolean canMoveToStart(PsiExpression value) { + private static boolean canMoveToStart(PsiExpression value) { if (!ExpressionUtils.isSafelyRecomputableExpression(value)) return false; PsiReferenceExpression ref = tryCast(PsiUtil.skipParenthesizedExprDown(value), PsiReferenceExpression.class); if (ref != null && !ref.isQualified()) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterChangedParameter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterChangedParameter.java index 16249f260997..fc236e8d7572 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterChangedParameter.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterChangedParameter.java @@ -1,13 +1,12 @@ // "Transform body to single exit-point form" "true" class Test { String test2(List list, String foo, String bar) { - String result = null; + String result = foo; boolean finished = false; for (String s : list) { for (int i = 0; i < 10; i++) { bar = s; if (s.length() == i) { - result = foo; finished = true; break; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterDefaultValue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterDefaultValue.java new file mode 100644 index 000000000000..1305e703e8c7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/afterDefaultValue.java @@ -0,0 +1,18 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int i) { + String result = null; + boolean finished = false; + if (i > 0) { + if (i == 10) { + finished = true; + } else { + System.out.println(i); + } + } + if (!finished) { + result = String.valueOf(i); + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeDefaultValue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeDefaultValue.java new file mode 100644 index 000000000000..363d495ebcc7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convertToSingleReturn/beforeDefaultValue.java @@ -0,0 +1,10 @@ +// "Transform body to single exit-point form" "true" +class Test { + String test(int i) { + if (i > 0) { + if (i == 10) return null; + System.out.println(i); + } + return String.valueOf(i); + } +} \ No newline at end of file