fix: ignore null items in ReplacementVisitor

src/main/java/spoon/generating/replace/ReplacementVisitor.java

@@ -19,6 +19,7 @@
 import spoon.SpoonException;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.visitor.CtScanner;
+import spoon.support.visitor.replace.InvalidReplaceException;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -37,12 +38,16 @@ class ReplacementVisitor extends CtScanner {
 	public static void replace(CtElement original, CtElement replace) {
 		try {
 			new ReplacementVisitor(original, replace == null ? EMPTY : new CtElement[]{replace}).scan(original.getParent());
+		} catch (InvalidReplaceException e) {
+			throw e;
 		} catch (SpoonException ignore) {
 		}
 	}
 	public static <E extends CtElement> void replace(CtElement original, Collection<E> replaces) {
 		try {
 			new ReplacementVisitor(original, replaces.toArray(new CtElement[replaces.size()])).scan(original.getParent());
+		} catch (InvalidReplaceException e) {
+			throw e;
 		} catch (SpoonException ignore) {
 		}
 	}
@@ -71,11 +76,15 @@ private <K, V extends CtElement> void replaceInMapIfExist(Map<K, V> mapProtected
 		if (shouldBeDeleted != null) {
 			if (replace.length > 0) {
 				if (replace.length > 1) {
-					throw new SpoonException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName());
+					throw new InvalidReplaceException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName());
 				}
 				V val = (V) replace[0];
-				map.put(key, val);
-				val.setParent(shouldBeDeleted.getParent());
+				if (val != null) {
+					map.put(key, val);
+					val.setParent(shouldBeDeleted.getParent());
+				} else {
+					map.remove(key);
+				}
 			} else {
 				map.remove(key);
 			}
@@ -95,8 +104,10 @@ private <T extends CtElement> void replaceInSetIfExist(Set<T> setProtected, Repl
 		if (shouldBeDeleted != null) {
 			set.remove(shouldBeDeleted);
 			for (CtElement ele : replace) {
-				set.add((T) ele);
-				ele.setParent(shouldBeDeleted.getParent());
+				if (ele != null) {
+					set.add((T) ele);
+					ele.setParent(shouldBeDeleted.getParent());
+				}
 			}
 			listener.set(set);
 		}
@@ -117,8 +128,12 @@ private <T extends CtElement> void replaceInListIfExist(List<T> listProtected, R
 			list.remove(index);
 			if (replace.length > 0) {
 				for (int i = 0; i < replace.length; i++) {
-					list.add(index + i, (T) replace[i]);
-					replace[i].setParent(shouldBeDeleted.getParent());
+					T ele = (T) replace[i];
+					if (ele != null) {
+						list.add(index, ele);
+						ele.setParent(shouldBeDeleted.getParent());
+						index = index + 1;
+					}
 				}
 			}
 			listener.set(list);
@@ -130,7 +145,7 @@ private void replaceElementIfExist(CtElement candidate, ReplaceListener listener
 			CtElement val = null;
 			if (replace.length > 0) {
 				if (replace.length > 1) {
-					throw new SpoonException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName());
+					throw new InvalidReplaceException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName());
 				}
 				val = replace[0];
 			}

src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java

@@ -0,0 +1,41 @@
+/**
+ * Copyright (C) 2006-2017 INRIA and contributors
+ * Spoon - http://spoon.gforge.inria.fr/
+ *
+ * This software is governed by the CeCILL-C License under French law and
+ * abiding by the rules of distribution of free software. You can use, modify
+ * and/or redistribute the software under the terms of the CeCILL-C license as
+ * circulated by CEA, CNRS and INRIA at http://www.cecill.info.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details.
+ *
+ * The fact that you are presently reading this means that you have had
+ * knowledge of the CeCILL-C license and that you accept its terms.
+ */
+package spoon.support.visitor.replace;
+
+import spoon.SpoonException;
+
+/**
+ * Thrown when replacing of element by another element is not possible
+ */
+public class InvalidReplaceException extends SpoonException {
+
+	public InvalidReplaceException() {
+	}
+
+	public InvalidReplaceException(String msg) {
+		super(msg);
+	}
+
+	public InvalidReplaceException(Throwable e) {
+		super(e);
+	}
+
+	public InvalidReplaceException(String msg, Throwable e) {
+		super(msg, e);
+	}
+
+}

src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java

@@ -26,13 +26,17 @@ public class ReplacementVisitor extends spoon.reflect.visitor.CtScanner {
 	public static void replace(spoon.reflect.declaration.CtElement original, spoon.reflect.declaration.CtElement replace) {
 		try {
 			new spoon.support.visitor.replace.ReplacementVisitor(original, (replace == null ? spoon.support.visitor.replace.ReplacementVisitor.EMPTY : new spoon.reflect.declaration.CtElement[]{ replace })).scan(original.getParent());
+		} catch (spoon.support.visitor.replace.InvalidReplaceException e) {
+			throw e;
 		} catch (spoon.SpoonException ignore) {
 		}
 	}
 
 	public static <E extends spoon.reflect.declaration.CtElement> void replace(spoon.reflect.declaration.CtElement original, java.util.Collection<E> replaces) {
 		try {
 			new spoon.support.visitor.replace.ReplacementVisitor(original, replaces.toArray(new spoon.reflect.declaration.CtElement[replaces.size()])).scan(original.getParent());
+		} catch (spoon.support.visitor.replace.InvalidReplaceException e) {
+			throw e;
 		} catch (spoon.SpoonException ignore) {
 		}
 	}
@@ -62,11 +66,15 @@ private <K, V extends spoon.reflect.declaration.CtElement> void replaceInMapIfEx
 		if (shouldBeDeleted != null) {
 			if ((replace.length) > 0) {
 				if ((replace.length) > 1) {
-					throw new spoon.SpoonException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName())));
+					throw new spoon.support.visitor.replace.InvalidReplaceException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName())));
 				}
 				V val = ((V) (replace[0]));
