fix some problem converting if to switch in combination with existing break statements and cleanup

This commit is contained in:
Bas Leijdekkers
2011-02-18 17:06:52 +01:00
parent bd3519f71f
commit 6c8eb8b086
6 changed files with 161 additions and 180 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2009 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2011 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.
@@ -78,14 +78,15 @@ public class BoolUtils{
return ParenthesesUtils.stripParentheses(operand);
}
public static boolean isBooleanLiteral(PsiExpression exp){
if(exp instanceof PsiLiteralExpression){
final PsiLiteralExpression expression = (PsiLiteralExpression) exp;
@NonNls final String text = expression.getText();
return PsiKeyword.TRUE.equals(text) ||
PsiKeyword.FALSE.equals(text);
public static boolean isBooleanLiteral(PsiExpression expression){
if (!(expression instanceof PsiLiteralExpression)) {
return false;
}
return false;
final PsiLiteralExpression literalExpression =
(PsiLiteralExpression) expression;
@NonNls final String text = literalExpression.getText();
return PsiKeyword.TRUE.equals(text) ||
PsiKeyword.FALSE.equals(text);
}
public static String getNegatedExpressionText(
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2005 Dave Griffith
* Copyright 2003-2011 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.
@@ -15,15 +15,12 @@
*/
package com.siyeh.ipp.psiutils;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.ConstantExpressionUtil;
import com.intellij.psi.util.PsiUtil;
public class ControlFlowUtils{
private ControlFlowUtils(){
super();
}
private ControlFlowUtils(){}
public static boolean statementMayCompleteNormally(PsiStatement statement){
if(statement instanceof PsiBreakStatement ||
@@ -40,8 +37,10 @@ public class ControlFlowUtils{
} else if(statement instanceof PsiForStatement){
final PsiForStatement loopStatement = (PsiForStatement) statement;
final PsiExpression test = loopStatement.getCondition();
return test != null && !isBooleanConstant(test, false) ||
return test != null && !isBooleanConstant(test, true) ||
statementIsBreakTarget(loopStatement);
} else if (statement instanceof PsiForeachStatement) {
return true;
} else if(statement instanceof PsiWhileStatement){
final PsiWhileStatement loopStatement =
(PsiWhileStatement) statement;
@@ -144,14 +143,23 @@ public class ControlFlowUtils{
return true;
}
private static boolean isBooleanConstant(PsiExpression test, boolean value){
if(!PsiUtil.isConstantExpression(test)){
private static boolean isBooleanConstant(PsiExpression expression,
boolean b){
if (expression == null) {
return false;
}
final Boolean constantValue =
(Boolean) ConstantExpressionUtil.computeCastTo(test,
PsiType.BOOLEAN);
return constantValue != null && constantValue.booleanValue() == value;
final Project project = expression.getProject();
final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
final PsiConstantEvaluationHelper constantEvaluationHelper =
psiFacade.getConstantEvaluationHelper();
final Object value =
constantEvaluationHelper.computeConstantExpression
(expression, false);
if (!(value instanceof Boolean)) {
return false;
}
final Boolean aBoolean = (Boolean) value;
return aBoolean.booleanValue() == b;
}
private static boolean statementIsBreakTarget(PsiStatement statement){
@@ -178,7 +186,6 @@ public class ControlFlowUtils{
private final PsiStatement m_target;
private BreakTargetFinder(PsiStatement target){
super();
m_target = target;
}
@@ -186,6 +193,14 @@ public class ControlFlowUtils{
return m_found;
}
@Override
public void visitElement(PsiElement element) {
if (m_found) {
return;
}
super.visitElement(element);
}
@Override public void visitReferenceExpression(
PsiReferenceExpression expression){
}
@@ -212,6 +227,14 @@ public class ControlFlowUtils{
return m_found;
}
@Override
public void visitElement(PsiElement element) {
if (m_found) {
return;
}
super.visitElement(element);
}
@Override public void visitReferenceExpression(PsiReferenceExpression expression){
}
@@ -230,6 +253,11 @@ public class ControlFlowUtils{
// don't drill down
}
@Override
public void visitForeachStatement(PsiForeachStatement statement) {
// don't drill down
}
@Override public void visitWhileStatement(PsiWhileStatement statement){
// don't drill down
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2011 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.
@@ -18,6 +18,7 @@ package com.siyeh.ipp.switchtoif;
import com.intellij.pom.java.LanguageLevel;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ipp.psiutils.EquivalenceChecker;
import com.siyeh.ipp.psiutils.SideEffectChecker;
@@ -32,63 +33,25 @@ class CaseUtil{
super();
}
private static boolean canBeCaseLabel(PsiExpression expression){
private static boolean canBeCaseLabel(PsiExpression expression,
LanguageLevel languageLevel){
if(expression == null){
return false;
}
if(expression instanceof PsiReferenceExpression){
if (languageLevel.compareTo(LanguageLevel.JDK_1_5) >= 0
&& expression instanceof PsiReferenceExpression){
final PsiElement referent = ((PsiReference) expression).resolve();
if(referent instanceof PsiEnumConstant){
return true;
}
}
final PsiType type = expression.getType();
if(type == null){
return false;
}
if(!type.equals(PsiType.INT) &&
!type.equals(PsiType.CHAR) &&
!type.equals(PsiType.LONG) &&
!type.equals(PsiType.SHORT)){
return false;
}
return PsiUtil.isConstantExpression(expression);
}
public static boolean containsHiddenBreak(PsiStatement statement){
return containsHiddenBreak(statement, true);
}
private static boolean containsHiddenBreak(PsiStatement statement,
boolean isTopLevel){
if(statement instanceof PsiBlockStatement){
final PsiCodeBlock codeBlock =
((PsiBlockStatement) statement).getCodeBlock();
final PsiStatement[] statements = codeBlock.getStatements();
for(final PsiStatement childStatement : statements){
if(containsHiddenBreak(childStatement, false)){
return true;
}
}
} else if(statement instanceof PsiIfStatement){
final PsiIfStatement ifStatement = (PsiIfStatement) statement;
final PsiStatement thenBranch = ifStatement.getThenBranch();
final PsiStatement elseBranch = ifStatement.getElseBranch();
return containsHiddenBreak(thenBranch, false) ||
containsHiddenBreak(elseBranch, false);
} else if(statement instanceof PsiBreakStatement){
if(isTopLevel){
return false;
}
final PsiIdentifier identifier =
((PsiBreakStatement) statement).getLabelIdentifier();
if(identifier == null){
return true;
}
final String text = identifier.getText();
return "".equals(text);
}
return false;
return type != null &&
(type.equals(PsiType.INT) ||
type.equals(PsiType.CHAR) ||
type.equals(PsiType.LONG) ||
type.equals(PsiType.SHORT)) &&
PsiUtil.isConstantExpression(expression);
}
public static boolean isUsedByStatementList(PsiLocalVariable variable,
@@ -109,17 +72,10 @@ class CaseUtil{
return visitor.isUsed();
}
public static String findUniqueLabel(PsiStatement statement,
@NonNls String baseName){
PsiElement ancestor = statement;
while(ancestor.getParent() != null){
if(ancestor instanceof PsiMethod
|| ancestor instanceof PsiClass
|| ancestor instanceof PsiFile){
break;
}
ancestor = ancestor.getParent();
}
public static String findUniqueLabelName(PsiStatement statement,
@NonNls String baseName){
final PsiElement ancestor =
PsiTreeUtil.getParentOfType(statement, PsiMember.class);
if(!checkForLabel(baseName, ancestor)){
return baseName;
}
@@ -140,37 +96,35 @@ class CaseUtil{
}
@Nullable
public static PsiExpression getCaseExpression(PsiIfStatement statement){
public static PsiExpression getSwitchExpression(PsiIfStatement statement){
final PsiExpression condition = statement.getCondition();
final LanguageLevel languageLevel =
PsiUtil.getLanguageLevel(statement);
final boolean stringSwitch =
languageLevel.compareTo(LanguageLevel.JDK_1_7) >= 0;
final PsiExpression possibleCaseExpression =
determinePossibleCaseExpressions(condition, stringSwitch);
if(possibleCaseExpression == null){
final PsiExpression possibleSwitchExpression =
determinePossibleSwitchExpressions(condition, languageLevel);
if(possibleSwitchExpression == null){
return null;
}
if (SideEffectChecker.mayHaveSideEffects(possibleCaseExpression)) {
if (SideEffectChecker.mayHaveSideEffects(possibleSwitchExpression)) {
return null;
}
while(true){
final PsiExpression caseCondition = statement.getCondition();
if (!canBeMadeIntoCase(caseCondition, possibleCaseExpression,
stringSwitch)) {
if (!canBeMadeIntoCase(caseCondition, possibleSwitchExpression,
languageLevel)) {
break;
}
final PsiStatement elseBranch = statement.getElseBranch();
if(!(elseBranch instanceof PsiIfStatement)){
return possibleCaseExpression;
return possibleSwitchExpression;
}
statement = (PsiIfStatement) elseBranch;
}
return null;
}
private static PsiExpression determinePossibleCaseExpressions(
PsiExpression expression, boolean stringSwitch){
private static PsiExpression determinePossibleSwitchExpressions(
PsiExpression expression, LanguageLevel languageLevel){
while(expression instanceof PsiParenthesizedExpression){
final PsiParenthesizedExpression parenthesizedExpression =
(PsiParenthesizedExpression)expression;
@@ -179,9 +133,9 @@ class CaseUtil{
if (expression == null) {
return null;
}
if (stringSwitch) {
if (languageLevel.compareTo(LanguageLevel.JDK_1_7) >= 0) {
final PsiExpression jdk17Expression =
determinePossibleStringCaseExpression(expression);
determinePossibleStringSwitchExpression(expression);
if (jdk17Expression != null) {
return jdk17Expression;
}
@@ -196,18 +150,18 @@ class CaseUtil{
final PsiExpression lhs = binaryExpression.getLOperand();
final PsiExpression rhs = binaryExpression.getROperand();
if(operation.equals(JavaTokenType.OROR)){
return determinePossibleCaseExpressions(lhs, stringSwitch);
return determinePossibleSwitchExpressions(lhs, languageLevel);
} else if(operation.equals(JavaTokenType.EQEQ)){
if(canBeCaseLabel(lhs)){
if(canBeCaseLabel(lhs, languageLevel)){
return rhs;
} else if (canBeCaseLabel(rhs)){
} else if (canBeCaseLabel(rhs, languageLevel)){
return lhs;
}
}
return null;
}
private static PsiExpression determinePossibleStringCaseExpression(
private static PsiExpression determinePossibleStringSwitchExpression(
PsiExpression expression) {
if (!(expression instanceof PsiMethodCallExpression)) {
return null;
@@ -216,7 +170,8 @@ class CaseUtil{
(PsiMethodCallExpression) expression;
final PsiReferenceExpression methodExpression =
methodCallExpression.getMethodExpression();
final String referenceName = methodExpression.getReferenceName();
@NonNls final String referenceName =
methodExpression.getReferenceName();
if (!"equals".equals(referenceName)) {
return null;
}
@@ -251,15 +206,15 @@ class CaseUtil{
private static boolean canBeMadeIntoCase(
PsiExpression expression, PsiExpression caseExpression,
boolean stringSwitch) {
LanguageLevel languageLevel) {
while(expression instanceof PsiParenthesizedExpression){
final PsiParenthesizedExpression parenthesizedExpression =
(PsiParenthesizedExpression)expression;
expression = parenthesizedExpression.getExpression();
}
if (stringSwitch) {
if (languageLevel.compareTo(LanguageLevel.JDK_1_7) >=0 ) {
final PsiExpression stringCaseExpression =
determinePossibleStringCaseExpression(expression);
determinePossibleStringSwitchExpression(expression);
if (EquivalenceChecker.expressionsAreEquivalent(caseExpression,
stringCaseExpression)) {
return true;
@@ -275,20 +230,17 @@ class CaseUtil{
final PsiExpression lOperand = binaryExpression.getLOperand();
final PsiExpression rhs = binaryExpression.getROperand();
if(operation.equals(JavaTokenType.OROR)){
return canBeMadeIntoCase(lOperand, caseExpression, stringSwitch) &&
canBeMadeIntoCase(rhs, caseExpression, stringSwitch);
return canBeMadeIntoCase(lOperand, caseExpression, languageLevel) &&
canBeMadeIntoCase(rhs, caseExpression, languageLevel);
} else if(operation.equals(JavaTokenType.EQEQ)){
if(canBeCaseLabel(lOperand) &&
EquivalenceChecker.expressionsAreEquivalent(caseExpression,
rhs)){
return true;
} else if(canBeCaseLabel(rhs) &&
EquivalenceChecker.expressionsAreEquivalent(caseExpression,
lOperand)){
return true;
}
return false;
} else{
return (canBeCaseLabel(lOperand, languageLevel) &&
EquivalenceChecker.expressionsAreEquivalent(
caseExpression, rhs))
||
(canBeCaseLabel(rhs, languageLevel) &&
EquivalenceChecker.expressionsAreEquivalent(
caseExpression, lOperand));
} else {
return false;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2011 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.
@@ -40,6 +40,6 @@ class IfToSwitchPredicate implements PsiElementPredicate{
if(ErrorUtil.containsError(statement)){
return false;
}
return CaseUtil.getCaseExpression(statement) != null;
return CaseUtil.getSwitchExpression(statement) != null;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2011 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.
@@ -17,6 +17,7 @@ package com.siyeh.ipp.switchtoif;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.IncorrectOperationException;
import com.siyeh.ipp.base.Intention;
import com.siyeh.ipp.base.PsiElementPredicate;
@@ -52,26 +53,27 @@ public class ReplaceIfWithSwitchIntention extends Intention {
PsiStatement breakTarget = null;
String labelString = "";
if (ControlFlowUtils.statementContainsExitingBreak(ifStatement)) {
// what a pain.
PsiElement ancestor = ifStatement.getParent();
while (ancestor != null) {
if (ancestor instanceof PsiForStatement ||
ancestor instanceof PsiDoWhileStatement ||
ancestor instanceof PsiWhileStatement ||
ancestor instanceof PsiSwitchStatement) {
breakTarget = (PsiStatement)ancestor;
break;
}
ancestor = ancestor.getParent();
}
breakTarget = PsiTreeUtil.getParentOfType(ifStatement,
PsiLoopStatement.class, PsiSwitchStatement.class);
if (breakTarget != null) {
labelString = CaseUtil.findUniqueLabel(ifStatement, "Label");
breaksNeedRelabeled = true;
final PsiElement parent = breakTarget.getParent();
if (parent instanceof PsiLabeledStatement) {
final PsiLabeledStatement labeledStatement =
(PsiLabeledStatement) parent;
labelString =
labeledStatement.getLabelIdentifier().getText();
breakTarget = labeledStatement;
breaksNeedRelabeled = true;
} else {
labelString = CaseUtil.findUniqueLabelName(ifStatement,
"label");
breaksNeedRelabeled = true;
}
}
}
final PsiIfStatement statementToReplace = ifStatement;
final PsiExpression caseExpression =
CaseUtil.getCaseExpression(ifStatement);
CaseUtil.getSwitchExpression(ifStatement);
assert caseExpression != null;
final List<IfStatementBranch> branches =
@@ -181,8 +183,10 @@ public class ReplaceIfWithSwitchIntention extends Intention {
final PsiElementFactory factory = psiFacade.getElementFactory();
if (breaksNeedRelabeled) {
final StringBuilder out = new StringBuilder();
out.append(labelString);
out.append(':');
if (!(breakTarget instanceof PsiLabeledStatement)) {
out.append(labelString);
out.append(':');
}
termReplace(out, breakTarget, statementToReplace,
switchStatementText);
final String newStatementText = out.toString();
@@ -271,8 +275,7 @@ public class ReplaceIfWithSwitchIntention extends Intention {
if (target.equals(replace)) {
out.append(stringToReplaceWith);
} else if (target.getChildren().length == 0) {
final String text = target.getText();
out.append(text);
out.append(target.getText());
} else {
final PsiElement[] children = target.getChildren();
for (final PsiElement child : children) {
@@ -322,35 +325,36 @@ public class ReplaceIfWithSwitchIntention extends Intention {
}
}
} else if (expression instanceof PsiParenthesizedExpression) {
final PsiParenthesizedExpression parenExpression =
final PsiParenthesizedExpression parenthesizedExpression =
(PsiParenthesizedExpression)expression;
final PsiExpression contents = parenExpression.getExpression();
final PsiExpression contents =
parenthesizedExpression.getExpression();
getValuesFromExpression(contents, caseExpression, values);
}
return values;
}
private static void dumpBranch(StringBuilder switchStatementString,
private static void dumpBranch(StringBuilder switchStatementText,
List<String> comments,
List<String> labels,
List<String> statementComments,
PsiStatement body,
boolean wrap, boolean renameBreaks,
String breakLabelName) {
dumpComments(switchStatementString, comments);
dumpLabels(switchStatementString, labels);
dumpComments(switchStatementString, statementComments);
dumpBody(switchStatementString, body, wrap, renameBreaks,
dumpComments(switchStatementText, comments);
dumpLabels(switchStatementText, labels);
dumpComments(switchStatementText, statementComments);
dumpBody(switchStatementText, body, wrap, renameBreaks,
breakLabelName);
}
private static void dumpComments(StringBuilder switchStatementString,
private static void dumpComments(StringBuilder switchStatementText,
List<String> comments) {
if (!comments.isEmpty()) {
switchStatementString.append('\n');
switchStatementText.append('\n');
for (String comment : comments) {
switchStatementString.append(comment);
switchStatementString.append('\n');
switchStatementText.append(comment);
switchStatementText.append('\n');
}
}
}
@@ -367,21 +371,21 @@ public class ReplaceIfWithSwitchIntention extends Intention {
breakLabelName);
}
private static void dumpLabels(@NonNls StringBuilder switchStatementString,
private static void dumpLabels(@NonNls StringBuilder switchStatementText,
List<String> labels) {
for (String label : labels) {
switchStatementString.append("case ");
switchStatementString.append(label);
switchStatementString.append(": ");
switchStatementText.append("case ");
switchStatementText.append(label);
switchStatementText.append(": ");
}
}
private static void dumpBody(@NonNls StringBuilder switchStatementString,
private static void dumpBody(@NonNls StringBuilder switchStatementText,
PsiStatement bodyStatement, boolean wrap,
boolean renameBreaks, String breakLabelName) {
if (bodyStatement instanceof PsiBlockStatement) {
if (wrap) {
appendElement(switchStatementString, bodyStatement,
appendElement(switchStatementText, bodyStatement,
renameBreaks, breakLabelName);
} else {
final PsiCodeBlock codeBlock =
@@ -390,60 +394,55 @@ public class ReplaceIfWithSwitchIntention extends Intention {
//skip the first and last members, to unwrap the block
for (int i = 1; i < children.length - 1; i++) {
final PsiElement child = children[i];
appendElement(switchStatementString, child, renameBreaks,
appendElement(switchStatementText, child, renameBreaks,
breakLabelName);
}
}
} else {
if (wrap) {
switchStatementString.append('{');
appendElement(switchStatementString, bodyStatement,
switchStatementText.append('{');
appendElement(switchStatementText, bodyStatement,
renameBreaks, breakLabelName);
switchStatementString.append('}');
switchStatementText.append('}');
} else {
appendElement(switchStatementString, bodyStatement,
appendElement(switchStatementText, bodyStatement,
renameBreaks, breakLabelName);
}
}
if (ControlFlowUtils.statementMayCompleteNormally(bodyStatement)) {
switchStatementString.append("break; ");
switchStatementText.append("break; ");
}
}
private static void appendElement(
@NonNls StringBuilder switchStatementString,
@NonNls StringBuilder switchStatementText,
PsiElement element, boolean renameBreakElements,
String breakLabelString) {
final String text = element.getText();
if (!renameBreakElements) {
switchStatementString.append(text);
switchStatementText.append(text);
} else if (element instanceof PsiBreakStatement) {
final PsiBreakStatement breakStatement =
(PsiBreakStatement) element;
final PsiIdentifier identifier =
((PsiBreakStatement)element).getLabelIdentifier();
breakStatement.getLabelIdentifier();
if (identifier == null) {
switchStatementString.append("break ");
switchStatementString.append(breakLabelString);
switchStatementString.append(';');
switchStatementText.append("break ");
switchStatementText.append(breakLabelString);
switchStatementText.append(';');
} else {
final String identifierText = identifier.getText();
if ("".equals(identifierText)) {
switchStatementString.append("break ");
switchStatementString.append(breakLabelString);
switchStatementString.append(';');
} else {
switchStatementString.append(text);
}
switchStatementText.append(text);
}
} else if (element instanceof PsiBlockStatement ||
element instanceof PsiCodeBlock ||
element instanceof PsiIfStatement) {
final PsiElement[] children = element.getChildren();
for (final PsiElement child : children) {
appendElement(switchStatementString, child, renameBreakElements,
appendElement(switchStatementText, child, renameBreakElements,
breakLabelString);
}
} else {
switchStatementString.append(text);
switchStatementText.append(text);
}
}
}
@@ -92,7 +92,7 @@ public class ReplaceSwitchWithIfIntention extends Intention {
final PsiStatement[] statements = body.getStatements();
boolean renameBreaks = false;
for (int i = 1; i < statements.length - 1; i++) {
if (CaseUtil.containsHiddenBreak(statements[i])) {
if (ControlFlowUtils.statementContainsExitingBreak(statements[i])) {
renameBreaks = true;
break;
}
@@ -163,7 +163,8 @@ public class ReplaceSwitchWithIfIntention extends Intention {
final StringBuilder ifStatementText = new StringBuilder();
String breakLabel = null;
if (renameBreaks) {
breakLabel = CaseUtil.findUniqueLabel(switchStatement, "Label");
breakLabel =
CaseUtil.findUniqueLabelName(switchStatement, "label");
ifStatementText.append(breakLabel);
ifStatementText.append(':');
}