Repository: SpoonLabs/gumtree-spoon-ast-diff
Pull request #221 (2022-01-03 14:22:58)

feat: produce fine-grained AST diff for annotations
Fixes: #220 

The changes wrap values of `CtAnnotation` in `CtVirtualElement` which in turn wraps all the key-value pairs in `CtWrapper`. Note that this case is similar to how we deal with modifiers because values of annotation are not spoon nodes. They are of the form `Map.Entry<String, CtExpression>`.

However, there is one pending task before we move on to merging. ~The `rootOperations` also an update operation like in the linked issue along with two insert operations. The latter should not be reported because node in update operation is parent of the `srcNode`s in the insert operation. In my experience, the `ActionClassifier` should ignore the operation if a parent _at any level exists_. More technically, [this line](https://github.com/SpoonLabs/gumtree-spoon-ast-diff/blob/master/src/main/java/gumtree/spoon/diff/ActionClassifier.java#L75) should be modified to check for parent, grand-parent, and so on of `t`.~

EDIT:

A better way to deal with it would be to just remove the unnecessary update operation being computed. #125 keeps gaining priority. :exploding_head: 

algomaster99: @monperrus this is ready for review.

Patch:
diff --git a/src/main/java/gumtree/spoon/builder/LabelFinder.java b/src/main/java/gumtree/spoon/builder/LabelFinder.java
index 24ec1b6..53d10c4 100644
--- a/src/main/java/gumtree/spoon/builder/LabelFinder.java
+++ b/src/main/java/gumtree/spoon/builder/LabelFinder.java
@@ -140,7 +140,7 @@ class LabelFinder extends CtInheritanceScanner {
 
 	@Override
 	public <T extends Annotation> void visitCtAnnotation(CtAnnotation<T> annotation) {
-		label = annotation.toString();
+		label = annotation.getType().getQualifiedName();
 	}
 
 	@Override
diff --git a/src/main/java/gumtree/spoon/builder/NodeCreator.java b/src/main/java/gumtree/spoon/builder/NodeCreator.java
index e0c8df4..2a33923 100644
--- a/src/main/java/gumtree/spoon/builder/NodeCreator.java
+++ b/src/main/java/gumtree/spoon/builder/NodeCreator.java
@@ -1,11 +1,15 @@
 package gumtree.spoon.builder;
 
+import java.lang.annotation.Annotation;
 import java.util.Comparator;
+import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
 import com.github.gumtreediff.tree.Tree;
 
+import spoon.reflect.code.CtExpression;
+import spoon.reflect.declaration.CtAnnotation;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.declaration.CtModifiable;
@@ -157,4 +161,24 @@ public class NodeCreator extends CtInheritanceScanner {
 		}
 		super.visitCtMethod(e);
 	}
+
+	@Override
+	public <A extends Annotation> void visitCtAnnotation(CtAnnotation<A> annotation) {
+		if (annotation.getValues().isEmpty()) {
+			return;
+		}
+
+		final String virtualNodeDescription = "AnnotationValues_" + getClassName(annotation.getClass().getSimpleName());
+		Tree annotationNode = builder.createNode(virtualNodeDescription, "");
+
+		annotationNode.setMetadata(SpoonGumTreeBuilder.SPOON_OBJECT,
+				new CtVirtualElement(virtualNodeDescription, annotation, annotation.getValues().entrySet(), CtRole.VALUE));
+
+		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));
+		}
+		builder.addSiblingNode(annotationNode);
+	}
 }
diff --git a/src/main/java/gumtree/spoon/builder/TreeScanner.java b/src/main/java/gumtree/spoon/builder/TreeScanner.java
index 6395636..6bb33e2 100644
--- a/src/main/java/gumtree/spoon/builder/TreeScanner.java
+++ b/src/main/java/gumtree/spoon/builder/TreeScanner.java
@@ -10,7 +10,9 @@ import com.github.gumtreediff.tree.Type;
 
 import spoon.reflect.code.CtBlock;
 import spoon.reflect.code.CtCase;
+import spoon.reflect.code.CtLiteral;
 import spoon.reflect.code.CtStatementList;
+import spoon.reflect.declaration.CtAnnotation;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.path.CtRole;
 import spoon.reflect.reference.CtReference;
@@ -83,6 +85,11 @@ public class TreeScanner extends CtScanner {
 			return false;
 		}
 
+		// We need to ignore the literal in the annotation value because we store them in CtWrapper
+		if (element instanceof CtLiteral<?> && element.getParent() instanceof CtAnnotation) {
+			return true;
+		}
+
 		return element.isImplicit() || element instanceof CtReference;
 	}
 


Test patch:
diff --git a/src/test/java/gumtree/spoon/TreeTest.java b/src/test/java/gumtree/spoon/TreeTest.java
index 5050d45..7dd6802 100644
--- a/src/test/java/gumtree/spoon/TreeTest.java
+++ b/src/test/java/gumtree/spoon/TreeTest.java
@@ -542,7 +542,7 @@ public class TreeTest {
 		Tree root = scanner.getTree(spoonType);
 
 		Tree annotationNode = root.getChild(0).getChild(0).getChild(2);
-		assertEquals("@java.lang.Override", annotationNode.getLabel());
+		assertEquals("java.lang.Override", annotationNode.getLabel());
 	}
 
 	@Test
