diff --git a/java/typeMigration/src/META-INF/TypeMigration.xml b/java/typeMigration/src/META-INF/TypeMigration.xml index c27c63eb1afc..46042d875b03 100644 --- a/java/typeMigration/src/META-INF/TypeMigration.xml +++ b/java/typeMigration/src/META-INF/TypeMigration.xml @@ -8,6 +8,7 @@ + diff --git a/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/VoidConversionRule.java b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/VoidConversionRule.java new file mode 100644 index 000000000000..27fe8801bec4 --- /dev/null +++ b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/VoidConversionRule.java @@ -0,0 +1,84 @@ +/* + * Copyright 2000-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.refactoring.typeMigration.rules; + +import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.psi.*; +import com.intellij.psi.search.PsiElementProcessor; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.refactoring.typeMigration.TypeConversionDescriptorBase; +import com.intellij.refactoring.typeMigration.TypeEvaluator; +import com.intellij.refactoring.typeMigration.TypeMigrationLabeler; +import com.intellij.util.IncorrectOperationException; +import com.siyeh.ig.controlflow.UnnecessaryReturnInspection; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * @author Dmitry Batkovich + */ +public class VoidConversionRule extends TypeConversionRule { + @Nullable + @Override + public TypeConversionDescriptorBase findConversion(PsiType from, + PsiType to, + PsiMember member, + PsiExpression context, + TypeMigrationLabeler labeler) { + if (PsiType.VOID.equals(to) && context.getParent() instanceof PsiReturnStatement) { + final boolean isPure = PsiTreeUtil.processElements(context, new PsiElementProcessor() { + @Override + public boolean execute(@NotNull PsiElement element) { + if (element instanceof PsiPrefixExpression) { + return analyzeUnaryExpressionOperand(((PsiPrefixExpression)element).getOperand()); + } + if (element instanceof PsiPostfixExpression) { + return analyzeUnaryExpressionOperand(((PsiPostfixExpression)element).getOperand()); + } + if (element instanceof PsiMethodCallExpression) { + final PsiMethod method = ((PsiMethodCallExpression)element).resolveMethod(); + return method != null && ControlFlowAnalyzer.isPure(method); + } + return true; + } + + private boolean analyzeUnaryExpressionOperand(PsiExpression operand) { + if (!(operand instanceof PsiReferenceExpression)) return false; + final PsiElement resolved = ((PsiReferenceExpression)operand).resolve(); + return !(resolved instanceof PsiField); + } + }); + if (isPure) { + return new TypeConversionDescriptorBase() { + @Override + public PsiExpression replace(PsiExpression expression, @NotNull TypeEvaluator evaluator) throws IncorrectOperationException { + final PsiElement parent = expression.getParent(); + if (parent instanceof PsiReturnStatement) { + expression.delete(); + if (UnnecessaryReturnInspection.isReturnRedundant((PsiReturnStatement)parent, false, null)) { + parent.delete(); + } + } + return null; + } + }; + } else { + return null; + } + } + return null; + } +} diff --git a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java index 2d999892902a..e91b68c1ff1d 100644 --- a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java +++ b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java @@ -826,6 +826,10 @@ public class TypeMigrationTest extends TypeMigrationTestBase { doTestFieldType("fooDontMigrateName", PsiType.BOOLEAN); } + public void testMethodMigrationToVoidWithUnusedReturns() { + doTestMethodType("toVoidMethod", PsiType.VOID); + } + public void testMigrationToSuper() { doTestFieldType("b", myJavaFacade.getElementFactory().createTypeFromText("Test.A", null)); } diff --git a/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/Test.items new file mode 100644 index 000000000000..af6da3191a83 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/Test.items @@ -0,0 +1,9 @@ +Types: +PsiMethod:toVoidMethod : void +PsiMethodCallExpression:toVoidMethod() : void + +Conversions: + +New expression type changes: +Fails: +"" + String.valueOf(999)->void diff --git a/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/test.java new file mode 100644 index 000000000000..9a41034f67a0 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/after/test.java @@ -0,0 +1,11 @@ +class Test { + + public void toVoidMethod() { + int j = 0; + return "" + String.valueOf(999); + } + + public void main(String[] args) { + toVoidMethod(); + } +} diff --git a/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/before/test.java new file mode 100644 index 000000000000..dc030e222267 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/methodMigrationToVoidWithUnusedReturns/before/test.java @@ -0,0 +1,11 @@ +class Test { + + public String toVoidMethod() { + int j = 0; + return "" + String.valueOf(999); + } + + public void main(String[] args) { + toVoidMethod(); + } +} diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryReturnInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryReturnInspection.java index 5a91e861441a..193be4ee448f 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryReturnInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryReturnInspection.java @@ -16,6 +16,7 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; +import com.intellij.openapi.util.Ref; import com.intellij.psi.*; import com.intellij.psi.util.FileTypeUtils; import com.intellij.psi.util.PsiTreeUtil; @@ -26,6 +27,7 @@ import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.fixes.DeleteUnnecessaryStatementFix; import com.siyeh.ig.psiutils.ControlFlowUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import javax.swing.*; @@ -85,48 +87,59 @@ public class UnnecessaryReturnInspection extends BaseInspection { @Override public void visitReturnStatement(@NotNull PsiReturnStatement statement) { super.visitReturnStatement(statement); - if (statement.getReturnValue() != null) { - return; + final Ref constructorRef = Ref.create(); + if (isReturnRedundant(statement, ignoreInThenBranch, constructorRef)) { + registerStatementError(statement, constructorRef.get()); } - final PsiElement methodParent = PsiTreeUtil.getParentOfType(statement, PsiMethod.class, PsiLambdaExpression.class); - PsiCodeBlock codeBlock = null; - final boolean constructor; - if (methodParent instanceof PsiMethod) { - final PsiMethod method = (PsiMethod)methodParent; - codeBlock = method.getBody(); - constructor = method.isConstructor(); - } - else if (methodParent instanceof PsiLambdaExpression) { - constructor = false; - final PsiLambdaExpression lambdaExpression = (PsiLambdaExpression)methodParent; - final PsiElement lambdaBody = lambdaExpression.getBody(); - if (lambdaBody instanceof PsiCodeBlock) { - codeBlock = (PsiCodeBlock)lambdaBody; - } - } - else { - return; - } - if (codeBlock == null) { - return; - } - if (!ControlFlowUtils.blockCompletesWithStatement(codeBlock, statement)) { - return; - } - if (ignoreInThenBranch && isInThenBranch(statement)) { - return; - } - registerStatementError(statement, Boolean.valueOf(constructor)); } - private boolean isInThenBranch(PsiStatement statement) { - final PsiIfStatement ifStatement = - PsiTreeUtil.getParentOfType(statement, PsiIfStatement.class, true, PsiMethod.class, PsiLambdaExpression.class); - if (ifStatement == null) { - return false; - } - final PsiStatement elseBranch = ifStatement.getElseBranch(); - return elseBranch != null && !PsiTreeUtil.isAncestor(elseBranch, statement, true); + } + + public static boolean isReturnRedundant(@NotNull PsiReturnStatement statement, + boolean ignoreInThenBranch, + @Nullable Ref isInConstructorRef) { + if (statement.getReturnValue() != null) { + return false; } + final PsiElement methodParent = PsiTreeUtil.getParentOfType(statement, PsiMethod.class, PsiLambdaExpression.class); + PsiCodeBlock codeBlock = null; + if (methodParent instanceof PsiMethod) { + final PsiMethod method = (PsiMethod)methodParent; + codeBlock = method.getBody(); + if (isInConstructorRef != null) { + isInConstructorRef.set(method.isConstructor()); + } + } + else if (methodParent instanceof PsiLambdaExpression) { + isInConstructorRef.set(false); + final PsiLambdaExpression lambdaExpression = (PsiLambdaExpression)methodParent; + final PsiElement lambdaBody = lambdaExpression.getBody(); + if (lambdaBody instanceof PsiCodeBlock) { + codeBlock = (PsiCodeBlock)lambdaBody; + } + } + else { + return false; + } + if (codeBlock == null) { + return false; + } + if (!ControlFlowUtils.blockCompletesWithStatement(codeBlock, statement)) { + return false; + } + if (ignoreInThenBranch && isInThenBranch(statement)) { + return false; + } + return true; + } + + private static boolean isInThenBranch(PsiStatement statement) { + final PsiIfStatement ifStatement = + PsiTreeUtil.getParentOfType(statement, PsiIfStatement.class, true, PsiMethod.class, PsiLambdaExpression.class); + if (ifStatement == null) { + return false; + } + final PsiStatement elseBranch = ifStatement.getElseBranch(); + return elseBranch != null && !PsiTreeUtil.isAncestor(elseBranch, statement, true); } } \ No newline at end of file