Skip to content

Commit b8fb344

Browse files
jjohnstnjjohnstn
authored
Fix if/else to switch to handle null scenarios (#1726)
- for < JVM 21 add an if/else that checks for null - for JVM 21 and up, add case null - modified SwitchFixCore and modified tests in CleanUpTest12 to also not qualify enums if the switch value is an enum - fixes #1714
1 parent 4f8849c commit b8fb344

File tree

2 files changed

+108
-57
lines changed
  • org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix
  • org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix

2 files changed

+108
-57
lines changed

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SwitchFixCore.java‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@
4444
import org.eclipse.jdt.core.dom.ITypeBinding;
4545
import org.eclipse.jdt.core.dom.IfStatement;
4646
import org.eclipse.jdt.core.dom.InfixExpression;
47+
import org.eclipse.jdt.core.dom.InfixExpression.Operator;
4748
import org.eclipse.jdt.core.dom.MethodInvocation;
4849
import org.eclipse.jdt.core.dom.Name;
50+
import org.eclipse.jdt.core.dom.QualifiedName;
4951
import org.eclipse.jdt.core.dom.Statement;
5052
import org.eclipse.jdt.core.dom.SuperFieldAccess;
5153
import org.eclipse.jdt.core.dom.SwitchCase;
@@ -62,6 +64,7 @@
6264
import org.eclipse.jdt.internal.corext.refactoring.nls.NLSElement;
6365
import org.eclipse.jdt.internal.corext.refactoring.nls.NLSLine;
6466
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
67+
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
6568

6669
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
6770

@@ -432,29 +435,54 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
432435
});
433436

434437
SwitchStatement switchStatement= ast.newSwitchStatement();
435-
switchStatement.setExpression(ASTNodes.createMoveTarget(rewrite, switchExpression));
438+
ASTNode newStatement= switchStatement;
439+
440+
CompilationUnit unit= cuRewrite.getRoot();
441+
ICompilationUnit cu= (ICompilationUnit)unit.getJavaElement();
442+
ITypeBinding switchTypeBinding= switchExpression.resolveTypeBinding();
443+
boolean isEnum= switchTypeBinding != null && switchTypeBinding.isEnum();
444+
445+
if (cu != null && !JavaModelUtil.is21OrHigher(cu.getJavaProject())) {
446+
if (switchTypeBinding != null && !switchTypeBinding.isPrimitive()) {
447+
IfStatement newIfStatement= ast.newIfStatement();
448+
InfixExpression newInfixExpression= ast.newInfixExpression();
449+
newInfixExpression.setOperator(Operator.NOT_EQUALS);
450+
newInfixExpression.setLeftOperand((Expression) rewrite.createCopyTarget(switchExpression));
451+
newInfixExpression.setRightOperand(ast.newNullLiteral());
452+
newIfStatement.setExpression(newInfixExpression);
453+
Block newBlock= ast.newBlock();
454+
newIfStatement.setThenStatement(newBlock);
455+
newBlock.statements().add(switchStatement);
456+
newStatement= newIfStatement;
457+
}
458+
}
459+
460+
switchStatement.setExpression((Expression) rewrite.createCopyTarget(switchExpression));
436461

437462
for (SwitchCaseSection aCase : cases) {
438-
addCaseWithStatements(rewrite, ast, switchStatement, aCase.literalExpressions, aCase.tagList, aCase.statements);
463+
addCaseWithStatements(rewrite, ast, switchStatement, aCase.literalExpressions, aCase.tagList, aCase.statements, isEnum);
439464
}
440465

441466
if (remainingStatement != null) {
442467
remainingStatement.setProperty(UNTOUCH_COMMENT_PROPERTY, Boolean.TRUE);
443-
addCaseWithStatements(rewrite, ast, switchStatement, null, null, ASTNodes.asList(remainingStatement));
468+
addCaseWithStatements(rewrite, ast, switchStatement, null, null, ASTNodes.asList(remainingStatement), isEnum);
469+
if (newStatement instanceof IfStatement ifStatement) {
470+
ifStatement.setElseStatement((Statement) rewrite.createCopyTarget(remainingStatement));
471+
}
444472
} else {
445-
addCaseWithStatements(rewrite, ast, switchStatement, null, null, Collections.emptyList());
473+
addCaseWithStatements(rewrite, ast, switchStatement, null, null, Collections.emptyList(), isEnum);
446474
}
447475

448476
for (int i= 0; i < ifStatements.size() - 1; i++) {
449477
ASTNodes.removeButKeepComment(rewrite, ifStatements.get(i), group);
450478
}
451479

452-
ASTNodes.replaceButKeepComment(rewrite, ifStatements.get(ifStatements.size() - 1), switchStatement, group);
480+
ASTNodes.replaceButKeepComment(rewrite, ifStatements.get(ifStatements.size() - 1), newStatement, group);
453481
}
454482

