From 9733af39f63d1be71c2f47171adcd33dec76f461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Mon, 4 May 2020 10:00:24 +0200 Subject: [PATCH 01/11] Add failing test case for retention of super interface order --- .../java/spoon/test/ctType/CtTypeTest.java | 24 +++++++++++++++++++ .../MultiInterfaceImplementation.java | 20 ++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 src/test/java/spoon/test/ctType/testclasses/MultiInterfaceImplementation.java diff --git a/src/test/java/spoon/test/ctType/CtTypeTest.java b/src/test/java/spoon/test/ctType/CtTypeTest.java index ba8a71db92c..20b1ac5d5ab 100644 --- a/src/test/java/spoon/test/ctType/CtTypeTest.java +++ b/src/test/java/spoon/test/ctType/CtTypeTest.java @@ -18,11 +18,13 @@ import org.junit.Test; import spoon.Launcher; +import spoon.reflect.CtModel; import spoon.reflect.code.CtAssignment; import spoon.reflect.code.CtComment; import spoon.reflect.code.CtLocalVariable; import spoon.reflect.code.CtVariableAccess; import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtNamedElement; @@ -35,12 +37,16 @@ import spoon.reflect.visitor.filter.TypeFilter; import spoon.test.ctType.testclasses.X; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static spoon.testing.utils.ModelUtils.buildClass; @@ -213,4 +219,22 @@ private String getTypeName(CtTypeReference ref) { } return ref.toString() + " " + name; } + + @Test + public void testRetainsInterfaceOrder() { + final Launcher launcher = new Launcher(); + List expectedInterfaceOrder = Arrays.asList( + "java.util.function.Supplier", + "java.util.function.Consumer", + "java.lang.Comparable" + ); + launcher.addInputResource("./src/test/java/spoon/test/ctType/testclasses/MultiInterfaceImplementation.java"); + + CtModel model = launcher.buildModel(); + CtType type = model.getAllTypes().iterator().next(); + List interfaces = type.getSuperInterfaces() + .stream().map(CtElement::toString).collect(Collectors.toList()); + + assertEquals(expectedInterfaceOrder, interfaces); + } } diff --git a/src/test/java/spoon/test/ctType/testclasses/MultiInterfaceImplementation.java b/src/test/java/spoon/test/ctType/testclasses/MultiInterfaceImplementation.java new file mode 100644 index 00000000000..44dbe405d5f --- /dev/null +++ b/src/test/java/spoon/test/ctType/testclasses/MultiInterfaceImplementation.java @@ -0,0 +1,20 @@ +package spoon.test.ctType.testclasses; + +public class MultiInterfaceImplementation implements + java.util.function.Supplier, + java.util.function.Consumer, + java.lang.Comparable { + public int compareTo(Integer i) { + return 0; + } + + public Integer get() { + return 1; + } + + public void accept(Integer i) { + // thanks! + } +} + + From bebf1adaa7af4aaed4401aa72778dc4eac420f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Mon, 4 May 2020 13:05:17 +0200 Subject: [PATCH 02/11] Attempt to solve unnecessary sorting by qualified name --- .../util/QualifiedNameBasedSortedSet.java | 127 +++++++++++++++++- .../support/visitor/equals/CloneHelper.java | 4 +- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index cde4411ff52..16e66b91f5f 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -7,14 +7,24 @@ */ package spoon.support.util; +import java.io.Serializable; import java.util.Collection; -import java.util.TreeSet; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import spoon.Launcher; import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.declaration.CtTypeInformation; +import spoon.reflect.reference.CtReference; import spoon.support.comparator.QualifiedNameComparator; -public class QualifiedNameBasedSortedSet extends - TreeSet { +public class QualifiedNameBasedSortedSet implements Set, Serializable { + private final LinkedHashSet set; private static final long serialVersionUID = 1L; @@ -24,7 +34,116 @@ public QualifiedNameBasedSortedSet(Collection elements) { } public QualifiedNameBasedSortedSet() { - super(new QualifiedNameComparator()); + set = new LinkedHashSet<>(); + } + + @Override + public int size() { + return set.size(); + } + + @Override + public boolean isEmpty() { + return set.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return o instanceof CtElement && set.contains(new QualifiedNameHashEqualsWrapper((CtElement) o)); + } + + @Override + @SuppressWarnings("unchecked") + public Iterator iterator() { + return set.stream().map(e -> (E) e.element).iterator(); + } + + @Override + public Object[] toArray() { + return set.stream().map(e -> e.element).toArray(); + } + + @Override + public T[] toArray(T[] ts) { + return set.stream().map(e -> e.element).collect(Collectors.toList()).toArray(ts); + } + + @Override + public boolean add(E e) { + return set.add(new QualifiedNameHashEqualsWrapper(e)); + } + + @Override + public boolean remove(Object o) { + return o instanceof CtElement && + set.removeIf(e -> getQualifiedName(e.element).equals(getQualifiedName((CtElement) o))); + } + + @Override + public boolean containsAll(Collection collection) { + return collection.stream().allMatch(this::contains); + } + + @Override + public boolean addAll(Collection collection) { + return collection.stream().anyMatch(this::add); + } + + @Override + public boolean retainAll(Collection collection) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean removeAll(Collection collection) { + throw new UnsupportedOperationException(); + } + + @Override + public void clear() { + set.clear(); + } + + /** + * A small wrapper around a CtElement that provides hashCode and equals methods based on the element's qualified + * name. + */ + private static final class QualifiedNameHashEqualsWrapper implements Serializable { + final CtElement element; + + private static final long serialVersionUID = 1L; + + QualifiedNameHashEqualsWrapper(CtElement element) { + this.element = element; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + QualifiedNameHashEqualsWrapper that = (QualifiedNameHashEqualsWrapper) o; + return QualifiedNameComparator.INSTANCE.compare(element, that.element) == 0; + } + + @Override + public int hashCode() { + return Objects.hash(getQualifiedName(element)); + } + } + + private static String getQualifiedName(CtElement element) { + if (element instanceof CtTypeInformation) { + return ((CtTypeInformation) element).getQualifiedName(); + } else if (element instanceof CtPackage) { + return ((CtPackage) element).getQualifiedName(); + } else if (element instanceof CtReference) { + return ((CtReference) element).getSimpleName(); + } else if (element instanceof CtNamedElement) { + return ((CtNamedElement) element).getSimpleName(); + } + + Launcher.LOGGER.warn( QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); + return ""; } } diff --git a/src/main/java/spoon/support/visitor/equals/CloneHelper.java b/src/main/java/spoon/support/visitor/equals/CloneHelper.java index 2e178e95ec3..92a621148f3 100644 --- a/src/main/java/spoon/support/visitor/equals/CloneHelper.java +++ b/src/main/java/spoon/support/visitor/equals/CloneHelper.java @@ -17,7 +17,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -78,7 +78,7 @@ public Set clone(Set elements) { if (elements == null || elements.isEmpty()) { return EmptyClearableSet.instance(); } - Set others = new HashSet<>(elements.size()); + Set others = new LinkedHashSet<>(elements.size()); for (T element : elements) { addClone(others, element); } From dc63326fc17a88309bb7f35914a76093d2928ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Mon, 4 May 2020 13:37:07 +0200 Subject: [PATCH 03/11] Fix indentation --- .../util/QualifiedNameBasedSortedSet.java | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index 16e66b91f5f..a56891d77a5 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -24,17 +24,32 @@ import spoon.support.comparator.QualifiedNameComparator; public class QualifiedNameBasedSortedSet implements Set, Serializable { - private final LinkedHashSet set; - private static final long serialVersionUID = 1L; + private final LinkedHashSet set; + public QualifiedNameBasedSortedSet(Collection elements) { this(); addAll(elements); } public QualifiedNameBasedSortedSet() { - set = new LinkedHashSet<>(); + set = new LinkedHashSet<>(); + } + + private static String getQualifiedName(CtElement element) { + if (element instanceof CtTypeInformation) { + return ((CtTypeInformation) element).getQualifiedName(); + } else if (element instanceof CtPackage) { + return ((CtPackage) element).getQualifiedName(); + } else if (element instanceof CtReference) { + return ((CtReference) element).getSimpleName(); + } else if (element instanceof CtNamedElement) { + return ((CtNamedElement) element).getSimpleName(); + } + + Launcher.LOGGER.warn(QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); + return ""; } @Override @@ -53,7 +68,7 @@ public boolean contains(Object o) { } @Override - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") public Iterator iterator() { return set.stream().map(e -> (E) e.element).iterator(); } @@ -81,7 +96,7 @@ public boolean remove(Object o) { @Override public boolean containsAll(Collection collection) { - return collection.stream().allMatch(this::contains); + return collection.stream().allMatch(this::contains); } @Override @@ -91,12 +106,12 @@ public boolean addAll(Collection collection) { @Override public boolean retainAll(Collection collection) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException(); } @Override public boolean removeAll(Collection collection) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException(); } @Override @@ -109,9 +124,8 @@ public void clear() { * name. */ private static final class QualifiedNameHashEqualsWrapper implements Serializable { - final CtElement element; - private static final long serialVersionUID = 1L; + final CtElement element; QualifiedNameHashEqualsWrapper(CtElement element) { this.element = element; @@ -131,19 +145,4 @@ public int hashCode() { } } - private static String getQualifiedName(CtElement element) { - if (element instanceof CtTypeInformation) { - return ((CtTypeInformation) element).getQualifiedName(); - } else if (element instanceof CtPackage) { - return ((CtPackage) element).getQualifiedName(); - } else if (element instanceof CtReference) { - return ((CtReference) element).getSimpleName(); - } else if (element instanceof CtNamedElement) { - return ((CtNamedElement) element).getSimpleName(); - } - - Launcher.LOGGER.warn( QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); - return ""; - } - } From dcecbf46808ed782634019d6997c8e61dffceba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Tue, 5 May 2020 08:59:35 +0200 Subject: [PATCH 04/11] Revert "Fix indentation" This reverts commit dc63326fc17a88309bb7f35914a76093d2928ae8. --- .../util/QualifiedNameBasedSortedSet.java | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index a56891d77a5..16e66b91f5f 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -24,9 +24,9 @@ import spoon.support.comparator.QualifiedNameComparator; public class QualifiedNameBasedSortedSet implements Set, Serializable { - private static final long serialVersionUID = 1L; + private final LinkedHashSet set; - private final LinkedHashSet set; + private static final long serialVersionUID = 1L; public QualifiedNameBasedSortedSet(Collection elements) { this(); @@ -34,22 +34,7 @@ public QualifiedNameBasedSortedSet(Collection elements) { } public QualifiedNameBasedSortedSet() { - set = new LinkedHashSet<>(); - } - - private static String getQualifiedName(CtElement element) { - if (element instanceof CtTypeInformation) { - return ((CtTypeInformation) element).getQualifiedName(); - } else if (element instanceof CtPackage) { - return ((CtPackage) element).getQualifiedName(); - } else if (element instanceof CtReference) { - return ((CtReference) element).getSimpleName(); - } else if (element instanceof CtNamedElement) { - return ((CtNamedElement) element).getSimpleName(); - } - - Launcher.LOGGER.warn(QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); - return ""; + set = new LinkedHashSet<>(); } @Override @@ -68,7 +53,7 @@ public boolean contains(Object o) { } @Override - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") public Iterator iterator() { return set.stream().map(e -> (E) e.element).iterator(); } @@ -96,7 +81,7 @@ public boolean remove(Object o) { @Override public boolean containsAll(Collection collection) { - return collection.stream().allMatch(this::contains); + return collection.stream().allMatch(this::contains); } @Override @@ -106,12 +91,12 @@ public boolean addAll(Collection collection) { @Override public boolean retainAll(Collection collection) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException(); } @Override public boolean removeAll(Collection collection) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException(); } @Override @@ -124,9 +109,10 @@ public void clear() { * name. */ private static final class QualifiedNameHashEqualsWrapper implements Serializable { - private static final long serialVersionUID = 1L; final CtElement element; + private static final long serialVersionUID = 1L; + QualifiedNameHashEqualsWrapper(CtElement element) { this.element = element; } @@ -145,4 +131,19 @@ public int hashCode() { } } + private static String getQualifiedName(CtElement element) { + if (element instanceof CtTypeInformation) { + return ((CtTypeInformation) element).getQualifiedName(); + } else if (element instanceof CtPackage) { + return ((CtPackage) element).getQualifiedName(); + } else if (element instanceof CtReference) { + return ((CtReference) element).getSimpleName(); + } else if (element instanceof CtNamedElement) { + return ((CtNamedElement) element).getSimpleName(); + } + + Launcher.LOGGER.warn( QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); + return ""; + } + } From f9800a346f034fea600a65ef7d56f998455d328f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Tue, 5 May 2020 08:59:43 +0200 Subject: [PATCH 05/11] Revert "Attempt to solve unnecessary sorting by qualified name" This reverts commit bebf1adaa7af4aaed4401aa72778dc4eac420f91. --- .../util/QualifiedNameBasedSortedSet.java | 127 +----------------- .../support/visitor/equals/CloneHelper.java | 4 +- 2 files changed, 6 insertions(+), 125 deletions(-) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index 16e66b91f5f..cde4411ff52 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -7,24 +7,14 @@ */ package spoon.support.util; -import java.io.Serializable; import java.util.Collection; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; +import java.util.TreeSet; -import spoon.Launcher; import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtNamedElement; -import spoon.reflect.declaration.CtPackage; -import spoon.reflect.declaration.CtTypeInformation; -import spoon.reflect.reference.CtReference; import spoon.support.comparator.QualifiedNameComparator; -public class QualifiedNameBasedSortedSet implements Set, Serializable { - private final LinkedHashSet set; +public class QualifiedNameBasedSortedSet extends + TreeSet { private static final long serialVersionUID = 1L; @@ -34,116 +24,7 @@ public QualifiedNameBasedSortedSet(Collection elements) { } public QualifiedNameBasedSortedSet() { - set = new LinkedHashSet<>(); - } - - @Override - public int size() { - return set.size(); - } - - @Override - public boolean isEmpty() { - return set.isEmpty(); - } - - @Override - public boolean contains(Object o) { - return o instanceof CtElement && set.contains(new QualifiedNameHashEqualsWrapper((CtElement) o)); - } - - @Override - @SuppressWarnings("unchecked") - public Iterator iterator() { - return set.stream().map(e -> (E) e.element).iterator(); - } - - @Override - public Object[] toArray() { - return set.stream().map(e -> e.element).toArray(); - } - - @Override - public T[] toArray(T[] ts) { - return set.stream().map(e -> e.element).collect(Collectors.toList()).toArray(ts); - } - - @Override - public boolean add(E e) { - return set.add(new QualifiedNameHashEqualsWrapper(e)); - } - - @Override - public boolean remove(Object o) { - return o instanceof CtElement && - set.removeIf(e -> getQualifiedName(e.element).equals(getQualifiedName((CtElement) o))); - } - - @Override - public boolean containsAll(Collection collection) { - return collection.stream().allMatch(this::contains); - } - - @Override - public boolean addAll(Collection collection) { - return collection.stream().anyMatch(this::add); - } - - @Override - public boolean retainAll(Collection collection) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean removeAll(Collection collection) { - throw new UnsupportedOperationException(); - } - - @Override - public void clear() { - set.clear(); - } - - /** - * A small wrapper around a CtElement that provides hashCode and equals methods based on the element's qualified - * name. - */ - private static final class QualifiedNameHashEqualsWrapper implements Serializable { - final CtElement element; - - private static final long serialVersionUID = 1L; - - QualifiedNameHashEqualsWrapper(CtElement element) { - this.element = element; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - QualifiedNameHashEqualsWrapper that = (QualifiedNameHashEqualsWrapper) o; - return QualifiedNameComparator.INSTANCE.compare(element, that.element) == 0; - } - - @Override - public int hashCode() { - return Objects.hash(getQualifiedName(element)); - } - } - - private static String getQualifiedName(CtElement element) { - if (element instanceof CtTypeInformation) { - return ((CtTypeInformation) element).getQualifiedName(); - } else if (element instanceof CtPackage) { - return ((CtPackage) element).getQualifiedName(); - } else if (element instanceof CtReference) { - return ((CtReference) element).getSimpleName(); - } else if (element instanceof CtNamedElement) { - return ((CtNamedElement) element).getSimpleName(); - } - - Launcher.LOGGER.warn( QualifiedNameBasedSortedSet.class.getName() + " used for element without name: " + element.getClass().getName()); - return ""; + super(new QualifiedNameComparator()); } } diff --git a/src/main/java/spoon/support/visitor/equals/CloneHelper.java b/src/main/java/spoon/support/visitor/equals/CloneHelper.java index 92a621148f3..2e178e95ec3 100644 --- a/src/main/java/spoon/support/visitor/equals/CloneHelper.java +++ b/src/main/java/spoon/support/visitor/equals/CloneHelper.java @@ -17,7 +17,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -78,7 +78,7 @@ public Set clone(Set elements) { if (elements == null || elements.isEmpty()) { return EmptyClearableSet.instance(); } - Set others = new LinkedHashSet<>(elements.size()); + Set others = new HashSet<>(elements.size()); for (T element : elements) { addClone(others, element); } From 3edb71afa97067122409124c38054831706fa3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Tue, 5 May 2020 09:19:44 +0200 Subject: [PATCH 06/11] Sort stream and iterator with CtLineElementComparator --- .../support/util/QualifiedNameBasedSortedSet.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index cde4411ff52..f5e85854453 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -8,9 +8,12 @@ package spoon.support.util; import java.util.Collection; +import java.util.Iterator; import java.util.TreeSet; +import java.util.stream.Stream; import spoon.reflect.declaration.CtElement; +import spoon.support.comparator.CtLineElementComparator; import spoon.support.comparator.QualifiedNameComparator; public class QualifiedNameBasedSortedSet extends @@ -27,4 +30,13 @@ public QualifiedNameBasedSortedSet() { super(new QualifiedNameComparator()); } + @Override + public Iterator iterator() { + return stream().iterator(); + } + + @Override + public Stream stream() { + return super.stream().sorted(new CtLineElementComparator()); + } } From 7477ea8056f100af534659687937053021f5a8bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Tue, 5 May 2020 20:33:27 +0200 Subject: [PATCH 07/11] Fix shadow model test by unsetting source position of normal model elements --- .../support/visitor/java/JavaReflectionTreeBuilderTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/spoon/support/visitor/java/JavaReflectionTreeBuilderTest.java b/src/test/java/spoon/support/visitor/java/JavaReflectionTreeBuilderTest.java index e44309de1fc..ce1b0b6a2e2 100644 --- a/src/test/java/spoon/support/visitor/java/JavaReflectionTreeBuilderTest.java +++ b/src/test/java/spoon/support/visitor/java/JavaReflectionTreeBuilderTest.java @@ -26,6 +26,7 @@ import spoon.reflect.code.CtExpression; import spoon.reflect.code.CtLambda; import spoon.reflect.code.CtLocalVariable; +import spoon.reflect.cu.SourcePosition; import spoon.reflect.declaration.CtAnnotation; import spoon.reflect.declaration.CtAnnotationMethod; import spoon.reflect.declaration.CtAnnotationType; @@ -220,6 +221,11 @@ private List checkShadowTypeIsEqual(CtType type) { assertFalse(type.isShadow()); assertTrue(shadowType.isShadow()); + // Some elements, such as superinterfaces and thrown types, are ordered by their source position if they have + // one. As a shadow model has no source positions, but a model built from source does, we must unset the source + // positions of the normal model's elements to ensure that there are no ordering discrepancies. + type.descendantIterator().forEachRemaining(e -> e.setPosition(SourcePosition.NOPOSITION)); + ShadowEqualsVisitor sev = new ShadowEqualsVisitor(new HashSet<>(Arrays.asList( //shadow classes has no body CtRole.STATEMENT, From e7e3bf59f815253cb9fbcd9be6899e7cb17965cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 6 May 2020 09:23:10 +0200 Subject: [PATCH 08/11] Fix incorrect order of thrown types in comment tests --- src/test/java/spoon/test/comment/CommentTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/test/comment/CommentTest.java b/src/test/java/spoon/test/comment/CommentTest.java index 765f8c3baab..a9121266542 100644 --- a/src/test/java/spoon/test/comment/CommentTest.java +++ b/src/test/java/spoon/test/comment/CommentTest.java @@ -476,7 +476,7 @@ public void testInLineComment() { + "// comment before parameters" + newLine + "// comment before type parameter" + newLine + "// comment before name parameter" + newLine - + "int i) throws java.lang.Error, java.lang.Exception {" + newLine + + "int i) throws java.lang.Exception, java.lang.Error {" + newLine + "}", m2.toString()); } @@ -638,7 +638,7 @@ public void testBlockComment() { + "/* comment before parameters */" + newLine + "/* comment before type parameter */" + newLine + "/* comment before name parameter */" + newLine - + "int i) throws java.lang.Error, java.lang.Exception {" + newLine + + "int i) throws java.lang.Exception, java.lang.Error {" + newLine + "}", m2.toString()); // contract: one does not crash when setting a comment starting with '//' in a block comment From 72d443f5c5faa3a775a2398d4b2005109984e738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 6 May 2020 09:23:39 +0200 Subject: [PATCH 09/11] Sort metamodel superconcepts before comparing --- src/test/java/spoon/test/api/MetamodelTest.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/test/java/spoon/test/api/MetamodelTest.java b/src/test/java/spoon/test/api/MetamodelTest.java index f04b161f8f0..540198d6c28 100644 --- a/src/test/java/spoon/test/api/MetamodelTest.java +++ b/src/test/java/spoon/test/api/MetamodelTest.java @@ -63,6 +63,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -284,10 +285,19 @@ private void assertConceptsEqual(MetamodelConcept expectedConcept, MetamodelConc } assertSame(expectedConcept.getMetamodelInterface().getActualClass(), runtimeConcept.getMetamodelInterface().getActualClass()); assertEquals(expectedConcept.getKind(), runtimeConcept.getKind()); - assertEquals(expectedConcept.getSuperConcepts().size(), runtimeConcept.getSuperConcepts().size()); - for (int i = 0; i < expectedConcept.getSuperConcepts().size(); i++) { - assertConceptsEqual(expectedConcept.getSuperConcepts().get(i), runtimeConcept.getSuperConcepts().get(i)); + + // must be sorted as the order of super concepts from source is affected by the order of elements in the + // implements clause + List expectedSuperConcepts = expectedConcept.getSuperConcepts(); + List runtimeSuperConcepts = runtimeConcept.getSuperConcepts(); + expectedSuperConcepts.sort(Comparator.comparing(MetamodelConcept::getName)); + runtimeSuperConcepts.sort(Comparator.comparing(MetamodelConcept::getName)); + + assertEquals(expectedSuperConcepts.size(), runtimeSuperConcepts.size()); + for (int i = 0; i < expectedSuperConcepts.size(); i++) { + assertConceptsEqual(expectedSuperConcepts.get(i), runtimeSuperConcepts.get(i)); } + Map expectedRoleToProperty = new HashMap(expectedConcept.getRoleToProperty()); for (Map.Entry e : runtimeConcept.getRoleToProperty().entrySet()) { MetamodelProperty runtimeProperty = e.getValue(); From ffe2127d2e6955c4a8426997eb84f13d18df0e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 6 May 2020 09:27:59 +0200 Subject: [PATCH 10/11] Document new behavior of QualifiedNameBasedSortedSet --- .../util/QualifiedNameBasedSortedSet.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java index f5e85854453..42527500250 100644 --- a/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java +++ b/src/main/java/spoon/support/util/QualifiedNameBasedSortedSet.java @@ -16,6 +16,15 @@ import spoon.support.comparator.CtLineElementComparator; import spoon.support.comparator.QualifiedNameComparator; +/** + * The set properties of this set are based on the qualified name of the element inserted. Using this set for elements + * without a qualified name does not work well. See the {@link QualifiedNameComparator} for details. + * + * The order of the iterator and stream of this set is based on two properties: qualified name and position. See + * {@link #stream()} for details. + * + * @param + */ public class QualifiedNameBasedSortedSet extends TreeSet { @@ -30,13 +39,34 @@ public QualifiedNameBasedSortedSet() { super(new QualifiedNameComparator()); } + /** + * The order of elements in this iterator is described in {@link #stream()}. + * + * @return A sorted iterator with all elements in this set. + */ @Override public Iterator iterator() { return stream().iterator(); } + /** + * The elements of this stream is ordered into two partitions: elements without source position and elements with + * source position. + * + * Elements without source position appear first, and are between themselves ordered by their qualified names. + * Elements with source position appear last, and are between themselves ordered by their source position. + * + * The rationale for this ordering is that elements such as types listed in implements clauses, or types + * listed in thrown clauses, should not be reordered by qualified name as a result of parsing and printing with + * Spoon. + * + * @return A sorted stream of all elements in this set. + */ @Override public Stream stream() { + // implementation detail: the elements are all ready sorted by the QualifiedNameComparator. Stable sort with + // the CtLineElementComparator ensures that elements with no source position appear before elements with source + // position, but the noposition elements' order relative to each other is not changed. return super.stream().sorted(new CtLineElementComparator()); } } From eb15938b0a5f0f484ba9659a8dc69be97bd04ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Lars=C3=A9n?= Date: Wed, 6 May 2020 11:13:04 +0200 Subject: [PATCH 11/11] Document iterator ordering with a test --- .../util/QualifiedNameBasedSortedSetTest.java | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src/test/java/spoon/support/util/QualifiedNameBasedSortedSetTest.java diff --git a/src/test/java/spoon/support/util/QualifiedNameBasedSortedSetTest.java b/src/test/java/spoon/support/util/QualifiedNameBasedSortedSetTest.java new file mode 100644 index 00000000000..86972d3f3c1 --- /dev/null +++ b/src/test/java/spoon/support/util/QualifiedNameBasedSortedSetTest.java @@ -0,0 +1,115 @@ +package spoon.support.util; + +import org.junit.Test; +import spoon.reflect.cu.CompilationUnit; +import spoon.reflect.cu.SourcePosition; +import spoon.reflect.declaration.CtType; +import spoon.reflect.reference.CtTypeReference; +import spoon.support.visitor.java.JavaReflectionTreeBuilder; +import spoon.testing.utils.ModelUtils; + +import java.io.File; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class QualifiedNameBasedSortedSetTest { + @Test + public void testIteratorOrdering() { + // contract: elements without source position should appear before any element with source position, + // and be ordered between themselves by qualified name. Elements with source position should be ordered + // between themselves by source position. + final CtType linkedListType = new JavaReflectionTreeBuilder(ModelUtils.createFactory()) + .scan(LinkedList.class); + + final MockSourcePosition smallerSourcePos = new MockSourcePosition(10, 12); + final MockSourcePosition largerSourcePos = new MockSourcePosition(14, 15); + + QualifiedNameBasedSortedSet> superInterfaces = new QualifiedNameBasedSortedSet<>( + linkedListType.getSuperInterfaces()); + List> expectedInterfaceOrder = new ArrayList<>(superInterfaces); + + CtTypeReference largerSourcePosElem = expectedInterfaceOrder.remove(0); + CtTypeReference smallerSourcePosElem = expectedInterfaceOrder.remove(0); + // setting the source positions will reorder the elements when fetched from the original set + largerSourcePosElem.setPosition(largerSourcePos); + smallerSourcePosElem.setPosition(smallerSourcePos); + expectedInterfaceOrder.add(smallerSourcePosElem); + expectedInterfaceOrder.add(largerSourcePosElem); + + Iterator> expected = expectedInterfaceOrder.iterator(); + Iterator> actual = superInterfaces.iterator(); + + assertTrue(expected.hasNext()); + while (expected.hasNext()) { + assertEquals(expected.next(), actual.next()); + } + assertFalse(actual.hasNext()); + } + + + private static class MockSourcePosition implements SourcePosition { + final int sourceStart; + final int sourceEnd; + + public MockSourcePosition(int sourceStart, int sourceEnd) { + this.sourceStart = sourceStart; + this.sourceEnd = sourceEnd; + } + + @Override + public boolean isValidPosition() { + return true; + } + + @Override + public int getSourceEnd() { + return sourceEnd; + } + + @Override + public int getSourceStart() { + return sourceStart; + } + + /* + * Methods below here don't matter + */ + + @Override + public File getFile() { + return null; + } + + @Override + public CompilationUnit getCompilationUnit() { + return null; + } + + @Override + public int getLine() { + return 0; + } + + @Override + public int getEndLine() { + return 0; + } + + @Override + public int getColumn() { + return 0; + } + + @Override + public int getEndColumn() { + return 0; + } + + } +}