-				map.put(key, val);
-				val.setParent(shouldBeDeleted.getParent());
+				if (val != null) {
+					map.put(key, val);
+					val.setParent(shouldBeDeleted.getParent());
+				}else {
+					map.remove(key);
+				}
 			}else {
 				map.remove(key);
 			}
@@ -86,8 +94,10 @@ private <T extends spoon.reflect.declaration.CtElement> void replaceInSetIfExist
 		if (shouldBeDeleted != null) {
 			set.remove(shouldBeDeleted);
 			for (spoon.reflect.declaration.CtElement ele : replace) {
-				set.add(((T) (ele)));
-				ele.setParent(shouldBeDeleted.getParent());
+				if (ele != null) {
+					set.add(((T) (ele)));
+					ele.setParent(shouldBeDeleted.getParent());
+				}
 			}
 			listener.set(set);
 		}
@@ -108,8 +118,12 @@ private <T extends spoon.reflect.declaration.CtElement> void replaceInListIfExis
 			list.remove(index);
 			if ((replace.length) > 0) {
 				for (int i = 0; i < (replace.length); i++) {
-					list.add((index + i), ((T) (replace[i])));
-					replace[i].setParent(shouldBeDeleted.getParent());
+					T ele = ((T) (replace[i]));
+					if (ele != null) {
+						list.add(index, ele);
+						ele.setParent(shouldBeDeleted.getParent());
+						index = index + 1;
+					}
 				}
 			}
 			listener.set(list);
@@ -121,7 +135,7 @@ private void replaceElementIfExist(spoon.reflect.declaration.CtElement candidate
 			spoon.reflect.declaration.CtElement val = null;
 			if ((replace.length) > 0) {
 				if ((replace.length) > 1) {
-					throw new spoon.SpoonException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName())));
+					throw new spoon.support.visitor.replace.InvalidReplaceException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName())));
 				}
 				val = replace[0];
 			}

src/test/java/spoon/test/annotation/AnnotationTest.java

@@ -4,6 +4,7 @@
 import org.junit.Test;
 import spoon.Launcher;
 import spoon.OutputType;
+import spoon.SpoonException;
 import spoon.processing.AbstractAnnotationProcessor;
 import spoon.processing.ProcessingManager;
 import spoon.reflect.annotations.PropertyGetter;
@@ -66,6 +67,8 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
@@ -77,6 +80,7 @@
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static spoon.testing.utils.ModelUtils.buildClass;
 import static spoon.testing.utils.ModelUtils.canBeBuilt;
 
@@ -1028,4 +1032,67 @@ public void testCreateAnnotation() throws Exception {
 		assertTrue(type.isAnnotationType());
 		assertSame(type, type.getReference().getDeclaration());
 	}
