From e7597f253a9a3562983832481b2b774921d59ade Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Thu, 16 Nov 2017 12:49:25 +0300 Subject: [PATCH] Java: Detect trivial duplicates containing conditional 'return' (IDEA-182195) --- .../ConditionalReturnStatementValue.java | 6 ++++ .../util/duplicates/DuplicatesFinder.java | 4 ++- .../NullableCheckVoidDuplicate.java | 27 +++++++++++++++++ .../NullableCheckVoidDuplicate_after.java | 29 +++++++++++++++++++ .../OutputVariableDuplicate.java | 19 ++++++++++++ .../OutputVariableDuplicate_after.java | 26 +++++++++++++++++ .../java/refactoring/ExtractMethodTest.java | 7 +++++ 7 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java index 3878619c6db9..23fa7c3d8e62 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ConditionalReturnStatementValue.java @@ -18,7 +18,9 @@ package com.intellij.refactoring.util.duplicates; import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.util.PsiUtil; import com.intellij.util.IncorrectOperationException; +import com.siyeh.ig.psiutils.ExpressionUtils; /** * @author ven @@ -57,4 +59,8 @@ public class ConditionalReturnStatementValue implements ReturnValue { condition.replace(methodCallExpression); return (PsiStatement)CodeStyleManager.getInstance(statement.getManager().getProject()).reformat(statement); } + + public boolean isEmptyOrConstantExpression() { + return myReturnValue == null || ExpressionUtils.isNullLiteral(myReturnValue) || PsiUtil.isConstantExpression(myReturnValue); + } } diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java index fad7fe3a67b0..f89565ccd385 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java @@ -267,7 +267,9 @@ public class DuplicatesFinder { if (returnValue == null) { returnValue = myReturnValue; } - if (returnValue instanceof GotoReturnValue) { + if (returnValue instanceof GotoReturnValue || + returnValue instanceof ConditionalReturnStatementValue && + ((ConditionalReturnStatementValue)returnValue).isEmptyOrConstantExpression()) { return false; } if (returnValue instanceof VariableReturnValue) { diff --git a/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate.java new file mode 100644 index 000000000000..daa8e9c75c30 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate.java @@ -0,0 +1,27 @@ +public class NullableCheckVoidDuplicate { + void foo() { + Object o = ""; + for (int i = 0; i < 5; i++) { + if (i == 10) { + o = null; + } + } + if (o == null) { + return; + } + System.out.println(o); + } + + void bar() { + Object o = ""; + for (int i = 0; i < 5; i++) { + if (i == 10) { + o = null; + } + } + if (o == null) { + return; + } + System.out.println(o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate_after.java new file mode 100644 index 000000000000..d8806dbca852 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NullableCheckVoidDuplicate_after.java @@ -0,0 +1,29 @@ +import org.jetbrains.annotations.Nullable; + +public class NullableCheckVoidDuplicate { + void foo() { + Object o = newMethod(); + if (o == null) return; + System.out.println(o); + } + + @Nullable + private Object newMethod() { + Object o = ""; + for (int i = 0; i < 5; i++) { + if (i == 10) { + o = null; + } + } + if (o == null) { + return null; + } + return o; + } + + void bar() { + Object o = newMethod(); + if (o == null) return; + System.out.println(o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate.java new file mode 100644 index 000000000000..09ea5f21f156 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate.java @@ -0,0 +1,19 @@ +public class OutputVariableDuplicate { + String foo() { + String var = ""; + if (var == null) { + return null; + } + System.out.println(var); + return var; + } + + String bar() { + String var = ""; + if (var == null) { + return null; + } + System.out.println(var); + return var; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate_after.java new file mode 100644 index 000000000000..17444e84d50a --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/OutputVariableDuplicate_after.java @@ -0,0 +1,26 @@ +import org.jetbrains.annotations.Nullable; + +public class OutputVariableDuplicate { + String foo() { + String var = newMethod(); + if (var == null) return null; + System.out.println(var); + return var; + } + + @Nullable + private String newMethod() { + String var = ""; + if (var == null) { + return null; + } + return var; + } + + String bar() { + String var = newMethod(); + if (var == null) return null; + System.out.println(var); + return var; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index cbdb907f3df8..425f6317ffb0 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1169,6 +1169,13 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testOutputVariableDuplicate() throws Exception { + doDuplicatesTest(); + } + + public void testNullableCheckVoidDuplicate() throws Exception { + doTest(); + } private void doTestDisabledParam() throws PrepareFailedException { final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject());