Skip to content

Commit 4816a1e

Browse files
jjohnstnjjohnstn
authored
Add conditional expression support to pattern instanceof cleanup (eclipse-jdt#2102)
* Add conditional expression support to pattern instanceof cleanup - modify PatternMatchingForInstanceofFixCore to handle conditional expressions in addition to if statements - add new scenarios to CleanUpTest16 - fixes eclipse-jdt#2094
1 parent 6eb3aed commit 4816a1e

File tree

2 files changed

+173
-17
lines changed

2 files changed

+173
-17
lines changed

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

Lines changed: 145 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,36 @@
1414
package org.eclipse.jdt.internal.corext.fix;
1515

1616
import java.util.ArrayList;
17+
import java.util.Collection;
18+
import java.util.HashSet;
1719
import java.util.List;
1820
import java.util.Objects;
21+
import java.util.Set;
1922

2023
import org.eclipse.core.runtime.CoreException;
2124

2225
import org.eclipse.text.edits.TextEditGroup;
2326

27+
import org.eclipse.jdt.core.IJavaProject;
2428
import org.eclipse.jdt.core.dom.AST;
2529
import org.eclipse.jdt.core.dom.ASTNode;
2630
import org.eclipse.jdt.core.dom.ASTVisitor;
2731
import org.eclipse.jdt.core.dom.Assignment;
2832
import org.eclipse.jdt.core.dom.Block;
2933
import org.eclipse.jdt.core.dom.CastExpression;
3034
import org.eclipse.jdt.core.dom.CompilationUnit;
35+
import org.eclipse.jdt.core.dom.ConditionalExpression;
3136
import org.eclipse.jdt.core.dom.Expression;
3237
import org.eclipse.jdt.core.dom.ExpressionStatement;
38+
import org.eclipse.jdt.core.dom.ITypeBinding;
3339
import org.eclipse.jdt.core.dom.IfStatement;
3440
import org.eclipse.jdt.core.dom.InfixExpression;
3541
import org.eclipse.jdt.core.dom.InfixExpression.Operator;
3642
import org.eclipse.jdt.core.dom.InstanceofExpression;
43+
import org.eclipse.jdt.core.dom.MethodDeclaration;
3744
import org.eclipse.jdt.core.dom.Modifier;
3845
import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
46+
import org.eclipse.jdt.core.dom.ParenthesizedExpression;
3947
import org.eclipse.jdt.core.dom.PatternInstanceofExpression;
4048
import org.eclipse.jdt.core.dom.PrefixExpression;
4149
import org.eclipse.jdt.core.dom.SimpleName;
@@ -48,7 +56,9 @@
4856
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
4957
import org.eclipse.jdt.core.dom.rewrite.TargetSourceRangeComputer;
5058

59+
import org.eclipse.jdt.internal.core.manipulation.StubUtility;
5160
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
61+
import org.eclipse.jdt.internal.corext.dom.ScopeAnalyzer;
5262
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
5363
import org.eclipse.jdt.internal.corext.refactoring.structure.ImportRemover;
5464

@@ -58,9 +68,9 @@
5868

5969
public class PatternMatchingForInstanceofFixCore extends CompilationUnitRewriteOperationsFixCore {
6070
public static final class PatternMatchingForInstanceofFinder extends ASTVisitor {
61-
private List<PatternMatchingForInstanceofFixOperation> fResult;
71+
private List<CompilationUnitRewriteOperation> fResult;
6272

63-
public PatternMatchingForInstanceofFinder(List<PatternMatchingForInstanceofFixOperation> ops) {
73+
public PatternMatchingForInstanceofFinder(List<CompilationUnitRewriteOperation> ops) {
6474
fResult= ops;
6575
}
6676

@@ -74,6 +84,7 @@ public boolean visit(final Block visited) {
7484
final class InstanceofVisitor extends ASTVisitor {
7585
private final Block startNode;
7686
private boolean result= true;
87+
private final Set<String> excludedNames= new HashSet<>();
7788

7889
public InstanceofVisitor(final Block startNode) {
7990
this.startNode= startNode;
@@ -101,9 +112,14 @@ public boolean visit(final InstanceofExpression visited) {
101112
boolean isPositiveCaseToAnalyze= true;
102113
ASTNode currentNode= visited;
103114

115+
if (visited.getLocationInParent() == ConditionalExpression.EXPRESSION_PROPERTY) {
116+
117+
}
104118
while (currentNode.getParent() != null
105119
&& (!(currentNode.getParent() instanceof IfStatement)
106-
|| currentNode.getLocationInParent() != IfStatement.EXPRESSION_PROPERTY)) {
120+
|| currentNode.getLocationInParent() != IfStatement.EXPRESSION_PROPERTY)
121+
&& (!(currentNode.getParent() instanceof ConditionalExpression)
122+
|| currentNode.getLocationInParent() != ConditionalExpression.EXPRESSION_PROPERTY)) {
107123
switch (currentNode.getParent().getNodeType()) {
108124
case ASTNode.PARENTHESIZED_EXPRESSION:
109125
break;
@@ -143,22 +159,96 @@ public boolean visit(final InstanceofExpression visited) {
143159
}
144160
}
145161

146-
IfStatement ifStatement= (IfStatement) currentNode.getParent();
162+
if (currentNode.getParent() instanceof ConditionalExpression condExp
163+
&& visited.getLeftOperand() instanceof SimpleName name) {
164+
ConditionalCollector collector= new ConditionalCollector(visited, name, excludedNames);
165+
if (isPositiveCaseToAnalyze) {
166+
condExp.getThenExpression().accept(collector);
167+
} else {
168+
condExp.getElseExpression().accept(collector);
169+
}
170+
if (collector.hasResult()) {
171+
fResult.add(collector.build());
172+
return false;
173+
}
174+
} else {
175+
IfStatement ifStatement= (IfStatement) currentNode.getParent();
176+
177+
ResultCollector collector= new ResultCollector(visited);
178+
if (isPositiveCaseToAnalyze) {
179+
collector.collect(ifStatement.getThenStatement());
180+
} else if (ifStatement.getElseStatement() != null) {
181+
collector.collect(ifStatement.getElseStatement());
182+
} else if (ASTNodes.fallsThrough(ifStatement.getThenStatement())) {
183+
collector.collect(ASTNodes.getNextSibling(ifStatement));
184+
}
185+
186+
if (collector.hasResult()) {
187+
fResult.add(collector.build());
188+
return false;
189+
}
190+
}
191+
return true;
192+
}
193+
194+
static class ConditionalCollector extends ASTVisitor {
195+
final InstanceofExpression visited;
196+
final Set<String> excludedNames;
197+
final SimpleName variableName;
198+
String replacementName= null;
199+
final List<Expression> expressionsToReplace = new ArrayList<>();
200+
201+
ConditionalCollector(InstanceofExpression visited, SimpleName name, Set<String> excludedNames) {
202+
this.visited = visited;
203+
this.excludedNames= excludedNames;
204+
this.variableName= name; }
147205

148-
ResultCollector collector = new ResultCollector(visited);
149-
if (isPositiveCaseToAnalyze) {
150-
collector.collect(ifStatement.getThenStatement());
151-
} else if (ifStatement.getElseStatement() != null) {
152-
collector.collect(ifStatement.getElseStatement());
153-
} else if (ASTNodes.fallsThrough(ifStatement.getThenStatement())) {
154-
collector.collect(ASTNodes.getNextSibling(ifStatement));
206+
public PatternMatchingForConditionalInstanceofFixOperation build() {
207+
return new PatternMatchingForConditionalInstanceofFixOperation(visited, expressionsToReplace, replacementName);
155208
}
156209

157-
if (collector.hasResult()) {
158-
fResult.add(collector.build());
210+
boolean hasResult() {
211+
return replacementName != null && !expressionsToReplace.isEmpty();
212+
}
213+
214+
void addMatching(final CastExpression castExp, final SimpleName name) {
215+
Expression expressionToReplace= castExp;
216+
ITypeBinding castBinding= castExp.resolveTypeBinding();
217+
if (this.variableName.getIdentifier().equals(name.getIdentifier())
218+
&& castBinding != null && castBinding.isEqualTo(visited.getRightOperand().resolveBinding())) {
219+
while (expressionToReplace.getLocationInParent() == ParenthesizedExpression.EXPRESSION_PROPERTY) {
220+
expressionToReplace= (Expression) expressionToReplace.getParent();
221+
}
222+
if (replacementName == null) {
223+
String baseName= castBinding.getName().toLowerCase().substring(0, 1);
224+
MethodDeclaration methodDecl= ASTNodes.getFirstAncestorOrNull(name, MethodDeclaration.class);
225+
if (methodDecl != null) {
226+
CompilationUnit cu= (CompilationUnit) name.getRoot();
227+
IJavaProject project= cu.getJavaElement().getJavaProject();
228+
replacementName= proposeLocalName(baseName, cu, project, methodDecl, excludedNames.toArray(new String[0]));
229+
}
230+
}
231+
this.expressionsToReplace.add(expressionToReplace);
232+
}
233+
}
234+
235+
private String proposeLocalName(String baseName, CompilationUnit root, IJavaProject javaProject, MethodDeclaration methodDecl, String[] usedNames) {
236+
// don't propose names that are already in use:
237+
Collection<String> variableNames= new ScopeAnalyzer(root).getUsedVariableNames(methodDecl.getStartPosition(), methodDecl.getLength());
238+
String[] names = new String[variableNames.size() + usedNames.length];
239+
variableNames.toArray(names);
240+
System.arraycopy(usedNames, 0, names, variableNames.size(), usedNames.length);
241+
return StubUtility.getLocalNameSuggestions(javaProject, baseName, 0, names)[0];
242+
}
243+
244+
@Override
245+
public boolean visit(SimpleName node) {
246+
if (node.getLocationInParent() == CastExpression.EXPRESSION_PROPERTY) {
247+
addMatching((CastExpression)node.getParent(), node);
248+
}
159249
return false;
160250
}
161-
return true;
251+
162252
}
163253

164254
static class ResultCollector {
@@ -256,6 +346,46 @@ boolean collect(final Statement conditionalStatements) {
256346
}
257347
}
258348

349+
public static class PatternMatchingForConditionalInstanceofFixOperation extends CompilationUnitRewriteOperation {
350+
private final InstanceofExpression nodeToComplete;
351+
private final List<Expression> expressionsToReplace;
352+
private final String replacementName;
353+
354+
public PatternMatchingForConditionalInstanceofFixOperation(final InstanceofExpression nodeToComplete, final List<Expression> expressionsToReplace, final String replacementName) {
355+
this.nodeToComplete= nodeToComplete;
356+
this.expressionsToReplace= expressionsToReplace;
357+
this.replacementName= replacementName;
358+
}
359+
360+
@SuppressWarnings("deprecation")
361+
@Override
362+
public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModelCore linkedModel) throws CoreException {
363+
ASTRewrite rewrite= cuRewrite.getASTRewrite();
364+
AST ast= rewrite.getAST();
365+
TextEditGroup group= createTextEditGroup(MultiFixMessages.PatternMatchingForInstanceofCleanup_description, cuRewrite);
366+
367+
PatternInstanceofExpression newInstanceof= ast.newPatternInstanceofExpression();
368+
newInstanceof.setLeftOperand(ASTNodes.createMoveTarget(rewrite, nodeToComplete.getLeftOperand()));
369+
SingleVariableDeclaration newSVDecl= ast.newSingleVariableDeclaration();
370+
newSVDecl.setName(ast.newSimpleName(replacementName));
371+
newSVDecl.setType(ASTNodes.createMoveTarget(rewrite, nodeToComplete.getRightOperand()));
372+
if ((ast.apiLevel() == AST.JLS20 && ast.isPreviewEnabled()) || ast.apiLevel() > AST.JLS20) {
373+
TypePattern newTypePattern= ast.newTypePattern();
374+
newTypePattern.setPatternVariable((VariableDeclaration) newSVDecl);
375+
newInstanceof.setPattern(newTypePattern);
376+
} else {
377+
newInstanceof.setRightOperand(newSVDecl);
378+
}
379+
ASTNodes.replaceButKeepComment(rewrite, nodeToComplete, newInstanceof, group);
380+
381+
ASTNodes.replaceButKeepComment(rewrite, nodeToComplete, newInstanceof, group);
382+
for (Expression expressionToReplace : expressionsToReplace) {
383+
SimpleName name= ast.newSimpleName(replacementName);
384+
rewrite.replace(expressionToReplace, name, group);
385+
}
386+
}
387+
}
388+
259389
public static class PatternMatchingForInstanceofFixOperation extends CompilationUnitRewriteOperation {
260390
private final InstanceofExpression nodeToComplete;
261391
private final List<VariableDeclarationStatement> statementsToRemove;
@@ -325,7 +455,7 @@ public SourceRange computeSourceRange(final ASTNode nodeWithComment) {
325455

326456

327457
public static ICleanUpFix createCleanUp(final CompilationUnit compilationUnit) {
328-
List<PatternMatchingForInstanceofFixOperation> operations= new ArrayList<>();
458+
List<CompilationUnitRewriteOperation> operations= new ArrayList<>();
329459
PatternMatchingForInstanceofFinder finder= new PatternMatchingForInstanceofFinder(operations);
330460
compilationUnit.accept(finder);
331461

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ public void foo3(Object x, boolean valid) {
591591
System.out.println(s);
592592
}
593593
}
594-
public void foo4(Object o, Object p) {
594+
public int foo4(Object o, Object p) {
595595
if (o instanceof String && p instanceof Integer) {
596596
if (o != p) {
597597
String s = (String) o;
@@ -602,6 +602,20 @@ public void foo4(Object o, Object p) {
602602
Integer i = (Integer) p;
603603
i = 7;
604604
}
605+
return o instanceof String ? ((String)o).length() : i;
606+
}
607+
public int foo5(Object o, Object p) {
608+
if (o instanceof String && p instanceof Integer) {
609+
if (o != p) {
610+
String s = (String) o;
611+
Integer i = (Integer) p;
612+
System.out.println(s.trim() + i.toString());
613+
i = 3;
614+
}
615+
Integer i = (Integer) p;
616+
i = 7;
617+
}
618+
return !(o instanceof String) ? i : ((String)o).length();
605619
}
606620
}
607621
""";
@@ -650,7 +664,18 @@ public void foo3(Object x, boolean valid) {
650664
System.out.println(s);
651665
}
652666
}
653-
public void foo4(Object o, Object p) {
667+
public int foo4(Object o, Object p) {
668+
if (o instanceof String s && p instanceof Integer i) {
669+
if (o != p) {
670+
System.out.println(s.trim() + i.toString());
671+
i = 3;
672+
}
673+
i = (Integer) p;
674+
i = 7;
675+
}
676+
return o instanceof String s2 ? s2.length() : i;
677+
}
678+
public int foo5(Object o, Object p) {
654679
if (o instanceof String s && p instanceof Integer i) {
655680
if (o != p) {
656681
System.out.println(s.trim() + i.toString());
@@ -659,6 +684,7 @@ public void foo4(Object o, Object p) {
659684
i = (Integer) p;
660685
i = 7;
661686
}
687+
return !(o instanceof String s2) ? i : s2.length();
662688
}
663689
}
664690
""";

0 commit comments

Comments
 (0)