diff --git a/src/test/java/gumtree/spoon/diff/DiffTest.java b/src/test/java/gumtree/spoon/diff/DiffTest.java
index f09737b..332b7cd 100644
--- a/src/test/java/gumtree/spoon/diff/DiffTest.java
+++ b/src/test/java/gumtree/spoon/diff/DiffTest.java
@@ -1,13 +1,20 @@
 package gumtree.spoon.diff;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsEqual.equalTo;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
 import gumtree.spoon.builder.CtVirtualElement;
@@ -15,6 +22,7 @@ import gumtree.spoon.AstComparator;
 import gumtree.spoon.diff.operations.Operation;
 import gumtree.spoon.diff.operations.OperationKind;
 import spoon.Launcher;
+import spoon.reflect.code.CtLiteral;
 import spoon.reflect.declaration.CtClass;
 
 /**
@@ -322,4 +330,47 @@ public class DiffTest {
 				"java.lang.ClassCastException"
 		}, thrownTypeRoot.getChildren().stream().map(Object::toString).toArray());
 	}
+
+	@Test
+	public void test_diffOfAnnotations_insertionOfValues() throws Exception {
+		File left = new File("src/test/resources/examples/annotationValues/insert/left.java");
+		File right = new File("src/test/resources/examples/annotationValues/insert/right.java");
+
+		Diff diff = new AstComparator().compare(left, right);
+
+		assertEquals(2, diff.getRootOperations().size());
+		assertTrue(diff.containsOperation(OperationKind.Insert, "ANNOTATION_VALUE", "type=\"int\""));
+		assertTrue(diff.containsOperation(OperationKind.Insert, "ANNOTATION_VALUE", "value=\"41\""));
+	}
+
+	@Test
+	public void test_diffOfAnnotations_firstInsertOfValue() throws Exception {
+		File left = new File("src/test/resources/examples/annotationValues/firstInsert/left.java");
+		File right = new File("src/test/resources/examples/annotationValues/firstInsert/right.java");
+
+		Diff diff = new AstComparator().compare(left, right);
+
+		assertEquals(1, diff.getRootOperations().size());
+		Map<String, CtLiteral<?>> expectedValues = new LinkedHashMap<>();
+		expectedValues.put("since", new Launcher().getFactory().createLiteral("4.2"));
+
+		CtVirtualElement annotation = (CtVirtualElement) diff.getRootOperations().get(0).getSrcNode();
+		List<?> actualValues = Arrays.asList(annotation.getChildren().toArray());
+		assertThat(new ArrayList<>(expectedValues.entrySet()), equalTo(actualValues));
+
+	}
+
+	// ToDo
+	@Ignore("Can be fixed after #154")
+	@Test
+	public void test_diffOfAnnotationValues_updateMethod() throws Exception {
+		File left = new File("src/test/resources/examples/annotationValues/updateMethod/left.java");
+		File right = new File("src/test/resources/examples/annotationValues/updateMethod/right.java");
+
+		Diff diff = new AstComparator().compare(left, right);
+
+		assertEquals(2, diff.getRootOperations().size());
+		assertTrue(diff.containsOperations(OperationKind.Update, "AnnotationMethod", "a", "b"));
+		assertTrue(diff.containsOperations(OperationKind.Update, "ANNOTATION_VALUE", "a=\"1\"", "b=\"1\""));
+	}
 }
diff --git a/src/test/resources/examples/annotationValues/firstInsert/left.java b/src/test/resources/examples/annotationValues/firstInsert/left.java
new file mode 100644
index 0000000..c9017fd
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/firstInsert/left.java
@@ -0,0 +1,4 @@
+class Test {
+    @Deprecated
+    public Test(int x) { }
+}
diff --git a/src/test/resources/examples/annotationValues/firstInsert/right.java b/src/test/resources/examples/annotationValues/firstInsert/right.java
new file mode 100644
index 0000000..701c491
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/firstInsert/right.java
@@ -0,0 +1,4 @@
+class Test {
+    @Deprecated(since = "4.2")
+    public Test(int x) { }
+}
diff --git a/src/test/resources/examples/annotationValues/insert/left.java b/src/test/resources/examples/annotationValues/insert/left.java
new file mode 100644
index 0000000..9cc8788
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/insert/left.java
@@ -0,0 +1,13 @@
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+@interface Column {
+    int id();
+    String description();
+    String type() default "String";
+    String value() default "42"
+}
+
+@Column(id = 1, description = "blah")
+public class AnnotationValue { }
diff --git a/src/test/resources/examples/annotationValues/insert/right.java b/src/test/resources/examples/annotationValues/insert/right.java
new file mode 100644
index 0000000..bc0b012
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/insert/right.java
@@ -0,0 +1,13 @@
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+@interface Column {
+    int id();
+    String description();
+    String type() default "String";
+    String value() default "42"
+}
+
+@Column(id = 1, type = "int", description = "blah", value = "41")
+public class AnnotationValue { }
diff --git a/src/test/resources/examples/annotationValues/updateMethod/left.java b/src/test/resources/examples/annotationValues/updateMethod/left.java
new file mode 100644
index 0000000..8269e1d
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/updateMethod/left.java
@@ -0,0 +1,10 @@
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+@interface Column {
+    String a();
+}
+
+@Column(a = "1")
+class A { }
diff --git a/src/test/resources/examples/annotationValues/updateMethod/right.java b/src/test/resources/examples/annotationValues/updateMethod/right.java
new file mode 100644
index 0000000..56f464e
--- /dev/null
+++ b/src/test/resources/examples/annotationValues/updateMethod/right.java
@@ -0,0 +1,10 @@
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+@interface Column {
+    String b();
+}
+
+@Column(b = "1")
+class A { }