From 260d3003fef6293f1916f355388bcb70921d4f47 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 31 Aug 2016 13:14:25 +0300 Subject: [PATCH 1/2] Java inspections: Created "Move return to computation" inspection, added a few tests (IDEA-121153) --- ...urnSeparatedFromComputationInspection.java | 467 ++++++++++++++++++ .../LabeledFor.java | 13 + .../LabeledFor2.java | 11 + .../LabeledIf.java | 9 + .../NestedBlock.java | 9 + .../NestedBlockSideEffect.java | 10 + .../NestedIf.java | 11 + .../NestedIfInnerElse.java | 10 + .../NestedIfOuterElse.java | 10 + .../ReturnOutsideTryWithResources.java | 15 + .../SimpleDoWhile.java | 25 + .../SimpleFor.java | 12 + .../SimpleForeach.java | 12 + .../SimpleIf.java | 7 + .../SimpleWhile.java | 21 + .../afterBreakFromLoopInTryWithResources.java | 19 + .../afterDoWhileTrue.java | 21 + .../afterForWithoutCondition.java | 11 + .../afterIfElseWriteInBoth.java | 8 + .../afterIfElseWriteInElse.java | 9 + .../afterIfElseWriteInIf.java | 9 + .../afterLabeledBlock.java | 12 + .../afterWhileTrue.java | 14 + .../beforeAssert.java | 8 + ...beforeBreakFromLoopInTryWithResources.java | 20 + .../beforeDoWhileTrue.java | 23 + .../beforeForWithoutCondition.java | 13 + .../beforeIfElseNoWrite.java | 9 + .../beforeIfElseWriteInBoth.java | 9 + .../beforeIfElseWriteInElse.java | 9 + .../beforeIfElseWriteInIf.java | 9 + .../beforeLabeledBlock.java | 13 + .../beforeWhileTrue.java | 16 + .../Assert.java | 7 + .../BreakFromLoopInTryWithResources.java | 19 + .../DoWhileTrue.java | 22 + .../ForWithoutCondition.java | 12 + .../IfElseNoWrite.java | 8 + .../IfElseWriteInBoth.java | 8 + .../IfElseWriteInElse.java | 8 + .../IfElseWriteInIf.java | 8 + .../LabeledBlock.java | 12 + .../LabeledFor.java | 13 + .../LabeledFor2.java | 11 + .../LabeledIf.java | 9 + .../NestedBlock.java | 9 + .../NestedBlockSideEffect.java | 10 + .../NestedIf.java | 11 + .../NestedIfInnerElse.java | 10 + .../NestedIfOuterElse.java | 10 + .../ReturnOutsideTryWithResources.java | 15 + .../SideEffectInIf.java | 9 + .../SimpleDoWhile.java | 25 + .../SimpleFor.java | 12 + .../SimpleForeach.java | 12 + .../SimpleIf.java | 7 + .../SimpleWhile.java | 21 + .../WhileTrue.java | 15 + ...eturnSeparatedFromComputationFix2Test.java | 48 ++ ...ReturnSeparatedFromComputationFixTest.java | 40 ++ .../ReturnSeparatedFromComputationTest.java | 136 +++++ .../src/messages/InspectionsBundle.properties | 5 + .../ReturnSeparatedFromComputation.html | 14 + resources/src/META-INF/IdeaPlugin.xml | 4 + 64 files changed, 1424 insertions(+) create mode 100644 java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor2.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlock.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlockSideEffect.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfInnerElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfOuterElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleDoWhile.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleForeach.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterBreakFromLoopInTryWithResources.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterDoWhileTrue.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterForWithoutCondition.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInBoth.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterLabeledBlock.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterWhileTrue.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeAssert.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeBreakFromLoopInTryWithResources.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeDoWhileTrue.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeForWithoutCondition.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseNoWrite.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInBoth.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeLabeledBlock.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeWhileTrue.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/Assert.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/BreakFromLoopInTryWithResources.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/DoWhileTrue.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/ForWithoutCondition.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseNoWrite.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInBoth.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInElse.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInIf.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledBlock.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor2.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledIf.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlock.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlockSideEffect.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIf.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfInnerElse.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfOuterElse.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SideEffectInIf.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleDoWhile.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleFor.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleForeach.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleIf.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleWhile.java create mode 100644 java/java-tests/testData/inspection/returnSeparatedFromComputation/WhileTrue.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFix2Test.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFixTest.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java create mode 100644 resources-en/src/inspectionDescriptions/ReturnSeparatedFromComputation.html diff --git a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java new file mode 100644 index 000000000000..d2ef0f79b83a --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java @@ -0,0 +1,467 @@ +/* + * 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.codeInspection.intermediaryVariable; + +import com.intellij.codeInspection.*; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.controlFlow.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.refactoring.util.RefactoringUtil; +import com.siyeh.ig.psiutils.ControlFlowUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import gnu.trove.THashMap; +import gnu.trove.THashSet; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; + +/** + * @author Pavel.Dolgov + */ +public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocalInspectionTool { + private static final Logger LOG = Logger.getInstance("#" + ReturnSeparatedFromComputationInspection.class.getName()); + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitReturnStatement(PsiReturnStatement returnStatement) { + super.visitReturnStatement(returnStatement); + final ReturnContext context = createReturnContext(returnStatement); + if (context != null && isApplicable(context)) { + registerProblem(holder, returnStatement, context.returnedVariable); + } + } + }; + } + + private static ReturnContext createReturnContext(PsiReturnStatement returnStatement) { + final PsiElement returnParent = returnStatement.getParent(); + if (returnParent instanceof PsiCodeBlock) { + final PsiCodeBlock returnScope = (PsiCodeBlock)returnParent; + final PsiStatement[] statements = returnScope.getStatements(); + if (statements.length != 0 && statements[statements.length - 1] == returnStatement) { + PsiStatement refactoredStatement = getPrevNonEmptyStatement(returnStatement, new THashSet<>()); + if (refactoredStatement != null) { + final PsiExpression returnValue = returnStatement.getReturnValue(); + if (returnValue instanceof PsiReferenceExpression) { + final PsiElement resolved = ((PsiReferenceExpression)returnValue).resolve(); + if (resolved instanceof PsiVariable) { + final PsiVariable returnedVariable = (PsiVariable)resolved; + final PsiCodeBlock variableScope = getVariableScopeBlock(returnedVariable); + if (variableScope != null) { + return new ReturnContext(returnStatement, returnScope, refactoredStatement, returnedVariable, variableScope); + } + } + } + } + } + } + return null; + } + + @Nullable + private static PsiCodeBlock getVariableScopeBlock(@Nullable PsiVariable variable) { + if (variable instanceof PsiLocalVariable) { + final PsiElement variableScope = RefactoringUtil.getVariableScope((PsiLocalVariable)variable); + if (variableScope instanceof PsiCodeBlock) { + return ((PsiCodeBlock)variableScope); + } + } + else if (variable instanceof PsiParameter) { + final PsiParameter parameter = (PsiParameter)variable; + final PsiElement parameterScope = parameter.getDeclarationScope(); + if (parameterScope instanceof PsiMethod) { + return ((PsiMethod)parameterScope).getBody(); + } + else if (parameterScope instanceof PsiLambdaExpression) { + final PsiElement lambdaBody = ((PsiLambdaExpression)parameterScope).getBody(); + if (lambdaBody instanceof PsiCodeBlock) { + return (PsiCodeBlock)lambdaBody; + } + } + } + return null; + } + + private static boolean isApplicable(@NotNull ReturnContext context) { + final ControlFlow flow = createControlFlow(context); + return flow != null && isApplicable(flow, context); + } + + @Nullable + private static ControlFlow createControlFlow(@NotNull ReturnContext context) { + try { + final ControlFlowPolicy policy = new LocalsControlFlowPolicy(context.variableScope); + return ControlFlowFactory.getInstance(context.variableScope.getProject()).getControlFlow(context.variableScope, policy); + } + catch (AnalysisCanceledException e) { + return null; + } + } + + private static boolean isApplicable(@NotNull ControlFlow flow, @NotNull ReturnContext context) { + final int flowStart = flow.getStartOffset(context.returnScope); + final int flowEnd = flow.getEndOffset(context.returnScope); + if (flowStart < 0 || flowEnd < 0) return false; + + final int returnStartOffset = flow.getStartOffset(context.returnStatement); + final int returnEndOffset = flow.getEndOffset(context.returnStatement); + if (returnStartOffset < 0 || returnEndOffset < 0) return false; + + if (context.returnScope != context.variableScope && + ControlFlowUtil.hasObservableThrowExitPoints(flow, flowStart, flowEnd, + new PsiElement[]{context.refactoredStatement}, context.variableScope)) { + return false; + } + + Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable); + mover.moveTo(context.refactoredStatement); + return !mover.isEmpty(); + } + + private static void doApply(PsiReturnStatement returnStatement) { + ReturnContext context = createReturnContext(returnStatement); + if (context != null) { + ControlFlow flow = createControlFlow(context); + if (flow != null) { + Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable); + boolean removeReturn = mover.moveTo(context.refactoredStatement); + if (!mover.isEmpty()) { + applyChanges(mover, context, removeReturn); + } + } + } + } + + private static void applyChanges(@NotNull Mover mover, @NotNull ReturnContext context, boolean removeReturn) { + mover.insertAfter.forEach(e -> e.getParent().addAfter(context.returnStatement, e)); + mover.insertBefore.forEach(e -> e.getParent().addBefore(context.returnStatement, e)); + mover.replaceInline.forEach(e -> { + if (e instanceof PsiBreakStatement) e.replace(context.returnStatement); + if (e instanceof PsiAssignmentExpression) inlineAssignment((PsiAssignmentExpression)e, context.returnStatement); + }); + mover.removeCompletely.forEach(PsiElement::delete); + + if (removeReturn) { + Set skippedEmptyStatements = new THashSet<>(); + getPrevNonEmptyStatement(context.returnStatement, skippedEmptyStatements); + skippedEmptyStatements.forEach(PsiElement::delete); + context.returnStatement.delete(); + } + } + + private static void inlineAssignment(PsiAssignmentExpression assignmentExpression, PsiReturnStatement returnStatement) { + PsiElement assignmentParent = assignmentExpression.getParent(); + LOG.assertTrue(assignmentParent instanceof PsiExpressionStatement, "PsiExpressionStatement"); + PsiReturnStatement returnStatementCopy = (PsiReturnStatement)returnStatement.copy(); + PsiExpression rExpression = assignmentExpression.getRExpression(); + PsiExpression returnValue = returnStatementCopy.getReturnValue(); + if (rExpression != null && returnValue!=null) { + returnValue.replace(rExpression); + assignmentParent.replace(returnStatementCopy); + } + } + + private static class Mover { + final ControlFlow flow; + final PsiStatement enclosingStatement; + final PsiVariable resultVariable; + final Set insertBefore = new THashSet<>(); + final Set insertAfter = new THashSet<>(); + final Set replaceInline = new THashSet<>(); + final Set removeCompletely = new THashSet<>(); + + private Map> breakStatements; + + private Mover(@NotNull ControlFlow flow, @NotNull PsiStatement enclosingStatement, @NotNull PsiVariable resultVariable) { + this.flow = flow; + this.enclosingStatement = enclosingStatement; + this.resultVariable = resultVariable; + } + + boolean isEmpty() { + return insertBefore.isEmpty() && insertAfter.isEmpty() && replaceInline.isEmpty(); + } + + /** + * Returns true if the targetStatement will always exit via return/throw/etc after the transformation, + * so if the next statement is a return or a break it can be removed safely. + */ + boolean moveTo(PsiStatement targetStatement) { + if (targetStatement instanceof PsiBlockStatement) { + return moveToBlock((PsiBlockStatement)targetStatement); + } + if (targetStatement instanceof PsiIfStatement) { + return moveToIf((PsiIfStatement)targetStatement); + } + if (targetStatement instanceof PsiForStatement) { + return moveToFor((PsiForStatement)targetStatement); + } + if (targetStatement instanceof PsiWhileStatement) { + return moveToWhile((PsiWhileStatement)targetStatement); + } + if (targetStatement instanceof PsiDoWhileStatement) { + return moveToDoWhile((PsiDoWhileStatement)targetStatement); + } + if (targetStatement instanceof PsiForeachStatement) { + return moveToForeach((PsiForeachStatement)targetStatement); + } + if (targetStatement instanceof PsiTryStatement) { + return moveToTry(((PsiTryStatement)targetStatement)); + } + if (targetStatement instanceof PsiLabeledStatement) { + return moveToLabeled(((PsiLabeledStatement)targetStatement)); + } + if (targetStatement instanceof PsiExpressionStatement) { + return inlineExpression(((PsiExpressionStatement)targetStatement)); + } + return false; + } + + private boolean moveToBlock(PsiBlockStatement targetStatement) { + return moveToBlock(targetStatement.getCodeBlock()); + } + + private boolean moveToBlock(@NotNull PsiCodeBlock codeBlock) { + PsiJavaToken rBrace = codeBlock.getRBrace(); + if (rBrace != null) { + PsiStatement lastNonEmptyStatement = getPrevNonEmptyStatement(rBrace, removeCompletely); + if (lastNonEmptyStatement == null || !moveTo(lastNonEmptyStatement)) { + insertBefore.add(rBrace); + } + return true; + } + return false; + } + + private boolean moveToIf(PsiIfStatement targetStatement) { + PsiStatement thenBranch = targetStatement.getThenBranch(); + PsiStatement elseBranch = targetStatement.getElseBranch(); + + boolean thenPart = thenBranch != null && moveTo(thenBranch); + boolean elsePart = elseBranch != null && moveTo(elseBranch); + return thenPart && elsePart; + } + + private boolean moveToFor(PsiForStatement targetStatement) { + moveToBreaks(targetStatement); + return isAlwaysTrue(targetStatement.getCondition(), true); + } + + private boolean moveToDoWhile(PsiDoWhileStatement targetStatement) { + moveToBreaks(targetStatement); + return isAlwaysTrue(targetStatement.getCondition(), false); + } + + private boolean moveToWhile(PsiWhileStatement targetStatement) { + moveToBreaks(targetStatement); + return isAlwaysTrue(targetStatement.getCondition(), false); + } + private boolean moveToForeach(PsiForeachStatement targetStatement) { + moveToBreaks(targetStatement); + return false; + } + + private boolean moveToTry(PsiTryStatement targetStatement) { + PsiCodeBlock tryBlock = targetStatement.getTryBlock(); + if (tryBlock == null) { + return false; + } + PsiCodeBlock finallyBlock = targetStatement.getFinallyBlock(); + if (finallyBlock != null && ControlFlowUtils.codeBlockMayCompleteNormally(finallyBlock) && writesVariable(finallyBlock)) { + return false; + } + PsiCatchSection[] catchSections = targetStatement.getCatchSections(); + for (PsiCatchSection catchSection : catchSections) { + PsiCodeBlock catchBlock = catchSection.getCatchBlock(); + if (catchBlock != null && ControlFlowUtils.codeBlockMayCompleteNormally(catchBlock) && writesVariable(finallyBlock)) { + return false; + } + } + return moveToBlock(tryBlock); + } + + private boolean moveToLabeled(PsiLabeledStatement targetStatement) { + PsiStatement statement = targetStatement.getStatement(); + if (statement == null) { + return false; + } + moveToBreaks(statement); + return moveTo(statement); + } + + private boolean inlineExpression(PsiExpressionStatement statement) { + PsiExpression expression = statement.getExpression(); + if (expression instanceof PsiAssignmentExpression) { + PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)expression; + PsiExpression lExpression = assignmentExpression.getLExpression(); + if (lExpression instanceof PsiReferenceExpression) { + PsiReferenceExpression referenceExpression = (PsiReferenceExpression)lExpression; + if (!referenceExpression.isQualified() && referenceExpression.resolve() == resultVariable) { + if (assignmentExpression.getOperationTokenType() == JavaTokenType.EQ) { + replaceInline.add(assignmentExpression); + return true; + } + } + } + } + return false; + } + + private void moveToBreaks(Set breaks) { + for (PsiBreakStatement breakStatement : breaks) { + PsiStatement prevNonEmptyStatement = getPrevNonEmptyStatement(breakStatement, removeCompletely); + if (prevNonEmptyStatement == null || !moveTo(prevNonEmptyStatement)) { + replaceInline.add(breakStatement); + } + else { + removeCompletely.add(breakStatement); + } + } + } + + private void moveToBreaks(PsiStatement targetStatement) { + Set breaks = getBreaks(targetStatement); + moveToBreaks(breaks); + } + + private boolean writesVariable(PsiElement element) { + int startOffset = flow.getStartOffset(element); + int endOffset = flow.getEndOffset(element); + if (startOffset < 0 || endOffset < 0) { + return true; + } + List instructions = flow.getInstructions(); + for (int i = startOffset; i < endOffset; i++) { + Instruction instruction = instructions.get(i); + if (instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == resultVariable) { + return true; + } + } + return false; + } + + private static boolean isAlwaysTrue(@Nullable PsiExpression condition, boolean nullIsTrue) { + if(condition == null) return nullIsTrue; + return ExpressionUtils.computeConstantExpression(condition) == Boolean.TRUE; + } + + private Set getBreaks(PsiStatement targetStatement) { + if (breakStatements == null) { + breakStatements = new THashMap<>(); + List instructions = flow.getInstructions(); + for (int i = 0; i < instructions.size(); i++) { + PsiElement element = flow.getElement(i); + PsiStatement statement = getNearestEnclosingStatement(element); + if (statement instanceof PsiBreakStatement) { + PsiStatement exitedStatement = ((PsiBreakStatement)statement).findExitedStatement(); + if (exitedStatement != null) { + breakStatements.computeIfAbsent(exitedStatement, unused -> new THashSet<>()).add((PsiBreakStatement)statement); + } + } + } + } + Set breaks = breakStatements.get(targetStatement); + return breaks != null ? breaks : Collections.emptySet(); + } + + @Nullable + private static PsiStatement getNearestEnclosingStatement(PsiElement element) { + return element instanceof PsiStatement ? (PsiStatement)element : PsiTreeUtil.getParentOfType(element, PsiStatement.class); + } + + } + + private static PsiStatement getPrevNonEmptyStatement(PsiElement psiElement, Set skippedEmptyStatements) { + PsiStatement prevStatement = PsiTreeUtil.getPrevSiblingOfType(psiElement, PsiStatement.class); + List skipped = new ArrayList<>(); + while (prevStatement instanceof PsiEmptyStatement) { + skipped.add(prevStatement); + prevStatement = PsiTreeUtil.getPrevSiblingOfType(prevStatement, PsiStatement.class); + } + if (prevStatement != null) { + skippedEmptyStatements.addAll(skipped); + } + return prevStatement; + } + + private static void registerProblem(@NotNull ProblemsHolder holder, + @NotNull PsiReturnStatement returnStatement, + @NotNull PsiVariable variable) { + String name = variable.getName(); + holder.registerProblem(returnStatement, InspectionsBundle.message("inspection.return.separated.from.computation.descriptor", name), + new VariableFix(name, variable instanceof PsiParameter)); + } + + private static class VariableFix implements LocalQuickFix { + private String myName; + private boolean myIsParameter; + + public VariableFix(String name, boolean isParameter) { + myName = name; + myIsParameter = isParameter; + } + + @Nls + @NotNull + @Override + public String getName() { + return InspectionsBundle.message("inspection.return.separated.from.computation.quickfix", myName); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.return.separated.from.computation.family.quickfix"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement element = descriptor.getPsiElement(); + if (element instanceof PsiReturnStatement) { + doApply(((PsiReturnStatement)element)); + } + } + } + + private static class ReturnContext { + private final PsiReturnStatement returnStatement; + private final PsiCodeBlock returnScope; + private final PsiStatement refactoredStatement; + private final PsiVariable returnedVariable; + private final PsiCodeBlock variableScope; + + private ReturnContext(@NotNull PsiReturnStatement returnStatement, + @NotNull PsiCodeBlock returnScope, + @NotNull PsiStatement refactoredStatement, + @NotNull PsiVariable returnedVariable, + @NotNull PsiCodeBlock variableScope) { + + this.returnStatement = returnStatement; + this.returnScope = returnScope; + this.refactoredStatement = refactoredStatement; + this.returnedVariable = returnedVariable; + this.variableScope = variableScope; + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor.java new file mode 100644 index 000000000000..098854f69b60 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor.java @@ -0,0 +1,13 @@ +class T { + int f(int[] a) { + int n = -1; + myLabel: + for (int i = 0; i < a.length; i++) { + if (a[0] == 0) { + n = i; + break myLabel; + } + } + return n; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor2.java new file mode 100644 index 000000000000..875bdfb19114 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledFor2.java @@ -0,0 +1,11 @@ +class T { + int f(int[] a) { + int n = -1; + myLabel: + for (int i = 0; i < a.length; i++) { + n = i; + if (a[0] == 0) break myLabel; + } + return n; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledIf.java new file mode 100644 index 000000000000..82067b472c18 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/LabeledIf.java @@ -0,0 +1,9 @@ +class T { + int f(boolean b) { + int n = 0; + myLabel: + if (b) n = 1; + else break myLabel; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlock.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlock.java new file mode 100644 index 000000000000..474f1d55f83e --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlock.java @@ -0,0 +1,9 @@ +class T { + int f() { + int n; + { + n = 1; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlockSideEffect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlockSideEffect.java new file mode 100644 index 000000000000..b085a4dd6651 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedBlockSideEffect.java @@ -0,0 +1,10 @@ +class T { + int f() { + int n; + { + n = 1; + System.out.println(); + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIf.java new file mode 100644 index 000000000000..fb6f7cd1c4b3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIf.java @@ -0,0 +1,11 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) { + n = 1; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfInnerElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfInnerElse.java new file mode 100644 index 000000000000..06bde7faac0a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfInnerElse.java @@ -0,0 +1,10 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) n = 1; + else n = 2; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfOuterElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfOuterElse.java new file mode 100644 index 000000000000..ae1b6a0d2672 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/NestedIfOuterElse.java @@ -0,0 +1,10 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) n = 1; + } + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java new file mode 100644 index 000000000000..71e87f22de37 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java @@ -0,0 +1,15 @@ +import java.io.*; + +class T { + private static String getString() throws IOException { + String s; + try (BufferedReader r = open()) { + s = r.readLine(); + } + return s; + } + + private static BufferedReader open() throws FileNotFoundException { + return null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleDoWhile.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleDoWhile.java new file mode 100644 index 000000000000..7b9a0fec76bd --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleDoWhile.java @@ -0,0 +1,25 @@ +class T { + String f(String a) { + String r = ""; + int i = 0; + do { + int j = a.indexOf(",", i); + String s = j > i ? a.substring(i, j) : a.substring(i); + if (s.startsWith("@")) { + r = s; + break; + } + i = j + 1; + } + while (i >= 0); + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java new file mode 100644 index 000000000000..4fb6dcb47dd2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java @@ -0,0 +1,12 @@ +class T { + int f(int[] a, int b) { + int n = -1; + for (int i = 0; i < a.length; i++) { + if (a[i] == b) { + n = i; + break; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleForeach.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleForeach.java new file mode 100644 index 000000000000..f77d41b7ee34 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleForeach.java @@ -0,0 +1,12 @@ +class T { + String f(String[] a) { + String r = ""; + for (String s : a) { + if (s != null && s.contains("@")) { + r = s + ":" + s.length(); + break; + } + } + return r; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java new file mode 100644 index 000000000000..3e10cc16d52b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java @@ -0,0 +1,7 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java new file mode 100644 index 000000000000..10f70fd719ae --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java @@ -0,0 +1,21 @@ +class T { + String f() { + String r = ""; + while (hasNext()) { + String s = next(); + if (s != null) { + r = s; + break; + } + } + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterBreakFromLoopInTryWithResources.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterBreakFromLoopInTryWithResources.java new file mode 100644 index 000000000000..0281d2efdc95 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterBreakFromLoopInTryWithResources.java @@ -0,0 +1,19 @@ +// "Move 'return' to computation of the value of 's'" "true" +import java.io.*; + +class T { + private static String getString() throws IOException { + String s; + try (BufferedReader reader = open()) { + while (true) { + s = reader.readLine(); + if (s == null || s.startsWith("$")) { + return s; + } + } + } + } + private static BufferedReader open() throws FileNotFoundException { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterDoWhileTrue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterDoWhileTrue.java new file mode 100644 index 000000000000..5dda4432661f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterDoWhileTrue.java @@ -0,0 +1,21 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + String f() { + String r = ""; + do { + if (!hasNext()) return r; + String s = next(); + if (s != null) { + return s; + } + } while (true); + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterForWithoutCondition.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterForWithoutCondition.java new file mode 100644 index 000000000000..c1068e6085bc --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterForWithoutCondition.java @@ -0,0 +1,11 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f() { + int n = -1; + for(int i=0;; i++) { + if (i % 127 == 0 && i % 129 == 0) { + return i + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInBoth.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInBoth.java new file mode 100644 index 000000000000..819d619d4e41 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInBoth.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) return 1; + else return 2; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java new file mode 100644 index 000000000000..6b39f5efb41c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) System.out.println("yes"); + else return 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java new file mode 100644 index 000000000000..5cd0516f9de5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) return 1; + else System.out.println("no"); + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterLabeledBlock.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterLabeledBlock.java new file mode 100644 index 000000000000..c59b5c149e20 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterLabeledBlock.java @@ -0,0 +1,12 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n; + myLabel: + { + n = 1; + if (b) return n; + return 2; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterWhileTrue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterWhileTrue.java new file mode 100644 index 000000000000..63a01fb335cb --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterWhileTrue.java @@ -0,0 +1,14 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + long f() { + long r; + long s = System.currentTimeMillis(); + long t = s; + while (true) { + t = System.currentTimeMillis(); + if (t - s > 100) { + return t; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeAssert.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeAssert.java new file mode 100644 index 000000000000..a51b9bf71b1a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeAssert.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "false" +class T { + int f(int a) { + int n = a; + assert n != 0; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeBreakFromLoopInTryWithResources.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeBreakFromLoopInTryWithResources.java new file mode 100644 index 000000000000..48230a643e28 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeBreakFromLoopInTryWithResources.java @@ -0,0 +1,20 @@ +// "Move 'return' to computation of the value of 's'" "true" +import java.io.*; + +class T { + private static String getString() throws IOException { + String s; + try (BufferedReader reader = open()) { + while (true) { + s = reader.readLine(); + if (s == null || s.startsWith("$")) { + break; + } + } + } + return s; + } + private static BufferedReader open() throws FileNotFoundException { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeDoWhileTrue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeDoWhileTrue.java new file mode 100644 index 000000000000..1cdde2ffdde8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeDoWhileTrue.java @@ -0,0 +1,23 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + String f() { + String r = ""; + do { + if (!hasNext()) break; + String s = next(); + if (s != null) { + r = s; + break; + } + } while (true); + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeForWithoutCondition.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeForWithoutCondition.java new file mode 100644 index 000000000000..49caccf5f1cd --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeForWithoutCondition.java @@ -0,0 +1,13 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f() { + int n = -1; + for(int i=0;; i++) { + if (i % 127 == 0 && i % 129 == 0) { + n = i + 1; + break; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseNoWrite.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseNoWrite.java new file mode 100644 index 000000000000..e8893863450d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseNoWrite.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "false" +class T { + int f(boolean b) { + int n = 0; + if (b) System.out.println("yes"); + else System.out.println("no"); + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInBoth.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInBoth.java new file mode 100644 index 000000000000..9cc257d8d008 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInBoth.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInElse.java new file mode 100644 index 000000000000..8a12ce8558f0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInElse.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) System.out.println("yes"); + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInIf.java new file mode 100644 index 000000000000..0786f31ef2d5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseWriteInIf.java @@ -0,0 +1,9 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + else System.out.println("no"); + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeLabeledBlock.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeLabeledBlock.java new file mode 100644 index 000000000000..db850a43d4f7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeLabeledBlock.java @@ -0,0 +1,13 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n; + myLabel: + { + n = 1; + if (b) break myLabel; + n = 2; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeWhileTrue.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeWhileTrue.java new file mode 100644 index 000000000000..93af902ecd88 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeWhileTrue.java @@ -0,0 +1,16 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + long f() { + long r; + long s = System.currentTimeMillis(); + long t = s; + while (true) { + t = System.currentTimeMillis(); + if (t - s > 100) { + r = t; + break; + } + } + return r; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/Assert.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/Assert.java new file mode 100644 index 000000000000..cfd08d21bc86 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/Assert.java @@ -0,0 +1,7 @@ +class T { + int f(int a) { + int n = a; + assert n != 0; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/BreakFromLoopInTryWithResources.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/BreakFromLoopInTryWithResources.java new file mode 100644 index 000000000000..65656c6f45d7 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/BreakFromLoopInTryWithResources.java @@ -0,0 +1,19 @@ +import java.io.*; + +class T { + private static String getString() throws IOException { + String s; + try (BufferedReader reader = open()) { + while (true) { + s = reader.readLine(); + if (s == null || s.startsWith("$")) { + break; + } + } + } + return s; + } + private static BufferedReader open() throws FileNotFoundException { + return null; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/DoWhileTrue.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/DoWhileTrue.java new file mode 100644 index 000000000000..7c7c9ecac3f8 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/DoWhileTrue.java @@ -0,0 +1,22 @@ +class T { + String f() { + String r = ""; + do { + if (!hasNext()) break; + String s = next(); + if (s != null) { + r = s; + break; + } + } while (true); + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/ForWithoutCondition.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/ForWithoutCondition.java new file mode 100644 index 000000000000..f926a4a1457e --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/ForWithoutCondition.java @@ -0,0 +1,12 @@ +class T { + int f() { + int n = -1; + for(int i=0;; i++) { + if (i % 127 == 0 && i % 129 == 0) { + n = i + 1; + break; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseNoWrite.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseNoWrite.java new file mode 100644 index 000000000000..0214a19911ab --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseNoWrite.java @@ -0,0 +1,8 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) System.out.println("yes"); + else System.out.println("no"); + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInBoth.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInBoth.java new file mode 100644 index 000000000000..d870af1e572f --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInBoth.java @@ -0,0 +1,8 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInElse.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInElse.java new file mode 100644 index 000000000000..826f5c281d3a --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInElse.java @@ -0,0 +1,8 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) System.out.println("yes"); + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInIf.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInIf.java new file mode 100644 index 000000000000..85d5d42a970d --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/IfElseWriteInIf.java @@ -0,0 +1,8 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + else System.out.println("no"); + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledBlock.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledBlock.java new file mode 100644 index 000000000000..722e673ded6d --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledBlock.java @@ -0,0 +1,12 @@ +class T { + int f(boolean b) { + int n; + myLabel: + { + n = 1; + if (b) break myLabel; + n = 2; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor.java new file mode 100644 index 000000000000..098854f69b60 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor.java @@ -0,0 +1,13 @@ +class T { + int f(int[] a) { + int n = -1; + myLabel: + for (int i = 0; i < a.length; i++) { + if (a[0] == 0) { + n = i; + break myLabel; + } + } + return n; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor2.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor2.java new file mode 100644 index 000000000000..875bdfb19114 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledFor2.java @@ -0,0 +1,11 @@ +class T { + int f(int[] a) { + int n = -1; + myLabel: + for (int i = 0; i < a.length; i++) { + n = i; + if (a[0] == 0) break myLabel; + } + return n; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledIf.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledIf.java new file mode 100644 index 000000000000..82067b472c18 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/LabeledIf.java @@ -0,0 +1,9 @@ +class T { + int f(boolean b) { + int n = 0; + myLabel: + if (b) n = 1; + else break myLabel; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlock.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlock.java new file mode 100644 index 000000000000..474f1d55f83e --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlock.java @@ -0,0 +1,9 @@ +class T { + int f() { + int n; + { + n = 1; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlockSideEffect.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlockSideEffect.java new file mode 100644 index 000000000000..b085a4dd6651 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedBlockSideEffect.java @@ -0,0 +1,10 @@ +class T { + int f() { + int n; + { + n = 1; + System.out.println(); + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIf.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIf.java new file mode 100644 index 000000000000..051d28b31b3a --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIf.java @@ -0,0 +1,11 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) { + n = 1; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfInnerElse.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfInnerElse.java new file mode 100644 index 000000000000..8d5ab26e740e --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfInnerElse.java @@ -0,0 +1,10 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) n = 1; + else n = 2; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfOuterElse.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfOuterElse.java new file mode 100644 index 000000000000..fa961bcb49bd --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/NestedIfOuterElse.java @@ -0,0 +1,10 @@ +class T { + int f(boolean a, boolean b) { + int n = -1; + if (a) { + if (b) n = 1; + } + else n = 2; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java new file mode 100644 index 000000000000..71e87f22de37 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/ReturnOutsideTryWithResources.java @@ -0,0 +1,15 @@ +import java.io.*; + +class T { + private static String getString() throws IOException { + String s; + try (BufferedReader r = open()) { + s = r.readLine(); + } + return s; + } + + private static BufferedReader open() throws FileNotFoundException { + return null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SideEffectInIf.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SideEffectInIf.java new file mode 100644 index 000000000000..d99f7d85ddb1 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SideEffectInIf.java @@ -0,0 +1,9 @@ +class T { + int[] f(boolean b) { + int[] r = new int[]{-1}; + if (b) { + r[0] = 1; + } + return r; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleDoWhile.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleDoWhile.java new file mode 100644 index 000000000000..7b9a0fec76bd --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleDoWhile.java @@ -0,0 +1,25 @@ +class T { + String f(String a) { + String r = ""; + int i = 0; + do { + int j = a.indexOf(",", i); + String s = j > i ? a.substring(i, j) : a.substring(i); + if (s.startsWith("@")) { + r = s; + break; + } + i = j + 1; + } + while (i >= 0); + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleFor.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleFor.java new file mode 100644 index 000000000000..4fb6dcb47dd2 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleFor.java @@ -0,0 +1,12 @@ +class T { + int f(int[] a, int b) { + int n = -1; + for (int i = 0; i < a.length; i++) { + if (a[i] == b) { + n = i; + break; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleForeach.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleForeach.java new file mode 100644 index 000000000000..f77d41b7ee34 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleForeach.java @@ -0,0 +1,12 @@ +class T { + String f(String[] a) { + String r = ""; + for (String s : a) { + if (s != null && s.contains("@")) { + r = s + ":" + s.length(); + break; + } + } + return r; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleIf.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleIf.java new file mode 100644 index 000000000000..3e10cc16d52b --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleIf.java @@ -0,0 +1,7 @@ +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleWhile.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleWhile.java new file mode 100644 index 000000000000..10f70fd719ae --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/SimpleWhile.java @@ -0,0 +1,21 @@ +class T { + String f() { + String r = ""; + while (hasNext()) { + String s = next(); + if (s != null) { + r = s; + break; + } + } + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} diff --git a/java/java-tests/testData/inspection/returnSeparatedFromComputation/WhileTrue.java b/java/java-tests/testData/inspection/returnSeparatedFromComputation/WhileTrue.java new file mode 100644 index 000000000000..8f0c4dca5dd2 --- /dev/null +++ b/java/java-tests/testData/inspection/returnSeparatedFromComputation/WhileTrue.java @@ -0,0 +1,15 @@ +class T { + long f() { + long r; + long s = System.currentTimeMillis(); + long t = s; + while (true) { + t = System.currentTimeMillis(); + if (t - s > 100) { + r = t; + break; + } + } + return r; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFix2Test.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFix2Test.java new file mode 100644 index 000000000000..1504232822d8 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFix2Test.java @@ -0,0 +1,48 @@ +/* + * 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.codeInsight.daemon.quickFix; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.intermediaryVariable.ReturnSeparatedFromComputationInspection; +import org.jetbrains.annotations.NotNull; + +/** + * @author Pavel.Dolgov + */ +public class ReturnSeparatedFromComputationFix2Test extends LightQuickFixTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new ReturnSeparatedFromComputationInspection()}; + } + + public void testAssert() { + doTest(); + } + + public void testBreakFromLoopInTryWithResources() { + doTest(); + } + + private void doTest() { + doSingleTest(getTestName(false) +".java"); + } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation"; + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFixTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFixTest.java new file mode 100644 index 000000000000..99a202048282 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ReturnSeparatedFromComputationFixTest.java @@ -0,0 +1,40 @@ +/* + * 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.codeInsight.daemon.quickFix; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.intermediaryVariable.ReturnSeparatedFromComputationInspection; +import org.jetbrains.annotations.NotNull; + +/** + * @author Pavel.Dolgov + */ +public class ReturnSeparatedFromComputationFixTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new ReturnSeparatedFromComputationInspection()}; + } + + public void test() throws Exception { + doAllTests(); + } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation"; + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java new file mode 100644 index 000000000000..0836b132bbdd --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ReturnSeparatedFromComputationTest.java @@ -0,0 +1,136 @@ +/* + * 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.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.intermediaryVariable.ReturnSeparatedFromComputationInspection; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; + +/** + * @author Pavel.Dolgov + */ +public class ReturnSeparatedFromComputationTest extends LightCodeInsightFixtureTestCase { + @Override + protected String getBasePath() { + return JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/returnSeparatedFromComputation"; + } + + public void testReturnOutsideTryWithResources() { + doTest(); + } + + public void testBreakFromLoopInTryWithResources() { + doTest(); + } + + public void testSimpleIf() { + doTest(); + } + + public void testSimpleFor() { + doTest(); + } + + public void testIfElseWriteInBoth() { + doTest(); + } + + public void testIfElseWriteInIf() { + doTest(); + } + + public void testIfElseWriteInElse() { + doTest(); + } + + public void testIfElseNoWrite() { + doTest(); + } + + public void testNestedIf() { + doTest(); + } + + public void testNestedIfInnerElse() { + doTest(); + } + + public void testNestedIfOuterElse() { + doTest(); + } + + public void testNestedBlock() { + doTest(); + } + + public void testNestedBlockSideEffect() { + doTest(); + } + + public void testAssert() { + doTest(); + } + + public void testLabeledBlock() { + doTest(); + } + + public void testLabeledFor() { + doTest(); + } + + public void testLabeledFor2() { + doTest(); + } + + public void testLabeledIf() { + doTest(); + } + + public void testWhileTrue() { + doTest(); + } + + public void testSimpleWhile() { + doTest(); + } + + public void testForWithoutCondition() { + doTest(); + } + + public void testSimpleForeach() { + doTest(); + } + + public void testDoWhileTrue() { + doTest(); + } + + public void testSimpleDoWhile() { + doTest(); + } + + public void testSideEffectInIf() { + doTest(); + } + + + private void doTest() { + myFixture.enableInspections(new ReturnSeparatedFromComputationInspection()); + myFixture.testHighlighting(getTestName(false) + ".java"); + } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index f1f63d9b73a2..ef63f3bad180 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -136,6 +136,11 @@ inspection.local.can.be.final.option3=Report foreach parameters inspection.can.be.local.parameter.problem.descriptor=Parameter #ref can have final modifier inspection.can.be.local.variable.problem.descriptor=Variable #ref can have final modifier +inspection.return.separated.from.computation.name=Return separated from computation of result +inspection.return.separated.from.computation.descriptor=Return separated from computation of value of ''{0}'' +inspection.return.separated.from.computation.quickfix=Move ''return'' to computation of the value of ''{0}'' +inspection.return.separated.from.computation.family.quickfix=Move ''return'' to computation of the result + inspection.nullable.problems.display.name=@NotNull/@Nullable problems #check box options inspection.nullable.problems.method.overrides.notnull.option=Report @NotNull ¶meters overriding @Nullable and
@Nullable methods overriding @NotNull diff --git a/resources-en/src/inspectionDescriptions/ReturnSeparatedFromComputation.html b/resources-en/src/inspectionDescriptions/ReturnSeparatedFromComputation.html new file mode 100644 index 000000000000..83e1a8c0a149 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/ReturnSeparatedFromComputation.html @@ -0,0 +1,14 @@ + + +This inspection detects return statements which return a local variable, where the value of the variable is computed +somewhere else within the same code block with the return statement. +

