IDEA-53497 ('Merge nested IFs' refactoring ignores change in the condition)

This commit is contained in:
Bas Leijdekkers
2010-04-06 08:08:02 +02:00
parent cb58bb2580
commit 6ae43d6012
4 changed files with 285 additions and 23 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2009 Bas Leijdekkers
* Copyright 2009-2010 Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -20,6 +20,11 @@ import com.intellij.psi.tree.IElementType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
public class VariableAccessUtils {
private VariableAccessUtils() {
@@ -137,4 +142,49 @@ public class VariableAccessUtils {
final PsiElement referent = referenceExpression.resolve();
return variable.equals(referent);
}
public static boolean isAnyVariableAssigned(
@NotNull Collection<PsiVariable> variables,
@Nullable PsiElement context) {
if (context == null) {
return false;
}
final VariableAssignedVisitor visitor =
new VariableAssignedVisitor(variables, true);
context.accept(visitor);
return visitor.isAssigned();
}
public static Set<PsiVariable> collectUsedVariables(
PsiElement context) {
if (context == null) {
return Collections.EMPTY_SET;
}
final VariableCollectingVisitor visitor =
new VariableCollectingVisitor();
context.accept(visitor);
return visitor.getUsedVariables();
}
private static class VariableCollectingVisitor
extends JavaRecursiveElementVisitor {
private final Set<PsiVariable> usedVariables = new HashSet();
@Override
public void visitReferenceExpression(
PsiReferenceExpression expression) {
super.visitReferenceExpression(expression);
final PsiElement target = expression.resolve();
if (!(target instanceof PsiVariable)) {
return;
}
final PsiVariable variable = (PsiVariable)target;
usedVariables.add(variable);
}
public Set<PsiVariable> getUsedVariables() {
return usedVariables;
}
}
}
@@ -0,0 +1,196 @@
/*
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
*
* 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.siyeh.ipp.psiutils;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
class VariableAssignedVisitor extends JavaRecursiveElementVisitor{
@NotNull private final Collection<PsiVariable> variables;
private final boolean recurseIntoClasses;
private boolean assigned = false;
public VariableAssignedVisitor(@NotNull Collection<PsiVariable> variables,
boolean recurseIntoClasses){
this.variables = variables;
this.recurseIntoClasses = recurseIntoClasses;
}
@Override public void visitElement(@NotNull PsiElement element){
if(assigned){
return;
}
super.visitElement(element);
}
@Override public void visitAssignmentExpression(
@NotNull PsiAssignmentExpression assignment){
if(assigned){
return;
}
super.visitAssignmentExpression(assignment);
final PsiExpression lhs = assignment.getLExpression();
for (PsiVariable variable : variables) {
if(mayEvaluateToVariable(lhs, variable)){
assigned = true;
}
}
}
@Override
public void visitClass(PsiClass aClass) {
if(!recurseIntoClasses){
return;
}
if(assigned){
return;
}
super.visitClass(aClass);
}
@Override public void visitPrefixExpression(
@NotNull PsiPrefixExpression prefixExpression){
if(assigned){
return;
}
super.visitPrefixExpression(prefixExpression);
final PsiJavaToken operationSign = prefixExpression.getOperationSign();
final IElementType tokenType = operationSign.getTokenType();
if(!tokenType.equals(JavaTokenType.PLUSPLUS) &&
!tokenType.equals(JavaTokenType.MINUSMINUS)){
return;
}
final PsiExpression operand = prefixExpression.getOperand();
for (PsiVariable variable : variables) {
if(mayEvaluateToVariable(operand, variable)){
assigned = true;
}
}
}
@Override public void visitPostfixExpression(
@NotNull PsiPostfixExpression postfixExpression){
if(assigned){
return;
}
super.visitPostfixExpression(postfixExpression);
final PsiJavaToken operationSign = postfixExpression.getOperationSign();
final IElementType tokenType = operationSign.getTokenType();
if(!tokenType.equals(JavaTokenType.PLUSPLUS) &&
!tokenType.equals(JavaTokenType.MINUSMINUS)){
return;
}
final PsiExpression operand = postfixExpression.getOperand();
for (PsiVariable variable : variables) {
if(mayEvaluateToVariable(operand, variable)){
assigned = true;
}
}
}
public static boolean mayEvaluateToVariable(
@Nullable PsiExpression expression,
@NotNull PsiVariable variable) {
if (expression == null){
return false;
}
if(expression instanceof PsiBinaryExpression) {
final PsiBinaryExpression binaryExpression =
(PsiBinaryExpression)expression;
final PsiExpression lOperand = binaryExpression.getLOperand();
final PsiExpression rOperand = binaryExpression.getROperand();
return mayEvaluateToVariable(lOperand, variable) ||
mayEvaluateToVariable(rOperand, variable);
}
if(expression instanceof PsiParenthesizedExpression){
final PsiParenthesizedExpression parenthesizedExpression =
(PsiParenthesizedExpression)expression;
final PsiExpression containedExpression =
parenthesizedExpression.getExpression();
return mayEvaluateToVariable(containedExpression, variable);
}
if(expression instanceof PsiTypeCastExpression){
final PsiTypeCastExpression typeCastExpression =
(PsiTypeCastExpression)expression;
final PsiExpression containedExpression =
typeCastExpression.getOperand();
return mayEvaluateToVariable(containedExpression, variable);
}
if(expression instanceof PsiConditionalExpression){
final PsiConditionalExpression conditional =
(PsiConditionalExpression) expression;
final PsiExpression thenExpression = conditional.getThenExpression();
final PsiExpression elseExpression = conditional.getElseExpression();
return mayEvaluateToVariable(thenExpression, variable) ||
mayEvaluateToVariable(elseExpression, variable);
}
if(expression instanceof PsiArrayAccessExpression){
final PsiElement parent = expression.getParent();
if (parent instanceof PsiArrayAccessExpression){
return false;
}
final PsiType type = variable.getType();
if (!(type instanceof PsiArrayType)) {
return false;
}
final PsiArrayType arrayType = (PsiArrayType)type;
final int dimensions = arrayType.getArrayDimensions();
if (dimensions <= 1) {
return false;
}
PsiArrayAccessExpression arrayAccessExpression =
(PsiArrayAccessExpression)expression;
PsiExpression arrayExpression =
arrayAccessExpression.getArrayExpression();
int count = 1;
while (arrayExpression instanceof PsiArrayAccessExpression) {
arrayAccessExpression =
(PsiArrayAccessExpression)arrayExpression;
arrayExpression = arrayAccessExpression.getArrayExpression();
count++;
}
return count != dimensions &&
mayEvaluateToVariable(arrayExpression, variable);
}
return evaluatesToVariable(expression, variable);
}
public static boolean evaluatesToVariable(
@Nullable PsiExpression expression,
@NotNull PsiVariable variable) {
final PsiExpression strippedExpression =
ParenthesesUtils.stripParentheses(expression);
if(strippedExpression == null){
return false;
}
if (!(expression instanceof PsiReferenceExpression)) {
return false;
}
final PsiReferenceExpression referenceExpression =
(PsiReferenceExpression) expression;
final PsiElement referent = referenceExpression.resolve();
return variable.equals(referent);
}
public boolean isAssigned(){
return assigned;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2006 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -25,11 +25,13 @@ import org.jetbrains.annotations.NonNls;
public class MergeParallelIfsIntention extends Intention {
@Override
@NotNull
public PsiElementPredicate getElementPredicate() {
return new MergeParallelIfsPredicate();
}
@Override
public void processIntention(PsiElement element)
throws IncorrectOperationException {
final PsiJavaToken token = (PsiJavaToken)element;
@@ -57,23 +59,23 @@ public class MergeParallelIfsIntention extends Intention {
final PsiStatement firstThenBranch = firstStatement.getThenBranch();
final PsiStatement secondThenBranch = secondStatement.getThenBranch();
@NonNls String statement = "if(" + conditionText + ')' +
printStatementsInSequence(firstThenBranch,
secondThenBranch);
printStatementsInSequence(firstThenBranch,
secondThenBranch);
final PsiStatement firstElseBranch = firstStatement.getElseBranch();
final PsiStatement secondElseBranch = secondStatement.getElseBranch();
if (firstElseBranch != null || secondElseBranch != null) {
if (firstElseBranch instanceof PsiIfStatement
&& secondElseBranch instanceof PsiIfStatement
&& MergeParallelIfsPredicate.ifStatementsCanBeMerged(
&& secondElseBranch instanceof PsiIfStatement
&& MergeParallelIfsPredicate.ifStatementsCanBeMerged(
(PsiIfStatement)firstElseBranch,
(PsiIfStatement)secondElseBranch)) {
statement += "else " +
mergeIfStatements((PsiIfStatement)firstElseBranch,
(PsiIfStatement)secondElseBranch);
mergeIfStatements((PsiIfStatement)firstElseBranch,
(PsiIfStatement)secondElseBranch);
} else {
statement += "else" +
printStatementsInSequence(firstElseBranch,
secondElseBranch);
printStatementsInSequence(firstElseBranch,
secondElseBranch);
}
}
return statement;
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2006 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -21,7 +21,9 @@ import com.siyeh.ipp.base.PsiElementPredicate;
import com.siyeh.ipp.psiutils.ControlFlowUtils;
import com.siyeh.ipp.psiutils.EquivalenceChecker;
import com.siyeh.ipp.psiutils.ErrorUtil;
import com.siyeh.ipp.psiutils.VariableAccessUtils;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
@@ -32,7 +34,6 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
return false;
}
final PsiJavaToken token = (PsiJavaToken) element;
final PsiElement parent = token.getParent();
if(!(parent instanceof PsiIfStatement)){
return false;
@@ -51,7 +52,19 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
if(ErrorUtil.containsError(nextIfStatement)){
return false;
}
return ifStatementsCanBeMerged(ifStatement, nextIfStatement);
if(!ifStatementsCanBeMerged(ifStatement, nextIfStatement)){
return false;
}
final PsiExpression condition = ifStatement.getCondition();
final Set<PsiVariable> variables =
VariableAccessUtils.collectUsedVariables(condition);
final PsiStatement thenBranch = ifStatement.getThenBranch();
if(VariableAccessUtils.isAnyVariableAssigned(variables, thenBranch)){
return false;
}
final PsiStatement elseBranch = ifStatement.getElseBranch();
return !VariableAccessUtils.isAnyVariableAssigned(variables,
elseBranch);
}
public static boolean ifStatementsCanBeMerged(PsiIfStatement statement1,
@@ -63,8 +76,8 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
}
final PsiExpression firstCondition = statement1.getCondition();
final PsiExpression secondCondition = statement2.getCondition();
if(! EquivalenceChecker.expressionsAreEquivalent(firstCondition,
secondCondition)){
if(!EquivalenceChecker.expressionsAreEquivalent(firstCondition,
secondCondition)){
return false;
}
final PsiStatement nextThenBranch = statement2.getThenBranch();
@@ -73,11 +86,11 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
}
final PsiStatement nextElseBranch = statement2.getElseBranch();
return elseBranch == null || nextElseBranch == null ||
canBeMerged(elseBranch, nextElseBranch);
canBeMerged(elseBranch, nextElseBranch);
}
private static boolean canBeMerged(PsiStatement statement1,
PsiStatement statement2){
PsiStatement statement2){
if(!ControlFlowUtils.statementMayCompleteNormally(statement1)){
return false;
}
@@ -89,13 +102,13 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
final Set<String> statement2Declarations =
calculateTopLevelDeclarations(statement2);
return !containsConflictingDeclarations(statement2Declarations,
statement1);
statement1);
}
private static boolean containsConflictingDeclarations(
Set<String> declarations, PsiStatement statement) {
Set<String> declarations, PsiElement context) {
final DeclarationVisitor visitor = new DeclarationVisitor(declarations);
statement.accept(visitor);
context.accept(visitor);
return visitor.hasConflict();
}
@@ -119,18 +132,19 @@ class MergeParallelIfsPredicate implements PsiElementPredicate{
}
private static void addDeclarations(PsiDeclarationStatement statement,
Set<String> declaredVars){
Collection<String> declaredVariables){
final PsiElement[] elements = statement.getDeclaredElements();
for(final PsiElement element : elements){
if(element instanceof PsiVariable){
final PsiVariable variable = (PsiVariable) element;
final String name = variable.getName();
declaredVars.add(name);
declaredVariables.add(name);
}
}
}
private static class DeclarationVisitor extends JavaRecursiveElementWalkingVisitor{
private static class DeclarationVisitor
extends JavaRecursiveElementWalkingVisitor{
private final Set<String> declarations;
private boolean hasConflict = false;