Skip to content

Commit 4369701

Browse files
jjohnstnjjohnstn
authored
Add pattern name fixing for pattern instanceof to switch for Java 22 (eclipse-jdt#2018)
- modify PatternInstanceofToSwitchFixCore to recognize when using Java 22 and up and the name specified in the pattern instanceof is not used by the if's then statement in which case replace it with an unnamed pattern instanceof in the switch - add new tests to CleanUpTest22 - fixes eclipse-jdt#2016
1 parent 50bc3ca commit 4369701

File tree

2 files changed

+246
-11
lines changed

2 files changed

+246
-11
lines changed

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

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,24 @@
4141
import org.eclipse.jdt.core.dom.ExpressionStatement;
4242
import org.eclipse.jdt.core.dom.ForStatement;
4343
import org.eclipse.jdt.core.dom.IBinding;
44+
import org.eclipse.jdt.core.dom.ITypeBinding;
4445
import org.eclipse.jdt.core.dom.IVariableBinding;
4546
import org.eclipse.jdt.core.dom.IfStatement;
4647
import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
4748
import org.eclipse.jdt.core.dom.Name;
4849
import org.eclipse.jdt.core.dom.PatternInstanceofExpression;
4950
import org.eclipse.jdt.core.dom.ReturnStatement;
51+
import org.eclipse.jdt.core.dom.SimpleName;
52+
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
5053
import org.eclipse.jdt.core.dom.Statement;
5154
import org.eclipse.jdt.core.dom.SwitchCase;
5255
import org.eclipse.jdt.core.dom.SwitchExpression;
5356
import org.eclipse.jdt.core.dom.SwitchStatement;
5457
import org.eclipse.jdt.core.dom.ThrowStatement;
5558
import org.eclipse.jdt.core.dom.TryStatement;
59+
import org.eclipse.jdt.core.dom.Type;
5660
import org.eclipse.jdt.core.dom.TypePattern;
61+
import org.eclipse.jdt.core.dom.VariableDeclaration;
5762
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
5863
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
5964
import org.eclipse.jdt.core.dom.WhileStatement;
@@ -64,6 +69,7 @@
6469
import org.eclipse.jdt.core.dom.rewrite.TargetSourceRangeComputer;
6570

6671
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
72+
import org.eclipse.jdt.internal.corext.dom.AbortSearchException;
6773
import org.eclipse.jdt.internal.corext.fix.SwitchExpressionsFixCore.SwitchExpressionsFixOperation;
6874
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
6975
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
@@ -77,10 +83,16 @@ public static final class SwitchStatementsFinder extends ASTVisitor {
7783
static final class TypeVariable {
7884
private final Expression name;
7985
private final TypePattern typePattern;
86+
private boolean patternNameUsed;
8087

8188
private TypeVariable(final Expression firstOp, final TypePattern typePattern) {
89+
this(firstOp, typePattern, true);
90+
}
91+
92+
private TypeVariable(final Expression firstOp, final TypePattern typePattern, final boolean patternNameUsed) {
8293
this.name= firstOp;
8394
this.typePattern= typePattern;
95+
this.patternNameUsed= patternNameUsed;
8496
}
8597

8698
public boolean isSameVariable(final TypeVariable other) {
@@ -90,6 +102,14 @@ public boolean isSameVariable(final TypeVariable other) {
90102
public TypePattern getTypePattern() {
91103
return typePattern;
92104
}
105+
106+
public boolean isPatternNameUsed() {
107+
return patternNameUsed;
108+
}
109+
110+
public void setPatternNameUsed(boolean value) {
111+
patternNameUsed= value;
112+
}
93113
}
94114

95115
private List<CompilationUnitRewriteOperation> fResult;
@@ -127,7 +147,7 @@ public boolean visit(final IfStatement visited) {
127147
IfStatement currentNode= ifStatement;
128148

129149
while (ASTNodes.isSameVariable(switchExpression, variable.name)) {
130-
cases.add(new SwitchCaseSection(variable.getTypePattern(),
150+
cases.add(new SwitchCaseSection(variable.getTypePattern(), variable.isPatternNameUsed(),
131151
ASTNodes.asList(currentNode.getThenStatement())));
132152

133153
if (!ASTNodes.fallsThrough(currentNode.getThenStatement())) {
@@ -201,7 +221,7 @@ private PatternToSwitchExpressionOperation getOperation(List<IfStatement> ifStat
201221
}
202222
// add default as a case section with null type pattern
203223
List<SwitchCaseSection> extendedCases= new ArrayList<>(cases);
204-
SwitchCaseSection defaultSection= new SwitchCaseSection(null, ASTNodes.asList(remainingStatement));
224+
SwitchCaseSection defaultSection= new SwitchCaseSection(null, false, ASTNodes.asList(remainingStatement));
205225
extendedCases.add(defaultSection);
206226

207227
for (SwitchCaseSection switchCaseSection : extendedCases) {
@@ -295,11 +315,44 @@ private PatternToSwitchExpressionOperation getOperation(List<IfStatement> ifStat
295315
return null;
296316
}
297317

298-
private TypeVariable extractVariableAndValues(final Statement statement) {
299-
if (statement instanceof IfStatement) {
300-
return extractVariableAndValues(((IfStatement) statement).getExpression());
318+
private class NameUsedVisitor extends ASTVisitor {
319+
private final IBinding nameBinding;
320+
321+
public NameUsedVisitor(final IBinding nameBinding) {
322+
this.nameBinding= nameBinding;
301323
}
302324

325+
@Override
326+
public boolean visit(SimpleName node) {
327+
IBinding nodeBinding= node.resolveBinding();
328+
if (nodeBinding != null && nodeBinding.isEqualTo(nameBinding)) {
329+
throw new AbortSearchException();
330+
}
331+
return false;
332+
}
333+
}
334+
335+
private TypeVariable extractVariableAndValues(final Statement statement) {
336+
if (statement instanceof IfStatement ifStmt) {
337+
TypeVariable result= extractVariableAndValues(((IfStatement) statement).getExpression());
338+
if (result != null) {
339+
CompilationUnit cu= (CompilationUnit) ifStmt.getRoot();
340+
if (JavaModelUtil.is22OrHigher(cu.getJavaElement().getJavaProject())) {
341+
TypePattern pattern= result.getTypePattern();
342+
VariableDeclaration vd= pattern.getPatternVariable2();
343+
SimpleName name= vd.getName();
344+
IBinding binding= name.resolveBinding();
345+
NameUsedVisitor visitor= new NameUsedVisitor(binding);
346+
try {
347+
ifStmt.getThenStatement().accept(visitor);
348+
result.setPatternNameUsed(false);
349+
} catch (AbortSearchException e) {
350+
result.setPatternNameUsed(true);
351+
}
352+
}
353+
return result;
354+
}
355+
}
303356
return null;
304357
}
305358

@@ -341,6 +394,7 @@ public PatternToSwitchOperation(final List<IfStatement> ifStatements, final Expr
341394
@Override
342395
public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModelCore linkedModel) throws CoreException {
343396
ASTRewrite rewrite= cuRewrite.getASTRewrite();
397+
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
344398
AST ast= cuRewrite.getRoot().getAST();
345399
TextEditGroup group= createTextEditGroup(MultiFixMessages.CodeStyleCleanUp_PatternInstanceOfToSwitch_description, cuRewrite);
346400
rewrite.setTargetSourceRangeComputer(new TargetSourceRangeComputer() {
@@ -360,14 +414,14 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
360414
switchStatement.setExpression((Expression) rewrite.createCopyTarget(switchExpression));
361415

362416
for (SwitchCaseSection aCase : cases) {
363-
addCaseWithStatements(rewrite, ast, switchStatement, aCase.typePattern, aCase.statements);
417+
addCaseWithStatements(rewrite, importRewrite, ast, switchStatement, aCase.typePattern, aCase.isNameUsed, aCase.statements);
364418
}
365419

366420
if (remainingStatement != null) {
367421
remainingStatement.setProperty(UNTOUCH_COMMENT_PROPERTY, Boolean.TRUE);
368-
addCaseWithStatements(rewrite, ast, switchStatement, null, ASTNodes.asList(remainingStatement));
422+
addCaseWithStatements(rewrite, importRewrite, ast, switchStatement, null, false, ASTNodes.asList(remainingStatement));
369423
} else {
370-
addCaseWithStatements(rewrite, ast, switchStatement, null, Collections.emptyList());
424+
addCaseWithStatements(rewrite, importRewrite, ast, switchStatement, null, false, Collections.emptyList());
371425
}
372426

373427
for (int i= 0; i < ifStatements.size() - 1; i++) {
@@ -377,8 +431,10 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
377431
ASTNodes.replaceButKeepComment(rewrite, ifStatements.get(ifStatements.size() - 1), newStatement, group);
378432
}
379433

380-
private void addCaseWithStatements(final ASTRewrite rewrite, final AST ast, final SwitchStatement switchStatement,
434+
private void addCaseWithStatements(final ASTRewrite rewrite, final ImportRewrite importRewrite,
435+
final AST ast, final SwitchStatement switchStatement,
381436
final TypePattern caseValueOrNullForDefault,
437+
final boolean isNameUsed,
382438
final List<Statement> innerStatements) {
383439
List<Statement> switchStatements= switchStatement.statements();
384440
boolean needBlock= innerStatements.size() > 1 || (innerStatements.size() == 1 && innerStatements.get(0) instanceof ReturnStatement);
@@ -388,7 +444,24 @@ private void addCaseWithStatements(final ASTRewrite rewrite, final AST ast, fina
388444
Expression caseValue= caseValueOrNullForDefault;
389445
SwitchCase newSwitchCase= ast.newSwitchCase();
390446
newSwitchCase.setSwitchLabeledRule(true);
391-
newSwitchCase.expressions().add(ASTNodes.createMoveTarget(rewrite, caseValue));
447+
if (isNameUsed) {
448+
newSwitchCase.expressions().add(ASTNodes.createMoveTarget(rewrite, caseValue));
449+
} else {
450+
if (caseValueOrNullForDefault.getPatternVariable2() instanceof SingleVariableDeclaration svd
451+
&& svd.getType().resolveBinding() != null) {
452+
TypePattern newPattern= ast.newTypePattern();
453+
SingleVariableDeclaration newDecl= ast.newSingleVariableDeclaration();
454+
newDecl.setName(ast.newSimpleName("_")); //$NON-NLS-1$
455+
Type oldType= svd.getType();
456+
ITypeBinding typeBinding= oldType.resolveBinding();
457+
Type newType= importRewrite.addImport(typeBinding, ast);
458+
newDecl.setType(newType);
459+
newPattern.setPatternVariable((VariableDeclaration)newDecl);
460+
newSwitchCase.expressions().add(newPattern);
461+
} else {
462+
newSwitchCase.expressions().add(ASTNodes.createMoveTarget(rewrite, caseValue));
463+
}
464+
}
392465
switchStatements.add(newSwitchCase);
393466
} else {
394467
SwitchCase newSwitchCase= ast.newSwitchCase();
@@ -458,6 +531,7 @@ public PatternToSwitchExpressionOperation(final List<IfStatement> ifStatements,
458531
@Override
459532
public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException {
460533
final ASTRewrite rewrite= cuRewrite.getASTRewrite();
534+
final ImportRewrite importRewrite= cuRewrite.getImportRewrite();
461535
final AST ast= rewrite.getAST();
462536

463537
TextEditGroup group= createTextEditGroup(MultiFixMessages.CodeStyleCleanUp_PatternInstanceOfToSwitch_description, cuRewrite);
@@ -482,6 +556,18 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
482556
} else {
483557
Expression oldExpression= switchCaseSection.typePattern;
484558
Expression newExpression= (Expression)rewrite.createCopyTarget(oldExpression);
559+
if (!switchCaseSection.isNameUsed && switchCaseSection.typePattern.getPatternVariable2() instanceof SingleVariableDeclaration svd
560+
&& svd.getType().resolveBinding() != null) {
561+
TypePattern newPattern= ast.newTypePattern();
562+
SingleVariableDeclaration newDecl= ast.newSingleVariableDeclaration();
563+
newDecl.setName(ast.newSimpleName("_")); //$NON-NLS-1$
564+
Type oldType= svd.getType();
565+
ITypeBinding typeBinding= oldType.resolveBinding();
566+
Type newType= importRewrite.addImport(typeBinding, ast);
567+
newDecl.setType(newType);
568+
newPattern.setPatternVariable((VariableDeclaration)newDecl);
569+
newExpression= newPattern;
570+
}
485571
switchCase.expressions().add(newExpression);
486572
}
487573

@@ -566,7 +652,6 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
566652
newVarFragment.setName(ast.newSimpleName(varName));
567653
newVarFragment.setInitializer(newSwitchExpression);
568654
VariableDeclarationStatement newVar= ast.newVariableDeclarationStatement(newVarFragment);
569-
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
570655
newVar.setType(importRewrite.addImport(assignmentBinding.getType(), ast));
571656
if (varDeclarationStatement != null && Modifier.isFinal(varDeclarationStatement.getModifiers())) {
572657
newVar.modifiers().add(ast.newModifier(ModifierKeyword.FINAL_KEYWORD));
@@ -665,15 +750,20 @@ private static final class SwitchCaseSection {
665750
/** The type pattern to use for the case. */
666751
private final TypePattern typePattern;
667752

753+
/** Whether the type pattern variable is used. */
754+
private final boolean isNameUsed;
755+
668756
/**
669757
* Used for if statements, only constant expressions are used.
670758
*
671759
* @param typePattern The type pattern
672760
* @param statements The statements
673761
*/
674762
private SwitchCaseSection(final TypePattern typePattern,
763+
final boolean isNameUsed,
675764
final List<Statement> statements) {
676765
this.typePattern= typePattern;
766+
this.isNameUsed= isNameUsed;
677767
this.statements= statements;
678768
}
679769

0 commit comments

Comments
 (0)