From f3d22142cd710d805850eb3ac6504fd961bac025 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 7 Sep 2016 18:48:46 +0300 Subject: [PATCH] Java inspection: Don't look for assignment chains in 'switch' and loop statements in "Move return to computation" inspection (IDEA-121153) --- ...urnSeparatedFromComputationInspection.java | 8 +++++- .../afterSwitchThrow.java | 23 ++++++++++++++++ .../beforeSwitchThrow.java | 27 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSwitchThrow.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSwitchThrow.java diff --git a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java index d17fe690b12f..af3d772815e4 100644 --- a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java @@ -383,7 +383,8 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal PsiJavaToken rBrace = codeBlock.getRBrace(); if (rBrace != null) { PsiStatement lastNonEmptyStatement = getPrevNonEmptyStatement(rBrace, removeCompletely); - if (lastNonEmptyStatement == null || hasChainedAssignmentsInScope(flow, resultVariable, lastNonEmptyStatement)) { + if (lastNonEmptyStatement == null || + isIfBranch(codeBlock) && hasChainedAssignmentsInScope(flow, resultVariable, lastNonEmptyStatement)) { return false; } if (moveTo(lastNonEmptyStatement, returnAtTheEnd)) { @@ -507,6 +508,11 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return ExpressionUtils.computeConstantExpression(condition) == Boolean.TRUE; } + private static boolean isIfBranch(@NotNull PsiCodeBlock codeBlock) { + final PsiElement parent = codeBlock.getParent(); + return parent instanceof PsiBlockStatement && parent.getParent() instanceof PsiIfStatement; + } + private Set getBreaks(@NotNull PsiStatement targetStatement) { if (breakStatements == null) { breakStatements = new THashMap<>(); diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSwitchThrow.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSwitchThrow.java new file mode 100644 index 000000000000..52bda5b86ee7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSwitchThrow.java @@ -0,0 +1,23 @@ +// "Move 'return' closer to computation of the value of 'n'" "true" +class T { + int f(int a) { + int n = -1; + switch (a) { + case 0: + case 1: + case 2: + return n; + case 10: + case 20: + return n + 1; + case 30: + case 40: + case 50: + return 2; + case 90: + return 3; + default: + throw new IllegalArgumentException(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSwitchThrow.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSwitchThrow.java new file mode 100644 index 000000000000..5fd61be245f7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSwitchThrow.java @@ -0,0 +1,27 @@ +// "Move 'return' closer to computation of the value of 'n'" "true" +class T { + int f(int a) { + int n = -1; + switch (a) { + case 0: + case 1: + case 2: + break; + case 10: + case 20: + n = n + 1; + break; + case 30: + case 40: + case 50: + n = 2; + break; + case 90: + n = 3; + break; + default: + throw new IllegalArgumentException(); + } + return n; + } +} \ No newline at end of file