Repository: SpoonLabs/gumtree-spoon-ast-diff
Pull request #263 (2022-12-14 00:45:20)

Fix for missing position information
**Missing Position Information Fix**
CtVirtualElement and CtWrapper objects do not contain the related position information. As this information is still useful, especially for debugging purposes, this fix correctly maps the objects to their position information during creation. For the case where CtWrapper objects represent more than one code element, the positions are merged.
Relates to: https://github.com/SpoonLabs/gumtree-spoon-ast-diff/issues/199 and https://github.com/SpoonLabs/gumtree-spoon-ast-diff/issues/132

Changes since previous pull request:

- Separated out invocation ordering fix 
- Removed `var` usage
- Extended testing

Remaining issues:

- Which contributors to include in the newly added (required) copyright/license headers

benmss: I thought it best to rename the branch and start a new PR than continue with an incorrectly named branch.
Changes are in the description above.
algomaster99: @benmss Sorry for the late response again. I am busy with some family business. I will look at this during weekend.
algomaster99: @benmss any update on this? Did you get time to review my patch?
benmss: Hi @algomaster99, sorry I didn't get a chance check in on this during the break.

I've looked through the patch and it all makes sense to me so I've applied the changes. Thanks for your hard work.

Only question left from me is what to do with the copyright headers. Would it be possible for you to supply a list of the contributors for me to add? Or can something else be used instead?
benmss: The headers are a requirement in order for Oracle to contribute to open source projects. If you are okay with them as is, that's fine for me also and we can go ahead with the merge.
algomaster99: @benmss thank you! I will proceed with merge.

