review: fix: Unsettable properties sets no value or sets different primary value

src/main/java/spoon/reflect/code/CtCatchVariable.java

@@ -19,6 +19,7 @@
 import spoon.reflect.declaration.CtMultiTypedElement;
 import spoon.reflect.declaration.CtVariable;
 import spoon.reflect.reference.CtCatchVariableReference;
+import spoon.reflect.reference.CtTypeReference;
 import spoon.support.DerivedProperty;
 import spoon.support.UnsettableProperty;
 
@@ -44,4 +45,8 @@ public interface CtCatchVariable<T> extends CtVariable<T>, CtMultiTypedElement,
 	@Override
 	@UnsettableProperty
 	<C extends CtVariable<T>> C setDefaultExpression(CtExpression<T> assignedExpression);
+
+	@Override
+	@DerivedProperty
+	CtTypeReference<T> getType();
 }

src/main/java/spoon/support/reflect/code/CtCatchVariableImpl.java

@@ -124,6 +124,7 @@ public <C extends CtNamedElement> C setSimpleName(String simpleName) {
 	}
 
 	@Override
+	@UnsettableProperty
 	public <C extends CtTypedElement> C setType(CtTypeReference<T> type) {
 		setMultiTypes(type == null ? emptyList() : Collections.singletonList(type));
 		return (C) this;

src/main/java/spoon/support/reflect/code/CtLambdaImpl.java

@@ -33,12 +33,12 @@
 import spoon.reflect.reference.CtExecutableReference;
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.CtVisitor;
+import spoon.support.UnsettableProperty;
 import spoon.support.reflect.declaration.CtElementImpl;
 import spoon.support.util.QualifiedNameBasedSortedSet;
 import spoon.support.visitor.SignaturePrinter;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -190,19 +190,8 @@ public Set<CtTypeReference<? extends Throwable>> getThrownTypes() {
 	}
 
 	@Override
+	@UnsettableProperty
 	public <C extends CtExecutable<T>> C setThrownTypes(Set<CtTypeReference<? extends Throwable>> thrownTypes) {
-		if (thrownTypes == null || thrownTypes.isEmpty()) {
-			this.thrownTypes = CtElementImpl.emptySet();
-			return (C) this;
-		}
-		if (this.thrownTypes == CtElementImpl.<CtTypeReference<? extends Throwable>>emptySet()) {
-			this.thrownTypes = new QualifiedNameBasedSortedSet<>();
-		}
-		getFactory().getEnvironment().getModelChangeListener().onSetDeleteAll(this, THROWN, this.thrownTypes, new HashSet<>(this.thrownTypes));
-		this.thrownTypes.clear();
-		for (CtTypeReference<? extends Throwable> thrownType : thrownTypes) {
-			addThrownType(thrownType);
-		}
 		return (C) this;
 	}
 

src/main/java/spoon/support/reflect/reference/CtReferenceImpl.java

@@ -17,16 +17,20 @@
 package spoon.support.reflect.reference;
 
 import spoon.reflect.annotations.MetamodelPropertyField;
+import spoon.reflect.code.CtComment;
+import spoon.reflect.declaration.CtElement;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.factory.FactoryImpl;
 import spoon.reflect.path.CtRole;
 import spoon.reflect.reference.CtReference;
 import spoon.reflect.visitor.CtVisitor;
 import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
+import spoon.support.UnsettableProperty;
 import spoon.support.reflect.declaration.CtElementImpl;
 
 import java.io.Serializable;
 import java.lang.reflect.AnnotatedElement;
+import java.util.List;
 
 import static spoon.reflect.path.CtRole.NAME;
 
@@ -63,6 +67,12 @@ public <T extends CtReference> T setSimpleName(String simplename) {
 		return (T) this;
 	}
 
+	@UnsettableProperty
+	@Override
+	public <E extends CtElement> E setComments(List<CtComment> comments) {
+		return (E) this;
+	}
+
 	@Override
 	public String toString() {
 		DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(

src/test/java/spoon/test/replace/ReplaceParametrizedTest.java

@@ -2,30 +2,33 @@
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static spoon.test.parent.ParentContractTest.createCompatibleObject;
-import static spoon.testing.utils.ModelUtils.createFactory;
 
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
-import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
 import spoon.SpoonException;
+import spoon.reflect.code.CtBlock;
 import spoon.reflect.code.CtFieldAccess;
+import spoon.reflect.code.CtStatement;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.meta.ContainerKind;
 import spoon.reflect.meta.RoleHandler;
 import spoon.reflect.meta.impl.RoleHandlerHelper;
 import spoon.reflect.path.CtRole;
+import spoon.reflect.reference.CtFieldReference;
+import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.CtScanner;
 import spoon.reflect.visitor.CtVisitable;
 import spoon.reflect.visitor.Filter;
@@ -60,6 +63,8 @@ public static Collection<Object[]> data() throws Exception {
 
 	@Test
 	public void testContract() throws Throwable {
+		List<String> problems = new ArrayList<>();
+
 		// contract: all elements are replaceable wherever they are in the model
 		// this test puts them at all possible locations
 		CtType<?> toTest = typeToTest.getModelInterface();
@@ -72,15 +77,18 @@ public void testContract() throws Throwable {
 			}
 
 
-			CtElement argument = (CtElement) createCompatibleObject(mmField.getItemValueType());
-
+			CtTypeReference<?> itemType = mmField.getItemValueType();
 			// special cases...
-			if (o.getClass().getSimpleName().equals("CtAnnotationFieldAccessImpl") && mmField.getRole()==CtRole.VARIABLE) {
-				argument = factory.Core().createFieldReference();
+			if (itemType.getQualifiedName().equals(CtStatement.class.getName())) {
+				//the children of CtLoop wraps CtStatement into an implicit CtBlock. So make a block directly to test plain get/set and not wrapping.
+				itemType = factory.createCtTypeReference(CtBlock.class);
 			}
-			if (CtFieldAccess.class.isAssignableFrom(o.getClass()) &&  mmField.getRole()==CtRole.VARIABLE) {
-				argument = factory.Core().createFieldReference();
+			if (o.getClass().getSimpleName().equals("CtAnnotationFieldAccessImpl") && mmField.getRole()==CtRole.VARIABLE) {
+				itemType = factory.createCtTypeReference(CtFieldReference.class);
+			} else if (CtFieldAccess.class.isAssignableFrom(o.getClass()) &&  mmField.getRole()==CtRole.VARIABLE) {
+				itemType = factory.createCtTypeReference(CtFieldReference.class);
 			}
+			CtElement argument = (CtElement) createCompatibleObject(itemType);
 
 			assertNotNull(argument);
 
@@ -97,29 +105,55 @@ public void testContract() throws Throwable {
 					return;
 				}
 				//this unsettable property has setter, but it should do nothing
-				CtElement arg = argument;
-				//Uncomment this assert to see all unsettable properties, which are setting something 
-				//assertTrue("Unsettable field " + mmField + " has setter, which changes model", receiver.getElements(e -> e==arg).size() == 0);
-				return;
+				CtRole argumentsRoleInParent = argument.getRoleInParent();
+				if (argumentsRoleInParent == null) {
+					//OK - unsettable property set no value
+					continue;
+				}
+				if (argumentsRoleInParent == mmField.getRole()) {
+					problems.add("UnsettableProperty " + mmField + " sets the value");
+				} else {
+					if (mmField.isDerived()) {
+						//it is OK, that setting of value into derived unsettable field influences other field
+						//Example 1: CtCatchVariable.setType(x) influences result of getMultitype()
+						//Example 2: CtEnumValue.setAssignment(x) influences result of getDefaultExpression()
+					} else {
+						problems.add("UnsettableProperty " + mmField + " sets the value into different role " + argumentsRoleInParent);
+					}
+				}
+				continue;
 			} 
 
 			// we invoke the setter
 			invokeSetter(rh, receiver, argument);
 
 			// contract: a property setter sets properties that are visitable by a scanner
 			CtElement finalArgument = argument;
-			try {
-				receiver.accept(new CtScanner() {
-					@Override
-					public void scan(CtElement e) {
-						super.scan(e);
-						if (e == finalArgument) {
-							throw new SpoonException();
+			class Scanner extends CtScanner {
+				boolean found = false;
+				@Override
+				public void scan(CtRole role, CtElement e) {
+					super.scan(role, e);
+					if (e == finalArgument) {
+						if (rh.getRole()==role || rh.getRole().getSuperRole()==role) {
+							found = true;
+							return;
 						}
+//						if (rh.getRole()==CtRole.TYPE && role==CtRole.MULTI_TYPE) {
+//							//CtCatchVaraible#type sets CtCatchVaraible#multiType - OK 
+//							found = true;
+//							return;
+//						}
+						problems.add("Argument was set into " + rh.getRole() + " but was found in " + role);
 					}
-				});
-				fail("Not derived field " + mmField.toString() + " should set value");
-			} catch (SpoonException expected) {}
+				}
+			};
+			Scanner s = new Scanner();
+			receiver.accept(s);
+			assertTrue("Settable field " + mmField.toString() + " should set value.\n" + getReport(problems), s.found);
+
+			// contract: a property getter on the same role can be used to get the value back
+			assertSame(argument, invokeGetter(rh, receiver));
 
 			final CtElement argument2 = argument.clone();
 			assertNotSame(argument, argument2);
@@ -135,6 +169,21 @@ public boolean matches(CtElement element) {
 				}
 			}).size() == 1);
 		}
+		if (problems.size() > 0) {
+			fail(getReport(problems));
+		}
+	}
+
+	private String getReport(List<String> problems) {
+		if (problems.size() > 0) {
+			StringBuilder report = new StringBuilder();
+			report.append("The accessors of " + typeToTest + " have problems:");
+			for (String problem : problems) {
+				report.append("\n").append(problem);
+			}
+			return report.toString();
+		}
+		return "";
 	}
 
 	private static void invokeSetter(RoleHandler rh, CtElement receiver, CtElement item) {
@@ -146,5 +195,14 @@ private static void invokeSetter(RoleHandler rh, CtElement receiver, CtElement i
 			rh.asCollection(receiver).add(item);
 		}
 	}
+	private static CtElement invokeGetter(RoleHandler rh, CtElement receiver) {
+		if (rh.getContainerKind() == ContainerKind.SINGLE) {
+			return rh.getValue(receiver);
+		} else if (rh.getContainerKind() == ContainerKind.MAP) {
+			return (CtElement) rh.asMap(receiver).get("dummyKey");
+		} else {
+			return (CtElement) rh.asCollection(receiver).stream().findFirst().get();
+		}
+	}
 
 }