455483
private void addCaseWithStatements(final ASTRewrite rewrite, final AST ast, final SwitchStatement switchStatement,
456484
final List<Expression> caseValuesOrNullForDefault, final List<Boolean> tagList,
457-
final List<Statement> innerStatements) {
485+
final List<Statement> innerStatements, boolean isEnum) {
458486
List<Statement> switchStatements= switchStatement.statements();
459487
boolean needBlock= checkForLocalDeclarations(innerStatements);
460488
Boolean hasTag= false;
@@ -466,6 +494,9 @@ private void addCaseWithStatements(final ASTRewrite rewrite, final AST ast, fina
466494
String spaceBefore= getCoreOption(cu.getJavaProject(), DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_BEFORE_COLON_IN_CASE, false) ? " " : ""; //$NON-NLS-1$ //$NON-NLS-2$
467495
for (int i= 0; i < caseValuesOrNullForDefault.size(); ++i) {
468496
Expression caseValue= caseValuesOrNullForDefault.get(i);
497+
if (isEnum && caseValue instanceof QualifiedName qName) {
498+
caseValue= qName.getName();
499+
}
469500
hasTag= tagList.get(i);
470501
if (hasTag) {
471502
try {
@@ -487,6 +518,16 @@ private void addCaseWithStatements(final ASTRewrite rewrite, final AST ast, fina
487518
}
488519
}
489520
} else {
521+
CompilationUnit unit= (CompilationUnit)switchExpression.getRoot();
522+
ICompilationUnit cu= (ICompilationUnit)unit.getJavaElement();
523+
if (cu != null && JavaModelUtil.is21OrHigher(cu.getJavaProject())) {
524+
ITypeBinding switchTypeBinding= switchExpression.resolveTypeBinding();
525+
if (switchTypeBinding != null && !switchTypeBinding.isPrimitive()) {
526+
SwitchCase newSwitchCase= ast.newSwitchCase();
527+
newSwitchCase.expressions().add(ast.newNullLiteral());
528+
switchStatements.add(newSwitchCase);
529+
}
530+
}
490531
switchStatements.add(ast.newSwitchCase());
491532
}
492533

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest12.java‎

Lines changed: 61 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,36 +1131,41 @@ public class E {
11311131
11321132
public void bug1(String i1) {
11331133
int i = 0;
1134-
switch (i1) {
1135-
case E.VALUE0 : {
1136-
int integer1 = 0;
1137-
i = integer1;
1138-
break;
1139-
}
1140-
case E.VALUE1 : {
1141-
char integer1 = 'a';
1142-
i = integer1;
1143-
break;
1144-
}
1145-
case E.VALUE2 : {
1146-
char integer1 = 'b';
1147-
i = integer1;
1148-
break;
1149-
}
1150-
case "5" : //$NON-NLS-1$
1151-
case "five" :
1152-
case "another string" : //$NON-NLS-1$
1153-
{
1154-
char integer1 = 'c';
1155-
i = integer1;
1156-
break;
1157-
}
1158-
default :
1159-
if (computeit(i1) || i1.equals(E.VALUE3)) {
1160-
//
1161-
//
1134+
if (i1 != null) {
1135+
switch (i1) {
1136+
case E.VALUE0 : {
1137+
int integer1 = 0;
1138+
i = integer1;
1139+
break;
1140+
}
1141+
case E.VALUE1 : {
1142+
char integer1 = 'a';
1143+
i = integer1;
1144+
break;
11621145
}
1163-
break;
1146+
case E.VALUE2 : {
1147+
char integer1 = 'b';
1148+
i = integer1;
1149+
break;
1150+
}
1151+
case "5" : //$NON-NLS-1$
1152+
case "five" :
1153+
case "another string" : //$NON-NLS-1$
1154+
{
1155+
char integer1 = 'c';
1156+
i = integer1;
1157+
break;
1158+
}
1159+
default :
1160+
if (computeit(i1) || i1.equals(E.VALUE3)) {
1161+
//
1162+
//
1163+
}
1164+
break;
1165+
}
1166+
} else if (computeit(i1) || i1.equals(E.VALUE3)) {
1167+
//
1168+
//
11641169
}
11651170
}
11661171
@@ -1221,29 +1226,34 @@ public enum MYENUM { VALUE0,VALUE1,VALUE2,VALUE3,VALUE4,VALUE5}
12211226
12221227
public void bug1(MYENUM i1) {
12231228
int i = 0;
1224-
switch (i1) {
1225-
case MYENUM.VALUE0 : {
1226-
int integer1 = 0;
1227-
i = integer1;
1228-
break;
1229-
}
1230-
case MYENUM.VALUE1 : {
1231-
char integer1 = 'a';
1232-
i = integer1;
1233-
break;
1234-
}
1235-
case MYENUM.VALUE2 : {
1236-
char integer1 = 'b';
1237-
i = integer1;
1238-
break;
1239-
}
1240-
default :
1241-
if (computeit(i1) || i1 == MYENUM.VALUE3) {
1242-
//
1243-
//
1229+
if (i1 != null) {
1230+
switch (i1) {
1231+
case VALUE0 : {
1232+
int integer1 = 0;
1233+
i = integer1;
1234+
break;
12441235
}
1245-
break;
1246-
}
1236+
case VALUE1 : {
1237+
char integer1 = 'a';
1238+
i = integer1;
1239+
break;
1240+
}
1241+
case VALUE2 : {
1242+
char integer1 = 'b';
1243+
i = integer1;
1244+
break;
1245+
}
1246+
default :
1247+
if (computeit(i1) || i1 == MYENUM.VALUE3) {
1248+
//
1249+
//
1250+
}
1251+
break;
1252+
}
1253+
} else if (computeit(i1) || i1 == MYENUM.VALUE3) {
1254+
//
1255+
//
1256+
}
12471257
}
12481258
12491259
private boolean computeit(MYENUM i) {

0 commit comments

Comments
 (0)