Patch:
diff --git a/src/main/java/gumtree/spoon/builder/NodeCreator.java b/src/main/java/gumtree/spoon/builder/NodeCreator.java
index 7da3bf8..0faa022 100644
--- a/src/main/java/gumtree/spoon/builder/NodeCreator.java
+++ b/src/main/java/gumtree/spoon/builder/NodeCreator.java
@@ -1,7 +1,26 @@
+/* *****************************************************************************
+ * Copyright 2016 Matias Martinez
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
 package gumtree.spoon.builder;
 
 import java.lang.annotation.Annotation;
+import java.util.ArrayList;
 import java.util.Comparator;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
@@ -9,11 +28,22 @@ import java.util.TreeSet;
 import com.github.gumtreediff.tree.Tree;
 
 import spoon.reflect.code.CtExpression;
-import spoon.reflect.declaration.*;
+import spoon.reflect.cu.CompilationUnit;
+import spoon.reflect.cu.SourcePosition;
+import spoon.reflect.cu.position.NoSourcePosition;
+import spoon.reflect.declaration.CtAnnotation;
+import spoon.reflect.declaration.CtElement;
+import spoon.reflect.declaration.CtMethod;
+import spoon.reflect.declaration.CtModifiable;
+import spoon.reflect.declaration.CtTypeInformation;
+import spoon.reflect.declaration.CtTypedElement;
+import spoon.reflect.declaration.CtVariable;
 import spoon.reflect.path.CtRole;
 import spoon.reflect.reference.CtActualTypeContainer;
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.CtInheritanceScanner;
+import spoon.support.reflect.CtExtendedModifier;
+import spoon.support.reflect.cu.position.SourcePositionImpl;
 
 /**
  * responsible to add additional nodes only overrides scan* to add new nodes
@@ -36,28 +66,46 @@ public class NodeCreator extends CtInheritanceScanner {
 		String type = MODIFIERS + getClassName(m.getClass().getSimpleName());
 		Tree modifiers = builder.createNode(type, "");
 
-		// We create a virtual node
-		modifiers.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT,
-				new CtVirtualElement(type, m, m.getModifiers(), CtRole.MODIFIER));
-
 		// ensuring an order (instead of hashset)
 		// otherwise some flaky tests in CI
-		Set<ModifierKind> modifiers1 = new TreeSet<>(new Comparator<ModifierKind>() {
-			@Override
-			public int compare(ModifierKind o1, ModifierKind o2) {
-				return o1.name().compareTo(o2.name());
-			}
-		});
-		modifiers1.addAll(m.getModifiers());
+		Set<CtExtendedModifier> modifiers1 = new TreeSet<>(Comparator.comparing(o -> o.getKind().name()));
+		modifiers1.addAll(m.getExtendedModifiers());
+
+		// We create a virtual node
+		CtVirtualElement virtualElement = new CtVirtualElement(type, m, m.getModifiers(), CtRole.MODIFIER);
+		modifiers.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT, virtualElement);
 
-		for (ModifierKind kind : modifiers1) {
-			Tree modifier = builder.createNode("Modifier", kind.toString());
+		List<SourcePosition> modifierPositions = new ArrayList<>();
+
+		for (CtExtendedModifier mod : modifiers1) {
+			Tree modifier = builder.createNode("Modifier", mod.getKind().toString());
 			modifiers.addChild(modifier);
-			// We wrap the modifier (which is not a ctelement)
-			modifier.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT, new CtWrapper(kind, m, CtRole.MODIFIER));
+			// We wrap the modifier's kind (which is not a CtElement)
+			CtWrapper wrapper = new CtWrapper<>(mod.getKind(), m, CtRole.MODIFIER);
+			wrapper.setPosition(mod.getPosition());
+			modifierPositions.add(mod.getPosition());
+			modifier.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT, wrapper);
 		}
+
+		virtualElement.setPosition(computeSourcePositionOfVirtualElement(modifierPositions));
 		builder.addSiblingNode(modifiers);
+	}
 
+	private static SourcePosition computeSourcePositionOfVirtualElement(List<SourcePosition> modifierPositions) {
+		CompilationUnit cu = null;
+		Integer sourceStart = null;
+		Integer sourceEnd = null;
+		for (SourcePosition position : modifierPositions) {
+			if (position instanceof NoSourcePosition) { return SourcePosition.NOPOSITION; }
+			if (sourceStart == null || position.getSourceStart() < sourceStart) {
+				sourceStart = position.getSourceStart();
+			}
+			if (sourceEnd == null || position.getSourceEnd() > sourceEnd) {
+				sourceEnd = position.getSourceEnd();
+			}
+			if (cu == null) { cu = position.getCompilationUnit(); }
+		}
+		return new SourcePositionImpl(cu, sourceStart, sourceEnd, cu.getLineSeparatorPositions());
 	}
 
 	private String getClassName(String simpleName) {
@@ -171,7 +219,9 @@ public class NodeCreator extends CtInheritanceScanner {
 		for (Map.Entry<String, CtExpression> entry: annotation.getValues().entrySet()) {
 			Tree annotationValueNode = builder.createNode("ANNOTATION_VALUE", entry.toString());
 			annotationNode.addChild(annotationValueNode);
-			annotationValueNode.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT, new CtWrapper(entry, annotation, CtRole.VALUE));
+			CtWrapper wrapper = new CtWrapper(entry, annotation, CtRole.VALUE);
+			wrapper.setPosition(entry.getValue().getPosition());
+			annotationValueNode.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT, wrapper);
 		}
 		builder.addSiblingNode(annotationNode);
 	}


Test patch:
diff --git a/src/test/java/gumtree/spoon/TreeTest.java b/src/test/java/gumtree/spoon/TreeTest.java
index 7dd6802..df5bbe2 100644
--- a/src/test/java/gumtree/spoon/TreeTest.java
+++ b/src/test/java/gumtree/spoon/TreeTest.java
@@ -1,8 +1,26 @@
+/* *****************************************************************************
+ * Copyright 2016 Matias Martinez
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
 package gumtree.spoon;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.util.List;
@@ -601,4 +619,73 @@ public class TreeTest {
 		assertEquals(1, returnType.getDescendants().size());
 		assertEquals("java.lang.Integer", returnType.getChild(0).getLabel());
 	}
+
+	private void assertPosition(SourcePosition position, int line, int start, int end) {
+		assertNotNull(position);
+		assertEquals(line, position.getLine());
+		assertEquals(start, position.getSourceStart());
+		assertEquals(end, position.getSourceEnd());
+	}
+
+	@Test
+	public void test_accessModifierWrapperInFile_shouldHaveSourcePosition() throws Exception {
+		// arrange
+		File fl = new File("src/test/resources/examples/position/WrapperLeft.java");
+		File fr = new File("src/test/resources/examples/position/WrapperRight.java");
+
+		// act
+		Diff diff = new AstComparator().compare(fl, fr);
+
+		// assert
+		CtElement privateModifier = diff.getRootOperations().get(0).getSrcNode();
+		assertPosition(privateModifier.getPosition(), 18, 789, 795);
+
+		CtElement synchronizedModifier = diff.getRootOperations().get(1).getSrcNode();
+		assertPosition(synchronizedModifier.getPosition(), 18, 804, 815);
+	}
+
+	@Test
+	public void test_accessModifierWrapperInCodeString_shouldHaveSourcePosition() {
+		// arrange
+		String left = "class A { static void a() {} }";
+		String right = "class A { private static synchronized void a() {} }";
+
+		// act
+		Diff diff = new AstComparator().compare(left, right);
+
+		// assert
+		CtElement privateModifier = diff.getRootOperations().get(0).getSrcNode();
+		assertPosition(privateModifier.getPosition(), 1, 10, 16);
+
+		CtElement synchronizedModifier = diff.getRootOperations().get(1).getSrcNode();
+		assertPosition(synchronizedModifier.getPosition(), 1, 25, 36);
+	}
+
+	@Test
+	public void test_accessModifierVirtualElementInFile_shouldHaveSourcePosition() throws Exception {
+		// arrange
+		File fl = new File("src/test/resources/examples/position/VirtualElementLeft.java");
+		File fr = new File("src/test/resources/examples/position/VirtualElementRight.java");
+
+		// act
+		Diff diff = new AstComparator().compare(fl, fr);
+
+		// assert
+		CtElement accessModifiers = diff.getRootOperations().get(0).getSrcNode();
+		assertPosition(accessModifiers.getPosition(), 18, 789, 802);
+	}
+
+	@Test
+	public void test_accessModifierVirtualElementInCodeString_shouldHaveSourcePosition() {
+		// arrange
+		String left = "class A { void a() {} }";
+		String right = "class A { private static void a() {} }";
+
+		// act
+		Diff diff = new AstComparator().compare(left, right);
+
+		// assert
+		CtElement accessModifiers = diff.getRootOperations().get(0).getSrcNode();
+		assertPosition(accessModifiers.getPosition(), 1, 10, 23);
+	}
 }
diff --git a/src/test/resources/examples/position/VirtualElementLeft.java b/src/test/resources/examples/position/VirtualElementLeft.java
new file mode 100644
index 0000000..7529fdf
--- /dev/null
+++ b/src/test/resources/examples/position/VirtualElementLeft.java
@@ -0,0 +1,19 @@
+/* *****************************************************************************
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
+class A {
+    void a() {}
+}
diff --git a/src/test/resources/examples/position/VirtualElementRight.java b/src/test/resources/examples/position/VirtualElementRight.java
new file mode 100644
index 0000000..e29b1be
--- /dev/null
+++ b/src/test/resources/examples/position/VirtualElementRight.java
@@ -0,0 +1,19 @@
+/* *****************************************************************************
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
+class A {
+    private static void a() {}
+}
diff --git a/src/test/resources/examples/position/WrapperLeft.java b/src/test/resources/examples/position/WrapperLeft.java
new file mode 100644
index 0000000..3ba3858
--- /dev/null
+++ b/src/test/resources/examples/position/WrapperLeft.java
@@ -0,0 +1,19 @@
+/* *****************************************************************************
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
+class A {
+    static void a() {}
+}
diff --git a/src/test/resources/examples/position/WrapperRight.java b/src/test/resources/examples/position/WrapperRight.java
new file mode 100644
index 0000000..f8ec3f8
--- /dev/null
+++ b/src/test/resources/examples/position/WrapperRight.java
@@ -0,0 +1,19 @@
+/* *****************************************************************************
+ * Copyright (c) 2022, Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * *****************************************************************************/
+
+class A {
+    private static synchronized void a() {}
+}