+
+	@Test
+	public void testReplaceAnnotationValue() throws Exception {
+		CtType<?> type = this.factory.Type().get("spoon.test.annotation.testclasses.Main");
+
+		CtMethod<?> m1 = type.getElements(new NameFilter<CtMethod<?>>("m1")).get(0);
+
+		List<CtAnnotation<? extends Annotation>> annotations = m1.getAnnotations();
+		assertEquals(1, annotations.size());
+
+		CtAnnotation<?> a = annotations.get(0);
+		AnnotParamTypes annot = (AnnotParamTypes) a.getActualAnnotation();
+
+		//contract: test replace of single value
+		CtExpression integerValue = a.getValue("integer");
+		assertEquals(42, ((CtLiteral<Integer>) integerValue).getValue().intValue());
+		assertEquals(42, annot.integer());
+		integerValue.replace(factory.createLiteral(17));
+		CtExpression newIntegerValue = a.getValue("integer");
+		assertEquals(17, ((CtLiteral<Integer>) newIntegerValue).getValue().intValue());
+		assertEquals(17, annot.integer());
+
+		//contract: replacing of single value of map by multiple values must fail
+		//even if second value is null
+		try {
+			a.getValue("integer").replace(Arrays.asList(factory.createLiteral(18), null));
+			fail();
+		} catch (SpoonException e)  {
+			//OK
+		}
+
+		//contract: replacing of single value by no value
+		a.getValue("integer").delete();
+		assertNull(a.getValue("integer"));
+		try {
+			annot.integer();
+			fail();
+		} catch (NullPointerException e) {
+			//OK - fails because int cannot be null
+		}
+		//contract: replace with null value means remove
+		a.getValue("string").replace((CtElement) null);
+		assertNull(a.getValue("string"));
+		//contract: check that null value can be returned
+		assertNull(annot.string());
+
+		//contract: replace with null value in collection means remove
+		a.getValue("clazz").replace(Collections.singletonList(null));
+		assertNull(a.getValue("clazz"));
+		//contract: check that null value can be returned
+		assertNull(annot.clazz());
+
+		//contract: test replace of item in collection
+		assertEquals(1, annot.integers().length);
+		assertEquals(42, annot.integers()[0]);
+		CtNewArray<?> integersNewArray = (CtNewArray)a.getValue("integers");
+		integersNewArray.getElements().get(0).replace(Arrays.asList(null, factory.createLiteral(101), null, factory.createLiteral(102)));
+		assertEquals(2, annot.integers().length);
+		assertEquals(101, annot.integers()[0]);
+		assertEquals(102, annot.integers()[1]);
+	}
+
+
 }

src/test/java/spoon/test/replace/ReplaceTest.java

@@ -30,15 +30,18 @@
 import spoon.reflect.visitor.filter.NameFilter;
 import spoon.reflect.visitor.filter.ReferenceTypeFilter;
 import spoon.reflect.visitor.filter.TypeFilter;
+import spoon.support.visitor.replace.InvalidReplaceException;
 import spoon.test.replace.testclasses.Mole;
 import spoon.test.replace.testclasses.Tacos;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
 import static spoon.testing.utils.ModelUtils.build;
 import static spoon.testing.utils.ModelUtils.buildClass;
 
@@ -163,6 +166,45 @@ public void testReplaceStmtByList() {
 		assertEquals(2, ((CtBlock) sample.getMethod("retry").getBody().getStatement(0)).getStatements().size());
 	}
 
+	@Test
+	public void testReplaceStmtByListStatements() {
+		CtClass<?> sample = factory.Package().get("spoon.test.replace.testclasses")
+				.getType("Foo");
+
+		// replace retry content by statements
+		CtStatement stmt = sample.getMethod("retry").getBody().getStatement(0);
+		List<CtStatement> lst = sample.getMethod("statements").getBody().getStatements();
+
+		// replace a single statement by a statement list
+		stmt.replace(lst);
+
+		// we should have only 2 statements after (from the stmt list)
+		assertEquals(2, sample.getMethod("retry").getBody().getStatements().size());
+	}
+
+	@Test
+	public void testReplaceStmtByListStatementsAndNull() {
+		//contract: null elements in list are ignored
+		CtClass<?> sample = factory.Package().get("spoon.test.replace.testclasses")
+				.getType("Foo");
+
+		// replace retry content by statements
+		CtStatement stmt = sample.getMethod("retry").getBody().getStatement(0);
+		List<CtStatement> lst = sample.getMethod("statements").getBody().getStatements();
+		List<CtStatement> lstWithNulls = new ArrayList<>();
+		lstWithNulls.add(null);
+		lstWithNulls.add(lst.get(0));
+		lstWithNulls.add(null);
+		lstWithNulls.add(lst.get(1));
+		lstWithNulls.add(null);
+
+		// replace a single statement by a statement list
+		stmt.replace(lstWithNulls);
+
+		// we should have only 2 statements after (from the stmt list)
+		assertEquals(2, sample.getMethod("retry").getBody().getStatements().size());
+	}
+
 	@Test
 	public void testReplaceField() {
 		CtClass<?> sample = factory.Package().get("spoon.test.replace.testclasses")
@@ -350,13 +392,21 @@ public void testReplaceExecutableReferenceByAnotherOne() throws Exception {
 		final CtExecutableReference oldExecutable = inv.getExecutable();
 		final CtExecutableReference<Object> newExecutable = factory.Executable().createReference("void java.io.PrintStream#print(java.lang.String)");
 
-		assertEquals(oldExecutable, inv.getExecutable());
+		assertSame(oldExecutable, inv.getExecutable());
 		assertEquals("java.io.PrintStream#println(java.lang.String)", inv.getExecutable().toString());
 
 		oldExecutable.replace(newExecutable);
 
-		assertEquals(newExecutable, inv.getExecutable());
+		assertSame(newExecutable, inv.getExecutable());
 		assertEquals("java.io.PrintStream#print(java.lang.String)", inv.getExecutable().toString());
+
+		//contract: replace of single value by multiple values in single value field must fail
+		try {
+			newExecutable.replace(Arrays.asList(oldExecutable, null));
+			fail();
+		} catch (InvalidReplaceException e) {
+			//OK
+		}
 	}
 
 	@Test