The quick fix inlines the returned variable by moving the return statement to the location where the value of the variable is computed. + For example, the code below could be simplified: +

int n = -1;
+if (condition) n = compute();
+return n;
+ After the quick fix it becomes the following: +
if (condition) return compute();
+return -1;
+ + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 98fbe10902fb..4d3f78ec8262 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -647,6 +647,10 @@ + Date: Mon, 5 Sep 2016 13:17:12 +0300 Subject: [PATCH 2/2] Java control flow: Improved "Move return to computation" inspection, added more tests (IDEA-121153) --- ...urnSeparatedFromComputationInspection.java | 250 ++++++++++++------ .../SimpleFor.java | 12 - .../SimpleIf.java | 7 - .../SimpleWhile.java | 21 -- .../afterIfElseThrow.java | 12 + .../afterIfElseWriteInElse.java | 2 +- .../afterIfElseWriteInIf.java | 2 +- .../afterIfThrowElse.java | 12 + .../afterIfTryCatch.java | 26 ++ .../afterSimpleFor.java | 12 + .../afterSimpleIf.java | 8 + .../afterSimpleIfDefaultValueComputed.java | 11 + .../afterSimpleIfDefaultValueParameter.java | 8 + .../afterSimpleWhile.java | 21 ++ .../beforeIfElseThrow.java | 13 + .../beforeIfThrow.java | 10 + .../beforeIfThrowElse.java | 13 + .../beforeIfTryCatch.java | 26 ++ .../beforeSimpleFor.java | 13 + .../beforeSimpleIf.java | 8 + .../beforeSimpleIfDefaultValueComputed.java | 11 + .../beforeSimpleIfDefaultValueParameter.java | 8 + .../beforeSimpleWhile.java | 22 ++ 23 files changed, 401 insertions(+), 127 deletions(-) delete mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java delete mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java delete mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseThrow.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfThrowElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfTryCatch.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleFor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueComputed.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueParameter.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleWhile.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseThrow.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrow.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrowElse.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfTryCatch.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleFor.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueComputed.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueParameter.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleWhile.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 d2ef0f79b83a..8b82ea1d4bfd 100644 --- a/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/intermediaryVariable/ReturnSeparatedFromComputationInspection.java @@ -134,7 +134,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal } Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable); - mover.moveTo(context.refactoredStatement); + mover.moveTo(context.refactoredStatement, true); return !mover.isEmpty(); } @@ -144,7 +144,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal ControlFlow flow = createControlFlow(context); if (flow != null) { Mover mover = new Mover(flow, context.refactoredStatement, context.returnedVariable); - boolean removeReturn = mover.moveTo(context.refactoredStatement); + boolean removeReturn = mover.moveTo(context.refactoredStatement, true); if (!mover.isEmpty()) { applyChanges(mover, context, removeReturn); } @@ -153,20 +153,89 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal } private static void applyChanges(@NotNull Mover mover, @NotNull ReturnContext context, boolean removeReturn) { - mover.insertAfter.forEach(e -> e.getParent().addAfter(context.returnStatement, e)); - mover.insertBefore.forEach(e -> e.getParent().addBefore(context.returnStatement, e)); + PsiReturnStatement returnStatement = (PsiReturnStatement)context.returnStatement.copy(); + if (removeReturn) { + removeReturn(context); + } + else { + //inlineReturnedValue(mover, context); + } + mover.insertAfter.forEach(e -> e.getParent().addAfter(returnStatement, e)); + mover.insertBefore.forEach(e -> e.getParent().addBefore(returnStatement, e)); mover.replaceInline.forEach(e -> { - if (e instanceof PsiBreakStatement) e.replace(context.returnStatement); - if (e instanceof PsiAssignmentExpression) inlineAssignment((PsiAssignmentExpression)e, context.returnStatement); + if (e instanceof PsiBreakStatement) e.replace(returnStatement); + if (e instanceof PsiAssignmentExpression) inlineAssignment((PsiAssignmentExpression)e, returnStatement); }); mover.removeCompletely.forEach(PsiElement::delete); + } - if (removeReturn) { - Set skippedEmptyStatements = new THashSet<>(); - getPrevNonEmptyStatement(context.returnStatement, skippedEmptyStatements); - skippedEmptyStatements.forEach(PsiElement::delete); - context.returnStatement.delete(); + private static void removeReturn(@NotNull ReturnContext context) { + Set skippedEmptyStatements = new THashSet<>(); + getPrevNonEmptyStatement(context.returnStatement, skippedEmptyStatements); + skippedEmptyStatements.forEach(PsiElement::delete); + context.returnStatement.delete(); + } + + private static void inlineReturnedValue(Mover mover, ReturnContext context) { + List instructions = mover.flow.getInstructions(); + PsiAssignmentExpression assignment = null; + for (int i = 0; i < instructions.size(); i++) { + Instruction instruction = instructions.get(i); + if (instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == mover.resultVariable) { + PsiElement element = mover.flow.getElement(i); + PsiElement parent = element.getParent(); + if (element instanceof PsiAssignmentExpression && parent instanceof PsiExpressionStatement) { + if (!mover.replaceInline.contains(element)) { + if (assignment != null) { + return; + } + assignment = (PsiAssignmentExpression)element; + } + } + else if (!(getNearestEnclosingStatement(element) instanceof PsiDeclarationStatement)) { + return; + } + } } + + PsiExpression localInitializer = context.returnedVariable instanceof PsiLocalVariable ? + context.returnedVariable.getInitializer() : null; + if (assignment != null && localInitializer == null) { + PsiExpression rExpression = assignment.getRExpression(); + replaceReturnedValue(rExpression, context.returnStatement, mover.flow); + } + else if (assignment == null && localInitializer != null) { + replaceReturnedValue(localInitializer, context.returnStatement, mover.flow); + } + } + + private static void replaceReturnedValue(PsiExpression newReturnValue, PsiReturnStatement returnStatement, ControlFlow flow) { + PsiExpression returnValue = returnStatement.getReturnValue(); + if (returnValue != null && + (ExpressionUtils.computeConstantExpression(newReturnValue) != null || isUnchangedReferenceToLocal(newReturnValue, flow))) { + returnValue.replace(newReturnValue); + } + } + + private static boolean isUnchangedReferenceToLocal(PsiExpression expression, ControlFlow flow) { + if (expression instanceof PsiReferenceExpression) { + PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; + if (!referenceExpression.isQualified()) { + PsiElement resolved = referenceExpression.resolve(); + if (resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter) { + if (((PsiVariable)resolved).hasModifierProperty(PsiModifier.FINAL)) { + return true; + } + for (Instruction instruction : flow.getInstructions()) { + if (instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == resolved) { + return false; + } + } + return true; + } + } + } + return false; } private static void inlineAssignment(PsiAssignmentExpression assignmentExpression, PsiReturnStatement returnStatement) { @@ -206,9 +275,9 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal * Returns true if the targetStatement will always exit via return/throw/etc after the transformation, * so if the next statement is a return or a break it can be removed safely. */ - boolean moveTo(PsiStatement targetStatement) { + boolean moveTo(PsiStatement targetStatement, boolean returnAtTheEnd) { if (targetStatement instanceof PsiBlockStatement) { - return moveToBlock((PsiBlockStatement)targetStatement); + return moveToBlock((PsiBlockStatement)targetStatement, returnAtTheEnd); } if (targetStatement instanceof PsiIfStatement) { return moveToIf((PsiIfStatement)targetStatement); @@ -226,111 +295,118 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return moveToForeach((PsiForeachStatement)targetStatement); } if (targetStatement instanceof PsiTryStatement) { - return moveToTry(((PsiTryStatement)targetStatement)); + return moveToTry((PsiTryStatement)targetStatement); } if (targetStatement instanceof PsiLabeledStatement) { - return moveToLabeled(((PsiLabeledStatement)targetStatement)); + return moveToLabeled((PsiLabeledStatement)targetStatement, returnAtTheEnd); } if (targetStatement instanceof PsiExpressionStatement) { - return inlineExpression(((PsiExpressionStatement)targetStatement)); + return inlineExpression((PsiExpressionStatement)targetStatement); } - return false; - } - - private boolean moveToBlock(PsiBlockStatement targetStatement) { - return moveToBlock(targetStatement.getCodeBlock()); - } - - private boolean moveToBlock(@NotNull PsiCodeBlock codeBlock) { - PsiJavaToken rBrace = codeBlock.getRBrace(); - if (rBrace != null) { - PsiStatement lastNonEmptyStatement = getPrevNonEmptyStatement(rBrace, removeCompletely); - if (lastNonEmptyStatement == null || !moveTo(lastNonEmptyStatement)) { - insertBefore.add(rBrace); - } + if (targetStatement instanceof PsiThrowStatement) { return true; } return false; } - private boolean moveToIf(PsiIfStatement targetStatement) { - PsiStatement thenBranch = targetStatement.getThenBranch(); - PsiStatement elseBranch = targetStatement.getElseBranch(); - - boolean thenPart = thenBranch != null && moveTo(thenBranch); - boolean elsePart = elseBranch != null && moveTo(elseBranch); - return thenPart && elsePart; + private boolean moveToBlock(@NotNull PsiBlockStatement targetStatement, boolean returnAtTheEnd) { + return moveToBlockBody(targetStatement.getCodeBlock(), returnAtTheEnd); } - private boolean moveToFor(PsiForStatement targetStatement) { - moveToBreaks(targetStatement); - return isAlwaysTrue(targetStatement.getCondition(), true); - } - - private boolean moveToDoWhile(PsiDoWhileStatement targetStatement) { - moveToBreaks(targetStatement); - return isAlwaysTrue(targetStatement.getCondition(), false); - } - - private boolean moveToWhile(PsiWhileStatement targetStatement) { - moveToBreaks(targetStatement); - return isAlwaysTrue(targetStatement.getCondition(), false); - } - private boolean moveToForeach(PsiForeachStatement targetStatement) { - moveToBreaks(targetStatement); + private boolean moveToBlockBody(@NotNull PsiCodeBlock codeBlock, boolean returnAtTheEnd) { + PsiJavaToken rBrace = codeBlock.getRBrace(); + if (rBrace != null) { + PsiStatement lastNonEmptyStatement = getPrevNonEmptyStatement(rBrace, removeCompletely); + if (lastNonEmptyStatement == null || lastNonEmptyStatement instanceof PsiReturnStatement) { + return false; + } + if (moveTo(lastNonEmptyStatement, returnAtTheEnd)) { + return true; + } + if (returnAtTheEnd) { + insertBefore.add(rBrace); + return true; + } + } return false; } - private boolean moveToTry(PsiTryStatement targetStatement) { + private boolean moveToIf(@NotNull PsiIfStatement targetStatement) { + PsiStatement thenBranch = targetStatement.getThenBranch(); + PsiStatement elseBranch = targetStatement.getElseBranch(); + + boolean thenPart = thenBranch != null && moveTo(thenBranch, false); + boolean elsePart = elseBranch != null && moveTo(elseBranch, false); + return thenPart && elsePart; + } + + private boolean moveToFor(@NotNull PsiForStatement targetStatement) { + moveToBreaks(targetStatement, false); + return isAlwaysTrue(targetStatement.getCondition(), true); + } + + private boolean moveToDoWhile(@NotNull PsiDoWhileStatement targetStatement) { + moveToBreaks(targetStatement, false); + return isAlwaysTrue(targetStatement.getCondition(), false); + } + + private boolean moveToWhile(@NotNull PsiWhileStatement targetStatement) { + moveToBreaks(targetStatement, false); + return isAlwaysTrue(targetStatement.getCondition(), false); + } + + private boolean moveToForeach(@NotNull PsiForeachStatement targetStatement) { + moveToBreaks(targetStatement, false); + return false; + } + + private boolean moveToTry(@NotNull PsiTryStatement targetStatement) { PsiCodeBlock tryBlock = targetStatement.getTryBlock(); if (tryBlock == null) { return false; } + boolean result = true; PsiCodeBlock finallyBlock = targetStatement.getFinallyBlock(); - if (finallyBlock != null && ControlFlowUtils.codeBlockMayCompleteNormally(finallyBlock) && writesVariable(finallyBlock)) { - return false; + if (finallyBlock != null && writesVariable(finallyBlock)) { + result = false; } PsiCatchSection[] catchSections = targetStatement.getCatchSections(); for (PsiCatchSection catchSection : catchSections) { PsiCodeBlock catchBlock = catchSection.getCatchBlock(); - if (catchBlock != null && ControlFlowUtils.codeBlockMayCompleteNormally(catchBlock) && writesVariable(finallyBlock)) { - return false; + if (catchBlock == null || !moveToBlockBody(catchBlock, false)) { + result = false; } } - return moveToBlock(tryBlock); + return moveToBlockBody(tryBlock, false) && result; } - private boolean moveToLabeled(PsiLabeledStatement targetStatement) { + private boolean moveToLabeled(@NotNull PsiLabeledStatement targetStatement, boolean returnAtTheEnd) { PsiStatement statement = targetStatement.getStatement(); if (statement == null) { return false; } - moveToBreaks(statement); - return moveTo(statement); + moveToBreaks(statement, false); + return moveTo(statement, returnAtTheEnd); } - private boolean inlineExpression(PsiExpressionStatement statement) { + private boolean inlineExpression(@NotNull PsiExpressionStatement statement) { PsiExpression expression = statement.getExpression(); if (expression instanceof PsiAssignmentExpression) { PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)expression; PsiExpression lExpression = assignmentExpression.getLExpression(); - if (lExpression instanceof PsiReferenceExpression) { - PsiReferenceExpression referenceExpression = (PsiReferenceExpression)lExpression; - if (!referenceExpression.isQualified() && referenceExpression.resolve() == resultVariable) { - if (assignmentExpression.getOperationTokenType() == JavaTokenType.EQ) { - replaceInline.add(assignmentExpression); - return true; - } - } + if (assignmentExpression.getOperationTokenType() == JavaTokenType.EQ && isReferenceTo(lExpression, resultVariable)) { + replaceInline.add(assignmentExpression); + return true; } } return false; } - private void moveToBreaks(Set breaks) { + private void moveToBreaks(@NotNull PsiStatement targetStatement, boolean returnAtTheEnd) { + Set breaks = getBreaks(targetStatement); for (PsiBreakStatement breakStatement : breaks) { PsiStatement prevNonEmptyStatement = getPrevNonEmptyStatement(breakStatement, removeCompletely); - if (prevNonEmptyStatement == null || !moveTo(prevNonEmptyStatement)) { + if (prevNonEmptyStatement == null || !moveTo(prevNonEmptyStatement, returnAtTheEnd)) { replaceInline.add(breakStatement); } else { @@ -339,12 +415,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal } } - private void moveToBreaks(PsiStatement targetStatement) { - Set breaks = getBreaks(targetStatement); - moveToBreaks(breaks); - } - - private boolean writesVariable(PsiElement element) { + private boolean writesVariable(@NotNull PsiElement element) { int startOffset = flow.getStartOffset(element); int endOffset = flow.getEndOffset(element); if (startOffset < 0 || endOffset < 0) { @@ -365,7 +436,7 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return ExpressionUtils.computeConstantExpression(condition) == Boolean.TRUE; } - private Set getBreaks(PsiStatement targetStatement) { + private Set getBreaks(@NotNull PsiStatement targetStatement) { if (breakStatements == null) { breakStatements = new THashMap<>(); List instructions = flow.getInstructions(); @@ -384,14 +455,18 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return breaks != null ? breaks : Collections.emptySet(); } - @Nullable - private static PsiStatement getNearestEnclosingStatement(PsiElement element) { - return element instanceof PsiStatement ? (PsiStatement)element : PsiTreeUtil.getParentOfType(element, PsiStatement.class); + private static boolean isReferenceTo(PsiExpression expression, PsiVariable variable) { + if (expression instanceof PsiReferenceExpression) { + PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; + if (!referenceExpression.isQualified() && referenceExpression.resolve() == variable) { + return true; + } + } + return false; } - } - private static PsiStatement getPrevNonEmptyStatement(PsiElement psiElement, Set skippedEmptyStatements) { + private static PsiStatement getPrevNonEmptyStatement(@NotNull PsiElement psiElement, @NotNull Set skippedEmptyStatements) { PsiStatement prevStatement = PsiTreeUtil.getPrevSiblingOfType(psiElement, PsiStatement.class); List skipped = new ArrayList<>(); while (prevStatement instanceof PsiEmptyStatement) { @@ -404,6 +479,11 @@ public class ReturnSeparatedFromComputationInspection extends BaseJavaBatchLocal return prevStatement; } + @Nullable + private static PsiStatement getNearestEnclosingStatement(@NotNull PsiElement element) { + return element instanceof PsiStatement ? (PsiStatement)element : PsiTreeUtil.getParentOfType(element, PsiStatement.class); + } + private static void registerProblem(@NotNull ProblemsHolder holder, @NotNull PsiReturnStatement returnStatement, @NotNull PsiVariable variable) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java deleted file mode 100644 index 4fb6dcb47dd2..000000000000 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleFor.java +++ /dev/null @@ -1,12 +0,0 @@ -class T { - int f(int[] a, int b) { - int n = -1; - for (int i = 0; i < a.length; i++) { - if (a[i] == b) { - n = i; - break; - } - } - return n; - } -} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java deleted file mode 100644 index 3e10cc16d52b..000000000000 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleIf.java +++ /dev/null @@ -1,7 +0,0 @@ -class T { - int f(boolean b) { - int n = 0; - if (b) n = 1; - return n; - } -} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java deleted file mode 100644 index 10f70fd719ae..000000000000 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/SimpleWhile.java +++ /dev/null @@ -1,21 +0,0 @@ -class T { - String f() { - String r = ""; - while (hasNext()) { - String s = next(); - if (s != null) { - r = s; - break; - } - } - return r; - } - - boolean hasNext() { - return true; - } - - String next() { - return null; - } -} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseThrow.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseThrow.java new file mode 100644 index 000000000000..fcb5925a9fa9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseThrow.java @@ -0,0 +1,12 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = -1; + if (b) { + return 1; + } + else { + throw new RuntimeException(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java index 6b39f5efb41c..792398002a0a 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInElse.java @@ -4,6 +4,6 @@ class T { int n = 0; if (b) System.out.println("yes"); else return 2; - return n; + return 0; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java index 5cd0516f9de5..2d4265440cca 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfElseWriteInIf.java @@ -4,6 +4,6 @@ class T { int n = 0; if (b) return 1; else System.out.println("no"); - return n; + return 0; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfThrowElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfThrowElse.java new file mode 100644 index 000000000000..2aa4d9932d84 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfThrowElse.java @@ -0,0 +1,12 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = -1; + if (b) { + throw new RuntimeException(); + } + else { + return 2; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfTryCatch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfTryCatch.java new file mode 100644 index 000000000000..8232b4078605 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterIfTryCatch.java @@ -0,0 +1,26 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b, boolean c) { + int n = -1; + if (b) { + try { + return g(); + } + catch (RuntimeException e) { + d(e); + } + } + else { + return 2; + } + return n; + } + + int g() { + return 1; + } + + void d(Exception e) { + e.printStackTrace() + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleFor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleFor.java new file mode 100644 index 000000000000..3e7a56d9ccfe --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleFor.java @@ -0,0 +1,12 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(int[] a, int b) { + int n = -1; + for (int i = 0; i < a.length; i++) { + if (a[i] == b) { + return i; + } + } + return -1; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIf.java new file mode 100644 index 000000000000..a2ef6c55bea5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIf.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) return 1; + return 0; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueComputed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueComputed.java new file mode 100644 index 000000000000..b26cdd5c41b7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueComputed.java @@ -0,0 +1,11 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = g(); + if (b) return 1; + return n; + } + int g() { + return 0; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueParameter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueParameter.java new file mode 100644 index 000000000000..ce3e9b346c43 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleIfDefaultValueParameter.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b, int d) { + int n = d; + if (b) return 1; + return d; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleWhile.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleWhile.java new file mode 100644 index 000000000000..57707b7bdb8b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/afterSimpleWhile.java @@ -0,0 +1,21 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + String f() { + String r = ""; + while (hasNext()) { + String s = next(); + if (s != null) { + return s; + } + } + return ""; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseThrow.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseThrow.java new file mode 100644 index 000000000000..e4a6eb5634f1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfElseThrow.java @@ -0,0 +1,13 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = -1; + if (b) { + n = 1; + } + else { + throw new RuntimeException(); + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrow.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrow.java new file mode 100644 index 000000000000..63f476304fb7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrow.java @@ -0,0 +1,10 @@ +// "Move 'return' to computation of the value of 'n'" "false" +class T { + int f(boolean b) { + int n = -1; + if (b) { + throw new RuntimeException(); + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrowElse.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrowElse.java new file mode 100644 index 000000000000..1d991b685844 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfThrowElse.java @@ -0,0 +1,13 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = -1; + if (b) { + throw new RuntimeException(); + } + else { + n = 2; + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfTryCatch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfTryCatch.java new file mode 100644 index 000000000000..f45e7dbe0f00 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeIfTryCatch.java @@ -0,0 +1,26 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b, boolean c) { + int n = -1; + if (b) { + try { + n = g(); + } + catch (RuntimeException e) { + d(e); + } + } + else { + n = 2; + } + return n; + } + + int g() { + return 1; + } + + void d(Exception e) { + e.printStackTrace() + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleFor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleFor.java new file mode 100644 index 000000000000..a2283fdb1652 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleFor.java @@ -0,0 +1,13 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(int[] a, int b) { + int n = -1; + for (int i = 0; i < a.length; i++) { + if (a[i] == b) { + n = i; + break; + } + } + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIf.java new file mode 100644 index 000000000000..4a98ee9c6302 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIf.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = 0; + if (b) n = 1; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueComputed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueComputed.java new file mode 100644 index 000000000000..7dc95fac9da5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueComputed.java @@ -0,0 +1,11 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b) { + int n = g(); + if (b) n = 1; + return n; + } + int g() { + return 0; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueParameter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueParameter.java new file mode 100644 index 000000000000..c1e967c6339d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleIfDefaultValueParameter.java @@ -0,0 +1,8 @@ +// "Move 'return' to computation of the value of 'n'" "true" +class T { + int f(boolean b, int d) { + int n = d; + if (b) n = 1; + return n; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleWhile.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleWhile.java new file mode 100644 index 000000000000..45f28d655eb4 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/returnSeparatedFromComputation/beforeSimpleWhile.java @@ -0,0 +1,22 @@ +// "Move 'return' to computation of the value of 'r'" "true" +class T { + String f() { + String r = ""; + while (hasNext()) { + String s = next(); + if (s != null) { + r = s; + break; + } + } + return r; + } + + boolean hasNext() { + return true; + } + + String next() { + return null; + } +} \ No newline at end of file