Skip to content

Commit e446e46

Browse files
author
Fabrice-TIERCELIN
committed
Handle looping backward
1 parent a983a33 commit e446e46

7 files changed

Lines changed: 129 additions & 25 deletions

File tree

‎plugin/src/main/java/org/autorefactor/jdt/internal/corext/dom/ForLoopHelper.java‎

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -77,26 +77,29 @@ public static final class ForLoopContent {
7777
private Expression iteratorVariable;
7878
private Name loopVariable;
7979
private Name elementVariable;
80+
private boolean isLoopingForward;
8081

8182
private ForLoopContent() {
8283
// Use method factories
8384
}
8485

85-
private static ForLoopContent indexedArray(Expression containerVariable, Name loopVariable) {
86+
private static ForLoopContent indexedArray(Expression containerVariable, Name loopVariable, boolean isLoopingForward) {
8687
final ForLoopContent content= new ForLoopContent();
8788
content.iterationType= IterationType.INDEX;
8889
content.containerType= ContainerType.ARRAY;
8990
content.containerVariable= containerVariable;
9091
content.loopVariable= loopVariable;
92+
content.isLoopingForward= isLoopingForward;
9193
return content;
9294
}
9395

94-
private static ForLoopContent indexedCollection(Expression containerVariable, Name loopVariable) {
96+
private static ForLoopContent indexedCollection(Expression containerVariable, Name loopVariable, boolean isLoopingForward) {
9597
final ForLoopContent content= new ForLoopContent();
9698
content.iterationType= IterationType.INDEX;
9799
content.containerType= ContainerType.COLLECTION;
98100
content.containerVariable= containerVariable;
99101
content.loopVariable= loopVariable;
102+
content.isLoopingForward= isLoopingForward;
100103
return content;
101104
}
102105

@@ -106,6 +109,7 @@ private static ForLoopContent iteratedCollection(Expression containerVariable, E
106109
content.containerType= ContainerType.COLLECTION;
107110
content.containerVariable= containerVariable;
108111
content.iteratorVariable= iteratorVariable;
112+
content.isLoopingForward= true;
109113
return content;
110114
}
111115

@@ -163,11 +167,21 @@ public IterationType getIterationType() {
163167
return iterationType;
164168
}
165169

170+
/**
171+
* Returns true if the loop iterate from the start to the end of the container.
172+
*
173+
* @return true if the loop iterate from the start to the end of the container
174+
*/
175+
public boolean isLoopingForward() {
176+
return isLoopingForward;
177+
}
178+
166179
@Override
167180
public String toString() {
168181
return getClass().getSimpleName() + "(" + "iterationType=" + iterationType + ", containerType=" //$NON-NLS-1$ $NON-NLS-2$ $NON-NLS-3$
169182
+ containerType + ", containerVariable=" + containerVariable + ", iteratorVariable=" //$NON-NLS-1$ $NON-NLS-2$
170183
+ iteratorVariable + ", loopVariable=" + loopVariable + ", elementVariable=" + elementVariable //$NON-NLS-1$ $NON-NLS-2$
184+
+ ", isLoopingForward=" + isLoopingForward //$NON-NLS-1$
171185
+ ")"; //$NON-NLS-1$
172186
}
173187
}
@@ -202,11 +216,25 @@ public static ForLoopContent iterateOverContainer(ForStatement node) {
202216
} else if (updaters.size() == 1 && ASTNodes.isPrimitive(firstInit, int.class.getSimpleName())) {
203217
final Pair<Name, Expression> initPair= decomposeInitializer(firstInit);
204218
final Name init= initPair.getFirst();
205-
final ForLoopContent forContent= getIndexOnIterable(condition, init);
206-
final Name updater= getUpdaterOperand(updaters.get(0));
207-
Long zero= ASTNodes.integerLiteral(initPair.getSecond());
219+
final Expression startValue= initPair.getSecond();
220+
Long zero= ASTNodes.integerLiteral(startValue);
221+
final InfixExpression startValueMinusOne= ASTNodes.as(startValue, InfixExpression.class);
222+
Expression collectionOnSize= null;
223+
Expression arrayOnLength= null;
224+
225+
if (startValueMinusOne != null && !startValueMinusOne.hasExtendedOperands() && ASTNodes.hasOperator(startValueMinusOne, InfixExpression.Operator.MINUS)) {
226+
final Long one= ASTNodes.integerLiteral(startValueMinusOne.getRightOperand());
227+
228+
if (one != null && one == 1) {
229+
collectionOnSize= getCollectionOnSize(startValueMinusOne.getLeftOperand());
230+
arrayOnLength= getArrayOnLength(startValueMinusOne.getLeftOperand());
231+
}
232+
}
208233

209-
if (forContent != null && zero != null && zero == 0 && ASTNodes.isSameVariable(init, forContent.loopVariable)
234+
final ForLoopContent forContent= getIndexOnIterable(condition, init, zero, collectionOnSize, arrayOnLength);
235+
final Name updater= getUpdaterOperand(updaters.get(0), zero != null && zero == 0);
236+
237+
if (forContent != null && ASTNodes.isSameVariable(init, forContent.loopVariable)
210238
&& ASTNodes.isSameVariable(init, updater)) {
211239
return forContent;
212240
}
@@ -224,19 +252,19 @@ private static ForLoopContent getIteratorOnCollection(Expression containerVar, E
224252
return null;
225253
}
226254

227-
private static Name getUpdaterOperand(Expression updater) {
255+
private static Name getUpdaterOperand(Expression updater, boolean isLoopingForward) {
228256
Expression updaterOperand= null;
229257

230258
if (updater instanceof PostfixExpression) {
231259
final PostfixExpression pe= (PostfixExpression) updater;
232260

233-
if (ASTNodes.hasOperator(pe, PostfixExpression.Operator.INCREMENT)) {
261+
if (isLoopingForward ? ASTNodes.hasOperator(pe, PostfixExpression.Operator.INCREMENT) : ASTNodes.hasOperator(pe, PostfixExpression.Operator.DECREMENT)) {
234262
updaterOperand= pe.getOperand();
235263
}
236264
} else if (updater instanceof PrefixExpression) {
237265
final PrefixExpression pe= (PrefixExpression) updater;
238266

239-
if (ASTNodes.hasOperator(pe, PrefixExpression.Operator.INCREMENT)) {
267+
if (isLoopingForward ? ASTNodes.hasOperator(pe, PrefixExpression.Operator.INCREMENT) : ASTNodes.hasOperator(pe, PrefixExpression.Operator.DECREMENT)) {
240268
updaterOperand= pe.getOperand();
241269
}
242270
}
@@ -261,6 +289,7 @@ public static Pair<Name, Expression> decomposeInitializer(Expression init) {
261289
if (init instanceof VariableDeclarationExpression) {
262290
final VariableDeclarationExpression vde= (VariableDeclarationExpression) init;
263291
final List<VariableDeclarationFragment> fragments= ASTNodes.fragments(vde);
292+
264293
if (fragments.size() == 1) {
265294
final VariableDeclarationFragment fragment= fragments.get(0);
266295
return Pair.of((Name) fragment.getName(), fragment.getInitializer());
@@ -276,7 +305,7 @@ public static Pair<Name, Expression> decomposeInitializer(Expression init) {
276305
return Pair.empty();
277306
}
278307

279-
private static ForLoopContent getIndexOnIterable(final Expression condition, Name loopVariable) {
308+
private static ForLoopContent getIndexOnIterable(final Expression condition, Name loopVariable, Long zero, Expression collectionOnSize, Expression arrayOnLength) {
280309
final InfixExpression ie= ASTNodes.as(condition, InfixExpression.class);
281310

282311
if (ie != null && !ie.hasExtendedOperands()) {
@@ -287,27 +316,45 @@ private static ForLoopContent getIndexOnIterable(final Expression condition, Nam
287316
return null;
288317
}
289318

290-
if (ASTNodes.hasOperator(ie, InfixExpression.Operator.LESS) && ASTNodes.isSameLocalVariable(loopVariable, leftOp)) {
291-
return buildForLoopContent((Name) loopVariable, rightOp);
292-
} else if (ASTNodes.hasOperator(ie, InfixExpression.Operator.GREATER) && ASTNodes.isSameLocalVariable(loopVariable, rightOp)) {
293-
return buildForLoopContent((Name) loopVariable, leftOp);
319+
if (zero != null && zero == 0) {
320+
if (ASTNodes.hasOperator(ie, InfixExpression.Operator.LESS) && ASTNodes.isSameLocalVariable(loopVariable, leftOp)) {
321+
return buildForLoopContent((Name) loopVariable, rightOp, zero, collectionOnSize, arrayOnLength);
322+
} else if (ASTNodes.hasOperator(ie, InfixExpression.Operator.GREATER) && ASTNodes.isSameLocalVariable(loopVariable, rightOp)) {
323+
return buildForLoopContent((Name) loopVariable, leftOp, zero, collectionOnSize, arrayOnLength);
324+
}
325+
} else if (collectionOnSize != null || arrayOnLength != null) {
326+
if (ASTNodes.hasOperator(ie, InfixExpression.Operator.GREATER_EQUALS) && ASTNodes.isSameLocalVariable(loopVariable, leftOp)) {
327+
return buildForLoopContent((Name) loopVariable, rightOp, zero, collectionOnSize, arrayOnLength);
328+
} else if (ASTNodes.hasOperator(ie, InfixExpression.Operator.LESS_EQUALS) && ASTNodes.isSameLocalVariable(loopVariable, rightOp)) {
329+
return buildForLoopContent((Name) loopVariable, leftOp, zero, collectionOnSize, arrayOnLength);
330+
}
294331
}
295332
}
296333

297334
return null;
298335
}
299336

300-
private static ForLoopContent buildForLoopContent(final Name loopVar, final Expression containerVar) {
301-
Expression collectionOnSize= getCollectionOnSize(containerVar);
337+
private static ForLoopContent buildForLoopContent(final Name loopVar, final Expression containerVar, Long zero, Expression collectionOnSize, Expression arrayOnLength) {
338+
Long zero2= ASTNodes.integerLiteral(containerVar);
339+
Expression collectionOnSize2= getCollectionOnSize(containerVar);
340+
Expression arrayOnLength2= getArrayOnLength(containerVar);
302341

303-
if (collectionOnSize != null) {
304-
return ForLoopContent.indexedCollection(collectionOnSize, loopVar);
305-
}
342+
if (zero != null && zero == 0) {
343+
if (collectionOnSize2 != null) {
344+
return ForLoopContent.indexedCollection(collectionOnSize2, loopVar, true);
345+
}
306346

307-
Expression arrayOnLength= getArrayOnLength(containerVar);
347+
if (arrayOnLength2 != null) {
348+
return ForLoopContent.indexedArray(arrayOnLength2, loopVar, true);
349+
}
350+
} else if (zero2 != null && zero2 == 0) {
351+
if (collectionOnSize != null) {
352+
return ForLoopContent.indexedCollection(collectionOnSize, loopVar, false);
353+
}
308354

309-
if (arrayOnLength != null) {
310-
return ForLoopContent.indexedArray(arrayOnLength, loopVar);
355+
if (arrayOnLength != null) {
356+
return ForLoopContent.indexedArray(arrayOnLength, loopVar, false);
357+
}
311358
}
312359

313360
return null;

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_in/ContainsAllRatherThanLoopSample.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,17 @@ public boolean replaceForCounterInvertedCondition(List<Long> collectionToAnalyze
236236
return true;
237237
}
238238

239+
public boolean replaceBackwardLoopOnCollection(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
240+
// Keep this comment
241+
for (int i = dataToSearch.size() - 1; i >= 0; i--) {
242+
Long number = dataToSearch.get(i);
243+
if (!collectionToAnalyze.contains(number)) {
244+
return false;
245+
}
246+
}
247+
return true;
248+
}
249+
239250
public boolean replaceForCounterPrefixedUpdater(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
240251
// Keep this comment
241252
for (int i = 0; i < dataToSearch.size(); ++i) {

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_in/ContainsRatherThanLoopSample.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,16 @@ public boolean replaceForCounterNoLoopVariableInvertedEquals(List<String> col, S
254254
return false;
255255
}
256256

257+
public boolean replaceBackwardLoopOnCollection(List<String> col, String toFind) {
258+
// Keep this comment
259+
for (int i = col.size() - 1; i >= 0 ; --i) {
260+
if (toFind.equals(col.get(i))) {
261+
return true;
262+
}
263+
}
264+
return false;
265+
}
266+
257267
public boolean replaceForIterator(List<String> col, String toFind) {
258268
// Keep this comment
259269
for (Iterator<String> it = col.iterator(); it.hasNext();) {

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_in/FillRatherThanLoopSample.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,21 @@ public String[] refactorStringArray() {
8888
return array;
8989
}
9090

91+
public String[] refactorBackwardLoopOnArrary() {
92+
String[] array = new String[10];
93+
94+
// Keep this comment
95+
for (int i = array.length - 1; i >= 0; i--) {
96+
array[i] = "foo";
97+
}
98+
// Keep this comment too
99+
for (int i = array.length - 1; 0 <= i; --i) {
100+
array[i] = "foo";
101+
}
102+
103+
return array;
104+
}
105+
91106
public void refactorExternalArray() {
92107
// Keep this comment
93108
for (int i = 0; i < booleanArray.length; i++) {

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_out/ContainsAllRatherThanLoopSample.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,14 @@ public boolean replaceForeachPreviousStatementBeforeIf(List<Long> collectionToAn
100100
}
101101

102102
public boolean replaceForeachHoldResultInVariableThenBreak(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
103-
103+
104104
// Keep this comment
105105
boolean result = collectionToAnalyze.containsAll(dataToSearch);
106106
return result;
107107
}
108108

109109
public boolean replaceForeachHoldResultInVariableNoBreak(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
110-
110+
111111
// Keep this comment
112112
boolean result = collectionToAnalyze.containsAll(dataToSearch);
113113
return result;
@@ -117,7 +117,7 @@ public boolean replaceForeachHoldResultInVariableCannotRemoveVariable(List<Long>
117117
// Keep this comment
118118
boolean result = true;
119119
;
120-
120+
121121
// Keep this comment too
122122
result = collectionToAnalyze.containsAll(dataToSearch);
123123
return result;
@@ -163,6 +163,11 @@ public boolean replaceForCounterInvertedCondition(List<Long> collectionToAnalyze
163163
return collectionToAnalyze.containsAll(dataToSearch);
164164
}
165165

166+
public boolean replaceBackwardLoopOnCollection(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
167+
// Keep this comment
168+
return collectionToAnalyze.containsAll(dataToSearch);
169+
}
170+
166171
public boolean replaceForCounterPrefixedUpdater(List<Long> collectionToAnalyze, List<Long> dataToSearch) {
167172
// Keep this comment
168173
return collectionToAnalyze.containsAll(dataToSearch);

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_out/ContainsRatherThanLoopSample.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ public boolean replaceForCounterNoLoopVariableInvertedEquals(List<String> col, S
168168
return col.contains(toFind);
169169
}
170170

171+
public boolean replaceBackwardLoopOnCollection(List<String> col, String toFind) {
172+
// Keep this comment
173+
return col.contains(toFind);
174+
}
175+
171176
public boolean replaceForIterator(List<String> col, String toFind) {
172177
// Keep this comment
173178
return col.contains(toFind);

‎samples/src/test/java/org/autorefactor/jdt/internal/ui/fix/samples_out/FillRatherThanLoopSample.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,17 @@ public String[] refactorStringArray() {
7474
return array;
7575
}
7676

77+
public String[] refactorBackwardLoopOnArrary() {
78+
String[] array = new String[10];
79+
80+
// Keep this comment
81+
Arrays.fill(array, "foo");
82+
// Keep this comment too
83+
Arrays.fill(array, "foo");
84+
85+
return array;
86+
}
87+
7788
public void refactorExternalArray() {
7889
// Keep this comment
7990
Arrays.fill(booleanArray, true);

0 commit comments

Comments
 (0)