From 9530f21f499fffdc3ea9e7a296bd6407ade490c7 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 6 Mar 2013 10:51:36 +0100 Subject: [PATCH] IDEA-102431 (Replace 'if' with 'switch' bug if there are comments and return statement) part 2 --- .../ig/migration/IfCanBeSwitchInspection.java | 31 +- .../siyeh/ig/migration/IfStatementBranch.java | 22 +- .../ipp/switchtoif/IfStatementBranch.java | 24 +- .../ReplaceIfWithSwitchIntention.java | 283 +++++++----------- .../replace_if_with_switch/Comments.java | 14 + .../Comments_after.java | 16 + .../ReplaceIfWithSwitchlIntentionTest.java | 36 +++ 7 files changed, 215 insertions(+), 211 deletions(-) create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments_after.java create mode 100644 plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchlIntentionTest.java diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java index ff8414ab225f..ce8808ef9cb5 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java @@ -178,11 +178,11 @@ public class IfCanBeSwitchInspection extends BaseInspection { } } final PsiIfStatement statementToReplace = ifStatement; - final List branches = new ArrayList(20); final PsiExpression switchExpression = SwitchUtils.getSwitchExpression(ifStatement, myMinimumBranches); if (switchExpression == null) { return; } + final List branches = new ArrayList(20); while (true) { final PsiExpression condition = ifStatement.getCondition(); final PsiStatement thenBranch = ifStatement.getThenBranch(); @@ -234,7 +234,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { if (!(breakTarget instanceof PsiLabeledStatement)) { out.append(labelString).append(':'); } - termReplace(out, breakTarget, statementToReplace, switchStatementText); + termReplace(breakTarget, statementToReplace, switchStatementText, out); final String newStatementText = out.toString(); final PsiStatement newStatement = factory.createStatementFromText(newStatementText, element); breakTarget.replace(newStatement); @@ -247,7 +247,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { @Nullable public static T getPrevSiblingOfType(@Nullable PsiElement element, @NotNull Class aClass, - @NotNull Class... stopAt) { + @NotNull Class... stopAt) { if (element == null) { return null; } @@ -302,12 +302,12 @@ public class IfCanBeSwitchInspection extends BaseInspection { else { commentText = comment.getText(); } - out.addStatementComment(commentText);out.addStatementComment(commentText); + out.addStatementComment(commentText); comment = getPrevSiblingOfType(comment, PsiComment.class, PsiStatement.class, PsiKeyword.class); } } - private static void termReplace(StringBuilder out, PsiElement target, PsiElement replace, StringBuilder stringToReplaceWith) { + private static void termReplace(PsiElement target, PsiElement replace, StringBuilder stringToReplaceWith, StringBuilder out) { if (target.equals(replace)) { out.append(stringToReplaceWith); } @@ -317,7 +317,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { else { final PsiElement[] children = target.getChildren(); for (final PsiElement child : children) { - termReplace(out, child, replace, stringToReplaceWith); + termReplace(child, replace, stringToReplaceWith, out); } } } @@ -347,8 +347,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { extractCaseExpressions(rhs, switchExpression, values); } else { - if (EquivalenceChecker.expressionsAreEquivalent( - switchExpression, rhs)) { + if (EquivalenceChecker.expressionsAreEquivalent(switchExpression, rhs)) { values.addCaseExpression(lhs); } else { @@ -364,13 +363,13 @@ public class IfCanBeSwitchInspection extends BaseInspection { } private static void dumpBranch(IfStatementBranch branch, boolean castToInt, boolean wrap, boolean renameBreaks, String breakLabelName, - @NonNls StringBuilder switchStatementText) { + @NonNls StringBuilder switchStatementText) { dumpComments(branch.getComments(), switchStatementText); if (branch.isElse()) { switchStatementText.append("default: "); } else { - for (PsiExpression caseExpression : branch.getConditions()) { + for (PsiExpression caseExpression : branch.getCaseExpressions()) { switchStatementText.append("case ").append(getCaseLabelText(caseExpression, castToInt)).append(": "); } } @@ -416,7 +415,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { } private static void dumpBody(PsiStatement bodyStatement, boolean wrap, boolean renameBreaks, String breakLabelName, - @NonNls StringBuilder switchStatementText) { + @NonNls StringBuilder switchStatementText) { if (wrap) { switchStatementText.append('{'); } @@ -426,11 +425,11 @@ public class IfCanBeSwitchInspection extends BaseInspection { //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(switchStatementText, child, renameBreaks, breakLabelName); + appendElement(child, renameBreaks, breakLabelName, switchStatementText); } } else { - appendElement(switchStatementText, bodyStatement, renameBreaks, breakLabelName); + appendElement(bodyStatement, renameBreaks, breakLabelName, switchStatementText); } if (ControlFlowUtils.statementMayCompleteNormally(bodyStatement)) { switchStatementText.append("break;"); @@ -440,8 +439,8 @@ public class IfCanBeSwitchInspection extends BaseInspection { } } - private static void appendElement(@NonNls StringBuilder switchStatementText, PsiElement element, boolean renameBreakElements, - String breakLabelString) { + private static void appendElement(PsiElement element, boolean renameBreakElements, String breakLabelString, + @NonNls StringBuilder switchStatementText) { final String text = element.getText(); if (!renameBreakElements) { switchStatementText.append(text); @@ -459,7 +458,7 @@ public class IfCanBeSwitchInspection extends BaseInspection { else if (element instanceof PsiBlockStatement || element instanceof PsiCodeBlock || element instanceof PsiIfStatement) { final PsiElement[] children = element.getChildren(); for (final PsiElement child : children) { - appendElement(switchStatementText, child, renameBreakElements, breakLabelString); + appendElement(child, renameBreakElements, breakLabelString, switchStatementText); } } else { diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfStatementBranch.java b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfStatementBranch.java index 6d5513ac4d3f..2efb2a90f817 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfStatementBranch.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/migration/IfStatementBranch.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 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. @@ -24,7 +24,7 @@ class IfStatementBranch { private final Set topLevelVariables = new HashSet(3); private final LinkedList comments = new LinkedList(); private final LinkedList statementComments = new LinkedList(); - private final List conditions = new ArrayList(3); + private final List caseExpressions = new ArrayList(3); private final PsiStatement statement; private final boolean elseBranch; @@ -43,29 +43,27 @@ class IfStatementBranch { } public void addCaseExpression(PsiExpression expression) { - conditions.add(expression); + caseExpressions.add(expression); } public PsiStatement getStatement() { return statement; } - public List getConditions() { - return Collections.unmodifiableList(conditions); + public List getCaseExpressions() { + return Collections.unmodifiableList(caseExpressions); } public boolean isElse() { return elseBranch; } - public boolean topLevelDeclarationsConflictWith( - IfStatementBranch testBranch) { + public boolean topLevelDeclarationsConflictWith(IfStatementBranch testBranch) { final Set topLevel = testBranch.topLevelVariables; return intersects(topLevelVariables, topLevel); } - private static boolean intersects(Set set1, - Set set2) { + private static boolean intersects(Set set1, Set set2) { for (final String s : set1) { if (set2.contains(s)) { return true; @@ -87,10 +85,8 @@ class IfStatementBranch { return; } if (statement instanceof PsiDeclarationStatement) { - final PsiDeclarationStatement declarationStatement = - (PsiDeclarationStatement)statement; - final PsiElement[] elements = - declarationStatement.getDeclaredElements(); + final PsiDeclarationStatement declarationStatement = (PsiDeclarationStatement)statement; + final PsiElement[] elements = declarationStatement.getDeclaredElements(); for (PsiElement element : elements) { final PsiVariable variable = (PsiVariable)element; final String varName = variable.getName(); diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/IfStatementBranch.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/IfStatementBranch.java index 218ddf50fb5e..598809d5e25a 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/IfStatementBranch.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/IfStatementBranch.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 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. @@ -24,7 +24,7 @@ class IfStatementBranch { private final Set topLevelVariables = new HashSet(3); private final LinkedList comments = new LinkedList(); private final LinkedList statementComments = new LinkedList(); - private final List conditions = new ArrayList(3); + private final List caseExpressions = new ArrayList(3); private final PsiStatement statement; private final boolean elseBranch; @@ -42,30 +42,28 @@ class IfStatementBranch { statementComments.addFirst(comment); } - public void addCondition(String conditionString) { - conditions.add(conditionString); + public void addCaseExpression(PsiExpression expression) { + caseExpressions.add(expression); } public PsiStatement getStatement() { return statement; } - public List getConditions() { - return Collections.unmodifiableList(conditions); + public List getCaseExpressions() { + return Collections.unmodifiableList(caseExpressions); } public boolean isElse() { return elseBranch; } - public boolean topLevelDeclarationsConflictWith( - IfStatementBranch testBranch) { + public boolean topLevelDeclarationsConflictWith(IfStatementBranch testBranch) { final Set topLevel = testBranch.topLevelVariables; return intersects(topLevelVariables, topLevel); } - private static boolean intersects(Set set1, - Set set2) { + private static boolean intersects(Set set1, Set set2) { for (final String s : set1) { if (set2.contains(s)) { return true; @@ -87,10 +85,8 @@ class IfStatementBranch { return; } if (statement instanceof PsiDeclarationStatement) { - final PsiDeclarationStatement declarationStatement = - (PsiDeclarationStatement)statement; - final PsiElement[] elements = - declarationStatement.getDeclaredElements(); + final PsiDeclarationStatement declarationStatement = (PsiDeclarationStatement)statement; + final PsiElement[] elements = declarationStatement.getDeclaredElements(); for (PsiElement element : elements) { final PsiVariable variable = (PsiVariable)element; final String varName = variable.getName(); diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchIntention.java index 4d7e34bc3834..b322d5300b76 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchIntention.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 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,7 +18,6 @@ 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; import com.siyeh.ipp.psiutils.ControlFlowUtils; @@ -39,8 +38,7 @@ public class ReplaceIfWithSwitchIntention extends Intention { } @Override - public void processIntention(@NotNull PsiElement element) - throws IncorrectOperationException { + public void processIntention(@NotNull PsiElement element) { final PsiJavaToken switchToken = (PsiJavaToken)element; PsiIfStatement ifStatement = (PsiIfStatement)switchToken.getParent(); if (ifStatement == null) { @@ -50,67 +48,37 @@ public class ReplaceIfWithSwitchIntention extends Intention { PsiStatement breakTarget = null; String labelString = ""; if (ControlFlowUtils.statementContainsNakedBreak(ifStatement)) { - breakTarget = PsiTreeUtil.getParentOfType(ifStatement, - PsiLoopStatement.class, PsiSwitchStatement.class); + breakTarget = PsiTreeUtil.getParentOfType(ifStatement, PsiLoopStatement.class, PsiSwitchStatement.class); if (breakTarget != null) { final PsiElement parent = breakTarget.getParent(); if (parent instanceof PsiLabeledStatement) { - final PsiLabeledStatement labeledStatement = - (PsiLabeledStatement)parent; - labelString = - labeledStatement.getLabelIdentifier().getText(); + final PsiLabeledStatement labeledStatement = (PsiLabeledStatement)parent; + labelString = labeledStatement.getLabelIdentifier().getText(); breakTarget = labeledStatement; breaksNeedRelabeled = true; } else { - labelString = SwitchUtils.findUniqueLabelName(ifStatement, - "label"); + labelString = SwitchUtils.findUniqueLabelName(ifStatement, "label"); breaksNeedRelabeled = true; } } } final PsiIfStatement statementToReplace = ifStatement; - final PsiExpression switchExpression = - SwitchUtils.getSwitchExpression(ifStatement); - assert switchExpression != null; - - final List branches = - new ArrayList(20); + final PsiExpression switchExpression = SwitchUtils.getSwitchExpression(ifStatement); + if (switchExpression == null) { + return; + } + final List branches = new ArrayList(20); while (true) { final PsiExpression condition = ifStatement.getCondition(); - final List labels = - getValuesFromExpression(condition, switchExpression, - new ArrayList()); final PsiStatement thenBranch = ifStatement.getThenBranch(); - final IfStatementBranch ifBranch = - new IfStatementBranch(thenBranch, false); + final IfStatementBranch ifBranch = new IfStatementBranch(thenBranch, false); + extractCaseExpressions(condition, switchExpression, ifBranch); if (!branches.isEmpty()) { extractIfComments(ifStatement, ifBranch); } extractStatementComments(thenBranch, ifBranch); - for (final PsiExpression label : labels) { - if (label instanceof PsiReferenceExpression) { - final PsiReferenceExpression reference = - (PsiReferenceExpression)label; - final PsiElement referent = reference.resolve(); - if (referent instanceof PsiEnumConstant) { - final PsiEnumConstant constant = - (PsiEnumConstant)referent; - final String constantName = constant.getName(); - ifBranch.addCondition(constantName); - } - else { - final String labelText = label.getText(); - ifBranch.addCondition(labelText); - } - } - else { - final String labelText = label.getText(); - ifBranch.addCondition(labelText); - } - } branches.add(ifBranch); - final PsiStatement elseBranch = ifStatement.getElseBranch(); if (elseBranch instanceof PsiIfStatement) { ifStatement = (PsiIfStatement)elseBranch; @@ -119,8 +87,7 @@ public class ReplaceIfWithSwitchIntention extends Intention { break; } else { - final IfStatementBranch elseIfBranch = - new IfStatementBranch(elseBranch, true); + final IfStatementBranch elseIfBranch = new IfStatementBranch(elseBranch, true); final PsiKeyword elseKeyword = ifStatement.getElseElement(); extractIfComments(elseKeyword, elseIfBranch); extractStatementComments(elseBranch, elseIfBranch); @@ -129,11 +96,10 @@ public class ReplaceIfWithSwitchIntention extends Intention { } } - @NonNls final StringBuilder switchStatementText = - new StringBuilder(); - switchStatementText.append("switch("); - switchStatementText.append(switchExpression.getText()); - switchStatementText.append("){"); + @NonNls final StringBuilder switchStatementText = new StringBuilder(); + switchStatementText.append("switch(").append(switchExpression.getText()).append("){"); + final PsiType type = switchExpression.getType(); + final boolean castToInt = type != null && type.equalsToText(CommonClassNames.JAVA_LANG_INTEGER); for (IfStatementBranch branch : branches) { boolean hasConflicts = false; for (IfStatementBranch testBranch : branches) { @@ -144,39 +110,30 @@ public class ReplaceIfWithSwitchIntention extends Intention { hasConflicts = true; } } - dumpBranch(branch, hasConflicts, breaksNeedRelabeled, labelString, - switchStatementText); + dumpBranch(branch, castToInt, hasConflicts, breaksNeedRelabeled, labelString, switchStatementText); } switchStatementText.append('}'); - final JavaPsiFacade psiFacade = - JavaPsiFacade.getInstance(element.getProject()); + final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(element.getProject()); final PsiElementFactory factory = psiFacade.getElementFactory(); if (breaksNeedRelabeled) { final StringBuilder out = new StringBuilder(); if (!(breakTarget instanceof PsiLabeledStatement)) { - out.append(labelString); - out.append(':'); + out.append(labelString).append(':'); } - termReplace(breakTarget, statementToReplace, switchStatementText, - out); + termReplace(breakTarget, statementToReplace, switchStatementText, out); final String newStatementText = out.toString(); - final PsiStatement newStatement = - factory.createStatementFromText(newStatementText, element); + final PsiStatement newStatement = factory.createStatementFromText(newStatementText, element); breakTarget.replace(newStatement); } else { - final PsiStatement newStatement = - factory.createStatementFromText( - switchStatementText.toString(), element); + final PsiStatement newStatement = factory.createStatementFromText(switchStatementText.toString(), element); statementToReplace.replace(newStatement); } } @Nullable - public static T getPrevSiblingOfType( - @Nullable PsiElement element, - @NotNull Class aClass, - @NotNull Class... stopAt) { + public static T getPrevSiblingOfType(@Nullable PsiElement element, @NotNull Class aClass, + @NotNull Class... stopAt) { if (element == null) { return null; } @@ -192,18 +149,15 @@ public class ReplaceIfWithSwitchIntention extends Intention { return (T)sibling; } - private static void extractIfComments(PsiElement element, - IfStatementBranch out) { - PsiComment comment = getPrevSiblingOfType(element, - PsiComment.class, PsiStatement.class); + private static void extractIfComments(PsiElement element, IfStatementBranch out) { + PsiComment comment = getPrevSiblingOfType(element, PsiComment.class, PsiStatement.class); while (comment != null) { final PsiElement sibling = comment.getPrevSibling(); final String commentText; if (sibling instanceof PsiWhiteSpace) { final String whiteSpaceText = sibling.getText(); if (whiteSpaceText.startsWith("\n")) { - commentText = whiteSpaceText.substring(1) + - comment.getText(); + commentText = whiteSpaceText.substring(1) + comment.getText(); } else { commentText = comment.getText(); @@ -213,23 +167,19 @@ public class ReplaceIfWithSwitchIntention extends Intention { commentText = comment.getText(); } out.addComment(commentText); - comment = getPrevSiblingOfType(comment, PsiComment.class, - PsiStatement.class); + comment = getPrevSiblingOfType(comment, PsiComment.class, PsiStatement.class); } } - private static void extractStatementComments(PsiElement element, - IfStatementBranch out) { - PsiComment comment = getPrevSiblingOfType(element, - PsiComment.class, PsiStatement.class, PsiKeyword.class); + private static void extractStatementComments(PsiElement element, IfStatementBranch out) { + PsiComment comment = getPrevSiblingOfType(element, PsiComment.class, PsiStatement.class, PsiKeyword.class); while (comment != null) { final PsiElement sibling = comment.getPrevSibling(); final String commentText; if (sibling instanceof PsiWhiteSpace) { final String whiteSpaceText = sibling.getText(); if (whiteSpaceText.startsWith("\n")) { - commentText = whiteSpaceText.substring(1) + - comment.getText(); + commentText = whiteSpaceText.substring(1) + comment.getText(); } else { commentText = comment.getText(); @@ -239,14 +189,11 @@ public class ReplaceIfWithSwitchIntention extends Intention { commentText = comment.getText(); } out.addStatementComment(commentText); - comment = getPrevSiblingOfType(comment, PsiComment.class, - PsiStatement.class, PsiKeyword.class); + comment = getPrevSiblingOfType(comment, PsiComment.class, PsiStatement.class, PsiKeyword.class); } } - private static void termReplace( - PsiElement target, PsiElement replace, - StringBuilder stringToReplaceWith, StringBuilder out) { + private static void termReplace(PsiElement target, PsiElement replace, StringBuilder stringToReplaceWith, StringBuilder out) { if (target.equals(replace)) { out.append(stringToReplaceWith); } @@ -261,120 +208,116 @@ public class ReplaceIfWithSwitchIntention extends Intention { } } - private static List getValuesFromExpression( - PsiExpression expression, PsiExpression caseExpression, - List values) { + private static void extractCaseExpressions(PsiExpression expression, PsiExpression switchExpression, IfStatementBranch values) { if (expression instanceof PsiMethodCallExpression) { - final PsiMethodCallExpression methodCallExpression = - (PsiMethodCallExpression)expression; - final PsiExpressionList argumentList = - methodCallExpression.getArgumentList(); + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); final PsiExpression[] arguments = argumentList.getExpressions(); final PsiExpression argument = arguments[0]; - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); - final PsiExpression qualifierExpression = - methodExpression.getQualifierExpression(); - if (EquivalenceChecker.expressionsAreEquivalent(caseExpression, - argument)) { - values.add(qualifierExpression); + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); + if (EquivalenceChecker.expressionsAreEquivalent(switchExpression, argument)) { + values.addCaseExpression(qualifierExpression); } else { - values.add(argument); + values.addCaseExpression(argument); } } else if (expression instanceof PsiBinaryExpression) { - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)expression; + final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)expression; final PsiExpression lhs = binaryExpression.getLOperand(); final PsiExpression rhs = binaryExpression.getROperand(); final IElementType tokenType = binaryExpression.getOperationTokenType(); if (JavaTokenType.OROR.equals(tokenType)) { - getValuesFromExpression(lhs, caseExpression, - values); - getValuesFromExpression(rhs, caseExpression, - values); + extractCaseExpressions(lhs, switchExpression, values); + extractCaseExpressions(rhs, switchExpression, values); } else { - if (EquivalenceChecker.expressionsAreEquivalent(caseExpression, - rhs)) { - values.add(lhs); + if (EquivalenceChecker.expressionsAreEquivalent(switchExpression, rhs)) { + values.addCaseExpression(lhs); } else { - values.add(rhs); + values.addCaseExpression(rhs); } } } else if (expression instanceof PsiParenthesizedExpression) { - final PsiParenthesizedExpression parenthesizedExpression = - (PsiParenthesizedExpression)expression; - final PsiExpression contents = - parenthesizedExpression.getExpression(); - getValuesFromExpression(contents, caseExpression, values); + final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression; + final PsiExpression contents = parenthesizedExpression.getExpression(); + extractCaseExpressions(contents, switchExpression, values); } - return values; } - private static void dumpBranch(IfStatementBranch branch, - boolean wrap, - boolean renameBreaks, - String breakLabelName, - StringBuilder switchStatementText) { + private static void dumpBranch(IfStatementBranch branch, boolean castToInt, boolean wrap, boolean renameBreaks, String breakLabelName, + @NonNls StringBuilder switchStatementText) { dumpComments(branch.getComments(), switchStatementText); if (branch.isElse()) { switchStatementText.append("default: "); } else { - for (String label : branch.getConditions()) { - switchStatementText.append("case "); - switchStatementText.append(label); - switchStatementText.append(": "); + for (PsiExpression caseExpression : branch.getCaseExpressions()) { + switchStatementText.append("case ").append(getCaseLabelText(caseExpression, castToInt)).append(": "); } } dumpComments(branch.getStatementComments(), switchStatementText); - dumpBody(branch.getStatement(), wrap, renameBreaks, breakLabelName, - switchStatementText - ); + dumpBody(branch.getStatement(), wrap, renameBreaks, breakLabelName, switchStatementText); } - private static void dumpComments(List comments, - StringBuilder switchStatementText) { - if (!comments.isEmpty()) { - switchStatementText.append('\n'); - for (String comment : comments) { - switchStatementText.append(comment); - switchStatementText.append('\n'); + @NonNls + private static String getCaseLabelText(PsiExpression expression, boolean castToInt) { + if (expression instanceof PsiReferenceExpression) { + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; + final PsiElement target = referenceExpression.resolve(); + if (target instanceof PsiEnumConstant) { + final PsiEnumConstant enumConstant = (PsiEnumConstant)target; + return enumConstant.getName(); } } + if (castToInt) { + final PsiType type = expression.getType(); + if (!PsiType.INT.equals(type)) { + /* + because + Integer a = 1; + switch (a) { + case (byte)7: + } + does not compile with javac (but does with Eclipse) + */ + return "(int)" + expression.getText(); + } + } + return expression.getText(); + } + + private static void dumpComments(List comments, StringBuilder switchStatementText) { + if (comments.isEmpty()) { + return; + } + switchStatementText.append('\n'); + for (String comment : comments) { + switchStatementText.append(comment).append('\n'); + } } - private static void dumpBody(PsiStatement bodyStatement, - boolean wrap, - boolean renameBreaks, - String breakLabelName, + private static void dumpBody(PsiStatement bodyStatement, boolean wrap, boolean renameBreaks, String breakLabelName, @NonNls StringBuilder switchStatementText) { if (wrap) { switchStatementText.append('{'); } if (bodyStatement instanceof PsiBlockStatement) { - final PsiCodeBlock codeBlock = - ((PsiBlockStatement)bodyStatement).getCodeBlock(); + final PsiCodeBlock codeBlock = ((PsiBlockStatement)bodyStatement).getCodeBlock(); final PsiElement[] children = codeBlock.getChildren(); //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(child, renameBreaks, breakLabelName, - switchStatementText - ); + appendElement(child, renameBreaks, breakLabelName, switchStatementText); } } else { - appendElement(bodyStatement, renameBreaks, breakLabelName, - switchStatementText - ); + appendElement(bodyStatement, renameBreaks, breakLabelName, switchStatementText); } - if (ControlFlowUtils.statementMayCompleteNormally( - bodyStatement)) { + if (ControlFlowUtils.statementMayCompleteNormally(bodyStatement)) { switchStatementText.append("break;"); } if (wrap) { @@ -382,39 +325,43 @@ public class ReplaceIfWithSwitchIntention extends Intention { } } - private static void appendElement(PsiElement element, - boolean renameBreakElements, - String breakLabelString, + private static void appendElement(PsiElement element, boolean renameBreakElements, String breakLabelString, @NonNls StringBuilder switchStatementText) { final String text = element.getText(); if (!renameBreakElements) { switchStatementText.append(text); } else if (element instanceof PsiBreakStatement) { - final PsiBreakStatement breakStatement = - (PsiBreakStatement)element; - final PsiIdentifier identifier = - breakStatement.getLabelIdentifier(); + final PsiBreakStatement breakStatement = (PsiBreakStatement)element; + final PsiIdentifier identifier = breakStatement.getLabelIdentifier(); if (identifier == null) { - switchStatementText.append("break "); - switchStatementText.append(breakLabelString); - switchStatementText.append(';'); + switchStatementText.append("break ").append(breakLabelString).append(';'); } else { switchStatementText.append(text); } } - else if (element instanceof PsiBlockStatement || - element instanceof PsiCodeBlock || - element instanceof PsiIfStatement) { + else if (element instanceof PsiBlockStatement || element instanceof PsiCodeBlock || element instanceof PsiIfStatement) { final PsiElement[] children = element.getChildren(); for (final PsiElement child : children) { - appendElement(child, renameBreakElements, breakLabelString, - switchStatementText); + appendElement(child, renameBreakElements, breakLabelString, switchStatementText); } } else { switchStatementText.append(text); } + final PsiElement lastChild = element.getLastChild(); + if (isEndOfLineComment(lastChild)) { + switchStatementText.append('\n'); + } } -} + + private static boolean isEndOfLineComment(PsiElement element) { + if (!(element instanceof PsiComment)) { + return false; + } + final PsiComment comment = (PsiComment)element; + final IElementType tokenType = comment.getTokenType(); + return JavaTokenType.END_OF_LINE_COMMENT.equals(tokenType); + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments.java new file mode 100644 index 000000000000..5a42a37ef3b0 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments.java @@ -0,0 +1,14 @@ +class Comments { + + String foo(int par) { + if(par == 11) + return "ciao";/* case 1 bla bla */ + else if(par == 14) + return "fourteen";//case 2 bla bla + else if(par == 15) + return "fifteen"; //case 3 chchah + else + return "default"; + + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments_after.java new file mode 100644 index 000000000000..3922f442af44 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/switchtoif/replace_if_with_switch/Comments_after.java @@ -0,0 +1,16 @@ +class Comments { + + String foo(int par) { + switch (par) { + case 11: + return "ciao";/* case 1 bla bla */ + case 14: + return "fourteen";//case 2 bla bla + case 15: + return "fifteen"; //case 3 chchah + default: + return "default"; + } + + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchlIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchlIntentionTest.java new file mode 100644 index 000000000000..67d27bac505e --- /dev/null +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/switchtoif/ReplaceIfWithSwitchlIntentionTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2013 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.siyeh.ipp.switchtoif; + +import com.siyeh.IntentionPowerPackBundle; +import com.siyeh.ipp.IPPTestCase; + +public class ReplaceIfWithSwitchlIntentionTest extends IPPTestCase { + + public void testComments() { + doTest(); + } + + @Override + protected String getIntentionName() { + return IntentionPowerPackBundle.message("replace.if.with.switch.intention.name"); + } + + @Override + protected String getRelativePath() { + return "switchtoif/replace_if_with_switch"; + } +}