diff --git a/javaparser-core-testing/src/test/java/com/github/javaparser/ast/NodeListTest.java b/javaparser-core-testing/src/test/java/com/github/javaparser/ast/NodeListTest.java index a28c3970a4..55d8004933 100644 --- a/javaparser-core-testing/src/test/java/com/github/javaparser/ast/NodeListTest.java +++ b/javaparser-core-testing/src/test/java/com/github/javaparser/ast/NodeListTest.java @@ -21,9 +21,25 @@ package com.github.javaparser.ast; +import com.github.javaparser.JavaParser; +import com.github.javaparser.ast.expr.ArrayInitializerExpr; +import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.MemberValuePair; import com.github.javaparser.ast.expr.Name; +import com.github.javaparser.ast.expr.NormalAnnotationExpr; +import com.github.javaparser.ast.expr.StringLiteralExpr; +import com.github.javaparser.ast.observer.AstObserver; +import com.github.javaparser.ast.observer.ObservableProperty; +import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.NoSuchElementException; import java.util.Optional; import static com.github.javaparser.ast.NodeList.nodeList; @@ -136,6 +152,7 @@ public void getFirstWhenNonEmpty() { assertTrue(first.isPresent()); assertEquals("Optional[abc]", first.toString()); } + @Test public void getLastWhenEmpty() { final NodeList list = nodeList(); @@ -155,4 +172,275 @@ public void getLastWhenNonEmpty() { assertTrue(last.isPresent()); assertEquals("Optional[cde]", last.toString()); } + + @Nested + class IteratorTest { + + @Nested + class ObserversTest { + NodeList list; + ListIterator iterator; + + List propertyChanges; + List parentChanges; + List listChanges; + List listReplacements; + AstObserver testObserver = new AstObserver() { + @Override + public void propertyChange(Node observedNode, ObservableProperty property, Object oldValue, Object newValue) { + propertyChanges.add(String.format("%s.%s changed from %s to %s", observedNode.getClass().getSimpleName(), property.name().toLowerCase(), oldValue, newValue)); + } + + @Override + public void parentChange(Node observedNode, Node previousParent, Node newParent) { + parentChanges.add(String.format("%s 's parent changed from %s to %s", observedNode.getClass().getSimpleName(), previousParent, newParent)); + } + + @Override + public void listChange(NodeList observedNode, ListChangeType type, int index, Node nodeAddedOrRemoved) { + listChanges.add(String.format("%s %s to/from %s at position %d", nodeAddedOrRemoved.getClass().getSimpleName(), type.name(), observedNode.getClass().getSimpleName(), index)); + } + + @Override + public void listReplacement(NodeList observedNode, int index, Node oldNode, Node newNode) { + listReplacements.add(String.format("%s replaced within %s at position %d", newNode.getClass().getSimpleName(), observedNode.getClass().getSimpleName(), index)); + } + + }; + + @BeforeEach + void pre() { + list = nodeList(); + list.register(testObserver); + iterator = list.listIterator(); + + propertyChanges = new ArrayList<>(); + parentChanges = new ArrayList<>(); + listChanges = new ArrayList<>(); + listReplacements = new ArrayList<>(); + } + + @Test + void whenAdd() { + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(0, listChanges.size()); + assertEquals(0, listReplacements.size()); + + iterator.add(new Name("abc")); + + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(1, listChanges.size()); + assertEquals(0, listReplacements.size()); + + assertEquals("Name ADDITION to/from NodeList at position 0", listChanges.get(0)); + } + + @Test + void whenRemove() { + iterator.add(new Name("abc")); + + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(1, listChanges.size()); + assertEquals(0, listReplacements.size()); + + iterator.previous(); + iterator.remove(); + + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(2, listChanges.size()); + assertEquals(0, listReplacements.size()); + + assertEquals("Name ADDITION to/from NodeList at position 0", listChanges.get(0)); + assertEquals("Name REMOVAL to/from NodeList at position 0", listChanges.get(1)); + } + + @Test + void whenSet() { + iterator.add(new Name("abc")); + + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(1, listChanges.size()); + assertEquals(0, listReplacements.size()); + + iterator.previous(); + iterator.set(new Name("xyz")); + + assertEquals(0, propertyChanges.size()); + assertEquals(0, parentChanges.size()); + assertEquals(1, listChanges.size()); + assertEquals(1, listReplacements.size()); + + assertEquals("Name ADDITION to/from NodeList at position 0", listChanges.get(0)); + assertEquals("Name replaced within NodeList at position 0", listReplacements.get(0)); + } + + + @Test + void usageTest() { + final String REFERENCE_TO_BE_DELETED = "bad"; + String original = "" + + "@MyAnnotation(myElements = {\"good\", \"bad\", \"ugly\"})\n" + + "public final class MyClass {\n" + + "}"; + String expected = "" + + "@MyAnnotation(myElements = {\"good\", \"ugly\"})\n" + + "public final class MyClass {\n" + + "}"; + + JavaParser javaParser = new JavaParser(); + javaParser.getParserConfiguration().setLexicalPreservationEnabled(true); + + CompilationUnit compilationUnit = javaParser.parse(original).getResult().get(); + List annotations = compilationUnit.findAll(NormalAnnotationExpr.class); + + annotations.forEach(annotation -> { + // testcase, per https://github.com/javaparser/javaparser/issues/2936#issuecomment-731370505 + MemberValuePair mvp = annotation.getPairs().get(0); + Expression value = mvp.getValue(); + if ((value instanceof ArrayInitializerExpr)) { + NodeList myElements = ((ArrayInitializerExpr) value).getValues(); + + for (Iterator iterator = myElements.iterator(); iterator.hasNext(); ) { + Node elt = iterator.next(); + { + String nameAsString = ((StringLiteralExpr) elt).asString(); + if (REFERENCE_TO_BE_DELETED.equals(nameAsString)) + iterator.remove(); + } + } + } + }); + + assertEquals(expected, LexicalPreservingPrinter.print(compilationUnit)); + } + } + + @Nested + class AddRemoveListIteratorTest { + NodeList list; + ListIterator iterator; + + @BeforeEach + void pre() { + list = nodeList(); + iterator = list.listIterator(); + } + + @Test + void whenAdd() { + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasPrevious()); + // Note that the element is added before the current cursor, thus is accessible via "previous" + iterator.add(new Name("abc")); + assertFalse(iterator.hasNext()); + assertTrue(iterator.hasPrevious()); + } + + } + + @Nested + class EmptyIteratorTest { + NodeList list; + ListIterator iterator; + + @BeforeEach + void pre() { + list = nodeList(); + iterator = list.listIterator(); + } + + @Test + void whenNext() { + assertThrows(NoSuchElementException.class, () -> { + iterator.next(); + }); + } + + @Test + void whenHasNext() { + assertFalse(iterator.hasNext()); + } + + @Test + void whenAdd() { + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasPrevious()); + // Note that the element is added before the current cursor, thus is accessible via "previous" + iterator.add(new Name("abc")); + assertFalse(iterator.hasNext()); + assertTrue(iterator.hasPrevious()); + } + + @Test + void whenSet() { + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasPrevious()); + assertThrows(IllegalArgumentException.class, () -> { + // Note that the cursor is initially at -1, thus not possible to set the value here + iterator.set(new Name("abc")); + }); + // Assert that next/previous are still empty + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasPrevious()); + } + + } + + @Nested + class SingleItemIteratorTest { + NodeList list; + Iterator iterator; + + @BeforeEach + void pre() { + list = nodeList(new Name("abc")); + iterator = list.iterator(); + } + + @Test + void whenNext() { + Name next = iterator.next(); + assertNotNull(next); + } + + @Test + void whenHasNext() { + assertTrue(iterator.hasNext()); + } + + @Test + void whenHasNextRepeated() { + assertTrue(iterator.hasNext()); + assertTrue(iterator.hasNext()); + assertTrue(iterator.hasNext()); + assertTrue(iterator.hasNext()); + } + + @Test + void whenHasNextThenNext() { + assertTrue(iterator.hasNext()); + iterator.next(); + assertFalse(iterator.hasNext()); + assertThrows(NoSuchElementException.class, () -> { + iterator.next(); + }); + } + + @Test + void whenRemove() { + Name current = iterator.next(); + iterator.remove(); + assertFalse(iterator.hasNext()); + assertThrows(NoSuchElementException.class, () -> { + iterator.next(); + }); + } + + } + } } diff --git a/javaparser-core/src/main/java/com/github/javaparser/ast/NodeList.java b/javaparser-core/src/main/java/com/github/javaparser/ast/NodeList.java index e06be0730f..2d7584b95b 100644 --- a/javaparser-core/src/main/java/com/github/javaparser/ast/NodeList.java +++ b/javaparser-core/src/main/java/com/github/javaparser/ast/NodeList.java @@ -48,11 +48,11 @@ */ public class NodeList implements List, Iterable, HasParentNode>, Visitable, Observable { @InternalProperty - private List innerList = new ArrayList<>(0); + private final List innerList = new ArrayList<>(0); private Node parentNode; - private List observers = new ArrayList<>(); + private final List observers = new ArrayList<>(); public NodeList() { parentNode = null; @@ -133,8 +133,8 @@ public N get(int i) { @Override public Iterator iterator() { - // TODO take care of "Iterator.remove" - return innerList.iterator(); + // Custom iterator required, to ensure that the relevant `notifyElement...` methods are called. + return new NodeListIterator(innerList); } @Override @@ -449,7 +449,8 @@ public int lastIndexOf(Object o) { */ @Override public ListIterator listIterator() { - return innerList.listIterator(); + // Custom iterator required, to ensure that the relevant `notifyElement...` methods are called. + return new NodeListIterator(innerList); } /** @@ -457,7 +458,8 @@ public ListIterator listIterator() { */ @Override public ListIterator listIterator(int index) { - return innerList.listIterator(index); + // Custom iterator required, to ensure that the relevant `notifyElement...` methods are called. + return new NodeListIterator(innerList, index); } /** @@ -564,4 +566,89 @@ private void setAsParentNodeOf(Node childNode) { public String toString() { return innerList.stream().map(Node::toString).collect(Collectors.joining(", ", "[", "]")); } + + protected class NodeListIterator implements ListIterator{ + + ListIterator iterator; + N current = null; + + // initialize pointer to head of the list for iteration + public NodeListIterator(List list) { + iterator = list.listIterator(); + } + + public NodeListIterator(List list, int index) { + iterator = list.listIterator(index); + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public N next() { + current = iterator.next(); + return current; + } + + @Override + public boolean hasPrevious() { + return iterator.hasPrevious(); + } + + @Override + public N previous() { + current = iterator.previous(); + return current; + } + + @Override + public int nextIndex() { + return iterator.nextIndex(); + } + + @Override + public int previousIndex() { + return iterator.previousIndex(); + } + + @Override + public void remove() { + int index = innerList.indexOf(current); + if (index != -1) { + notifyElementRemoved(index, current); + current.setParentNode(null); + } + iterator.remove(); + } + + @Override + public void set(N n) { + int index = innerList.indexOf(current); + if (index < 0 || index >= innerList.size()) { + throw new IllegalArgumentException("Illegal index. The index should be between 0 and " + innerList.size() + + " excluded. It is instead " + index); + } + if (n != innerList.get(index)) { + notifyElementReplaced(index, n); + innerList.get(index).setParentNode(null); + setAsParentNodeOf(n); + + iterator.set(n); + } + } + + @Override + public void add(N n) { + notifyElementAdded(innerList.size(), n); + own(n); + iterator.add(n); + } + + @Override + public void forEachRemaining(Consumer action) { + iterator.forEachRemaining(action); + } + } } diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Difference.java b/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Difference.java index 08b5a4ee11..7054309b23 100644 --- a/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Difference.java +++ b/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Difference.java @@ -476,9 +476,9 @@ private void cleanTheLineOfLeftOverSpace(RemovedGroup removedGroup, Removed remo } } - // note: + // note: // increment originalIndex if we want to keep the original element - // increment diffIndex if we don't want to skip the diff element + // increment diffIndex if we don't want to skip the diff element private void applyKeptDiffElement(Kept kept, TextElement originalElement, boolean originalElementIsChild, boolean originalElementIsToken) { if (originalElement.isComment()) { originalIndex++; @@ -525,15 +525,15 @@ private void applyKeptDiffElement(Kept kept, TextElement originalElement, boolea } else if (kept.isNewLine() && originalTextToken.isSpaceOrTab()) { originalIndex++; diffIndex++; - // case where originalTextToken is a separator like ";" and - // kept is not a new line or whitespace for example "}" - // see issue 2351 - } else if (!kept.isNewLine() && originalTextToken.isSeparator()) { - originalIndex++; } else if (kept.isWhiteSpaceOrComment()) { diffIndex++; } else if (originalTextToken.isWhiteSpaceOrComment()) { originalIndex++; + } else if (!kept.isNewLine() && originalTextToken.isSeparator()) { + // case where originalTextToken is a separator like ";" and + // kept is not a new line or whitespace for example "}" + // see issue 2351 + originalIndex++; } else { throw new UnsupportedOperationException("Csm token " + kept.getElement() + " NodeText TOKEN " + originalTextToken); } @@ -579,12 +579,12 @@ private boolean isNodeWithTypeArguments(DifferenceElement element) { * For example, * List is represented by 4 tokens ([List][<][String][>]) while it's a CsmChild element in the DiffElements list * So in this case, getIndexToNextTokenElement(..) on the [List] token returns 3 because we have to skip 3 tokens ([<][String][>]) to synchronize - * DiffElements list and originalElements list + * DiffElements list and originalElements list * The end of recursivity is reached when there is no next token or if the nested diamond operators are totally managed, to take into account this type of declaration * List > l * Be careful, this method must be call only if diamond operator could be found in the sequence - * - * @Param TokenTextElement the token currently analyzed + * + * @Param TokenTextElement the token currently analyzed * @Param int the number of nested diamond operators * @return the number of token to skip in originalElements list */ @@ -592,7 +592,7 @@ private int getIndexToNextTokenElement(TokenTextElement element, int nestedDiamo int step = 0; // number of token to skip Optional next = element.getToken().getNextToken(); if (!next.isPresent()) return step; - // because there is a token, first we need to increment the number of token to skip + // because there is a token, first we need to increment the number of token to skip step++; // manage nested diamond operators by incrementing the level on LT token and decrementing on GT JavaToken token = next.get(); @@ -611,7 +611,7 @@ private int getIndexToNextTokenElement(TokenTextElement element, int nestedDiamo // recursively analyze token to skip return step += getIndexToNextTokenElement(new TokenTextElement(token), nestedDiamondOperator); } - + /* * Returns true if the token is possibly a diamond operator */