diff --git a/src/main/java/spoon/reflect/factory/CodeFactory.java b/src/main/java/spoon/reflect/factory/CodeFactory.java index ac33e083fc2..62fe24f068c 100644 --- a/src/main/java/spoon/reflect/factory/CodeFactory.java +++ b/src/main/java/spoon/reflect/factory/CodeFactory.java @@ -27,6 +27,7 @@ import spoon.reflect.code.CtLocalVariable; import spoon.reflect.code.CtNewArray; import spoon.reflect.code.CtNewClass; +import spoon.reflect.code.CtReturn; import spoon.reflect.code.CtStatement; import spoon.reflect.code.CtStatementList; import spoon.reflect.code.CtTextBlock; @@ -647,6 +648,19 @@ public CtAnnotation createAnnotation(CtTypeReference the type of the expression + * @return a return. + */ + public CtReturn createCtReturn(CtExpression expression) { + final CtReturn result = factory.Core().createReturn(); + result.setReturnedExpression(expression); + return result; + } + /** * Gets a list of references from a list of elements. * diff --git a/src/main/java/spoon/reflect/factory/Factory.java b/src/main/java/spoon/reflect/factory/Factory.java index 7825d64c6b9..c428ad9fe9c 100644 --- a/src/main/java/spoon/reflect/factory/Factory.java +++ b/src/main/java/spoon/reflect/factory/Factory.java @@ -321,6 +321,14 @@ public interface Factory { */ CtLocalVariableReference createLocalVariableReference(CtLocalVariable localVariable); + /** + * @param expression the expression to return + * @param the type of the expression + * @return a return statement + * @see CodeFactory#createCtReturn(CtExpression) + */ + CtReturn createCtReturn(CtExpression expression); + /** * @see CodeFactory#createLocalVariableReference(CtTypeReference,String) */ diff --git a/src/main/java/spoon/reflect/factory/FactoryImpl.java b/src/main/java/spoon/reflect/factory/FactoryImpl.java index 615c157cb1f..d8b5cbb1141 100644 --- a/src/main/java/spoon/reflect/factory/FactoryImpl.java +++ b/src/main/java/spoon/reflect/factory/FactoryImpl.java @@ -509,6 +509,11 @@ public CtLocalVariable createLocalVariable(CtTypeReference type, Strin return Code().createLocalVariable(type, name, defaultExpression); } + @Override + public CtReturn createCtReturn(CtExpression expression) { + return Code().createCtReturn(expression); + } + @SuppressWarnings(value = "unchecked") @Override public CtNewArray createLiteralArray(T[] value) { diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index 35021a06db5..00e069c3563 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -251,8 +251,13 @@ public void scanCtType(CtType type) { return; } else if (child instanceof CtEnumValue && type instanceof CtEnum) { ((CtEnum) type).addEnumValue((CtEnumValue) child); - } else if (child instanceof CtField) { - type.addField((CtField) child); + } else if (child instanceof CtField field) { + // We add the field in addRecordComponent. Afterward, however, JDT visits the Field itself -> Duplication. + // To combat this, we delete the existing field and trust JDTs version. + if (type instanceof CtRecord record) { + record.removeField(record.getField(field.getSimpleName())); + } + type.addField(field); return; } else if (child instanceof CtConstructor) { return; diff --git a/src/main/java/spoon/support/reflect/declaration/CtRecordComponentImpl.java b/src/main/java/spoon/support/reflect/declaration/CtRecordComponentImpl.java index 9fb7553abbd..7a478c2d073 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtRecordComponentImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtRecordComponentImpl.java @@ -10,17 +10,24 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; + +import org.jspecify.annotations.Nullable; import spoon.JLSViolation; import spoon.reflect.annotations.MetamodelPropertyField; +import spoon.reflect.code.CtFieldAccess; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.declaration.CtRecord; import spoon.reflect.declaration.CtRecordComponent; import spoon.reflect.declaration.CtShadowable; import spoon.reflect.declaration.CtTypedElement; import spoon.reflect.declaration.ModifierKind; import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtFieldReference; import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.CtVisitor; import spoon.support.reflect.CtExtendedModifier; @@ -36,24 +43,47 @@ public class CtRecordComponentImpl extends CtNamedElementImpl implements CtRecor public CtMethod toMethod() { CtMethod method = this.getFactory().createMethod(); method.setSimpleName(getSimpleName()); - method.setType((CtTypeReference) getType()); + method.setType(getClonedType()); method.setExtendedModifiers(Collections.singleton(new CtExtendedModifier(ModifierKind.PUBLIC, true))); - method.setImplicit(true); - method.setBody(getFactory().createCodeSnippetStatement("return " + getSimpleName())); - return method; + + CtFieldAccess ctVariableAccess = (CtFieldAccess) getFactory().Code() + .createVariableRead(getRecordFieldReference(), false); + + method.setBody(getFactory().Code().createCtReturn(ctVariableAccess)); + + return makeTreeImplicit(method); + } + + private CtFieldReference getRecordFieldReference() { + CtRecord parent = isParentInitialized() ? (CtRecord) getParent() : null; + + // Reference the field we think should exist. It might be added to the record later on, so do not directly + // query for it. + CtFieldReference reference = getFactory().createFieldReference() + .setFinal(true) + .setStatic(false) + .setType(getClonedType()) + .setSimpleName(getSimpleName()); + + // We have a parent record, make the field refer to it. Ideally we could do this all the time, but if we + // do not yet have a parent that doesn't work. + if (parent != null) { + reference.setDeclaringType(parent.getReference()); + } + + return reference; } @Override public CtField toField() { CtField field = this.getFactory().createField(); field.setSimpleName(getSimpleName()); - field.setType((CtTypeReference) getType()); + field.setType(getClonedType()); Set modifiers = new HashSet<>(); modifiers.add(new CtExtendedModifier(ModifierKind.PRIVATE, true)); modifiers.add(new CtExtendedModifier(ModifierKind.FINAL, true)); field.setExtendedModifiers(modifiers); - field.setImplicit(true); - return field; + return makeTreeImplicit(field); } @Override @@ -61,6 +91,10 @@ public boolean isImplicit() { return true; } + private @Nullable CtTypeReference getClonedType() { + return getType() != null ? getType().clone() : null; + } + @Override public CtTypeReference getType() { return type; @@ -92,17 +126,15 @@ private void checkName(String simpleName) { JLSViolation.throwIfSyntaxErrorsAreNotIgnored(this, "The name '" + simpleName + "' is not allowed as record component name."); } } + private static Set createForbiddenNames() { return Set.of("clone", "finalize", "getClass", "notify", "notifyAll", "equals", "hashCode", "toString", "wait"); } - @Override public CtRecordComponent clone() { return (CtRecordComponent) super.clone(); } - - @Override public boolean isShadow() { return isShadow; @@ -114,5 +146,14 @@ public E setShadow(boolean isShadow) { this.isShadow = isShadow; return (E) this; } -} + private static T makeTreeImplicit(T element) { + element.accept(new CtScanner() { + @Override + protected void enter(CtElement e) { + e.setImplicit(true); + } + }); + return element; + } +} diff --git a/src/main/java/spoon/support/reflect/declaration/CtRecordImpl.java b/src/main/java/spoon/support/reflect/declaration/CtRecordImpl.java index 09914075639..4ccb8749a68 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtRecordImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtRecordImpl.java @@ -66,6 +66,10 @@ public CtRecord addRecordComponent(CtRecordComponent component) { component.setParent(this); getFactory().getEnvironment().getModelChangeListener().onSetAdd(this, CtRole.RECORD_COMPONENT, components, component); components.add(component); + + if (getField(component.getSimpleName()) == null) { + addField(component.toField()); + } if (!hasMethodWithSameNameAndNoParameter(component)) { addMethod(component.toMethod()); } @@ -100,12 +104,14 @@ public void accept(CtVisitor v) { public > C addTypeMemberAt(int position, CtTypeMember member) { // a record can have only implicit instance fields and this is the best point to preserve the invariant // because there are multiple ways to add a field to a record + String memberName = member.getSimpleName(); + if (member instanceof CtField && !member.isStatic()) { member.setImplicit(true); - getAnnotationsWithName(member.getSimpleName(), ElementType.FIELD).forEach(member::addAnnotation); + getAnnotationsWithName(memberName, ElementType.FIELD).forEach(member::addAnnotation); } if (member instanceof CtMethod && member.isImplicit()) { - getAnnotationsWithName(member.getSimpleName(), ElementType.METHOD).forEach(member::addAnnotation); + getAnnotationsWithName(memberName, ElementType.METHOD).forEach(member::addAnnotation); } if (member instanceof CtConstructor && member.isImplicit()) { for (CtParameter parameter : ((CtConstructor) member).getParameters()) { @@ -115,7 +121,7 @@ public > C addTypeMemberAt(int position, CtTypeMember m } if (member instanceof CtMethod && (member.isAbstract() || member.isNative())) { JLSViolation.throwIfSyntaxErrorsAreNotIgnored(this, String.format("%s method is native or abstract, both is not allowed", - member.getSimpleName())); + memberName)); } if (member instanceof CtAnonymousExecutable && !member.isStatic()) { JLSViolation.throwIfSyntaxErrorsAreNotIgnored(this, "Instance initializer is not allowed in a record (JLS 17 $8.10.2)"); diff --git a/src/test/java/spoon/test/record/CtRecordTest.java b/src/test/java/spoon/test/record/CtRecordTest.java index 67bc31d50dc..7a1ac7c4af7 100644 --- a/src/test/java/spoon/test/record/CtRecordTest.java +++ b/src/test/java/spoon/test/record/CtRecordTest.java @@ -1,12 +1,13 @@ package spoon.test.record; import static java.lang.System.lineSeparator; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static spoon.testing.assertions.SpoonAssertions.assertThat; +import static org.assertj.core.api.Assertions.assertThat; + import java.util.Arrays; import java.util.Collection; import java.util.Comparator; @@ -15,17 +16,25 @@ import java.util.stream.Collectors; import javax.validation.constraints.NotNull; +import org.assertj.core.api.InstanceOfAssertFactory; import org.junit.jupiter.api.Test; import spoon.Launcher; import spoon.reflect.CtModel; +import spoon.reflect.code.CtFieldRead; +import spoon.reflect.code.CtReturn; +import spoon.reflect.code.CtStatement; import spoon.reflect.declaration.CtAnonymousExecutable; import spoon.reflect.declaration.CtConstructor; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtRecord; +import spoon.reflect.declaration.CtRecordComponent; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; +import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.testing.assertions.SpoonAssertions; import spoon.testing.utils.ModelTest; public class CtRecordTest { @@ -76,24 +85,33 @@ public void testMultipleParameterRecord() { assertEquals(1, records.size()); assertEquals("public record MultiParameter(int first, float second) {}", head(records).toString()); - + // test fields assertEquals( Arrays.asList( - "private final int first;", + "private final int first;", "private final float second;" - ), + ), head(records).getFields().stream().map(String::valueOf).collect(Collectors.toList()) ); - + + // Make them explicit so we can print them (but assert they were implicit initially) + assertThat(head(records)).getMethods().allSatisfy(CtElement::isImplicit); + head(records).getMethods().forEach(it -> it.accept(new CtScanner() { + @Override + protected void enter(CtElement e) { + e.setImplicit(false); + } + })); + // test methods assertEquals( Arrays.asList( "int first() {\n" + - " return first;\n" + - "}", + " return this.first;\n" + + "}", "float second() {\n" + - " return second;\n" + + " return this.second;\n" + "}" ), head(records).getMethods().stream() @@ -211,7 +229,7 @@ private CtModel createModelFromPath(String code) { @Test void testGenericTypeParametersArePrintedBeforeTheFunctionParameters() { - // contract: a record with generic type arguments should be printed correctly + // contract: a record with generic type arguments should be printed correctly String code = "src/test/resources/records/GenericRecord.java"; CtModel model = createModelFromPath(code); Collection records = model.getElements(new TypeFilter<>(CtRecord.class)); @@ -225,7 +243,7 @@ void testBuildRecordModelWithStaticInitializer() { String code = "src/test/resources/records/WithStaticInitializer.java"; CtModel model = assertDoesNotThrow(() -> createModelFromPath(code)); List execs = model.getElements(new TypeFilter<>(CtAnonymousExecutable.class)); - assertThat(execs.size(), equalTo(2)); + assertThat(execs).hasSize(2); } @ModelTest(value = "./src/test/resources/records/MultipleConstructors.java", complianceLevel = 16) @@ -275,8 +293,29 @@ void testNonCompactCanonicalConstructor(Factory factory) { assertEquals(constructor.getParameters().get(0).getSimpleName(), "x"); } + @ModelTest(value = "./src/test/resources/records/GenericRecord.java", complianceLevel = 16) + void testProperReturnInRecordAccessor(Factory factory) { + // contract: the return statement in the accessor method should return a field read expression to the correct + // field + CtRecord record = head(factory.getModel().getElements(new TypeFilter<>(CtRecord.class))); + + assertThat(record.getRecordComponents()).isNotEmpty(); + for (CtRecordComponent component : record.getRecordComponents()) { + CtMethod method = component.toMethod(); + + assertThat(method.getBody().getLastStatement()) + .asInstanceOf(new InstanceOfAssertFactory<>(CtReturn.class, SpoonAssertions::assertThat)) + .getReturnedExpression() + .self() + .asInstanceOf(new InstanceOfAssertFactory<>(CtFieldRead.class, SpoonAssertions::assertThat)) + .getVariable() + .getDeclaringType() + .getSimpleName() + .isEqualTo(record.getSimpleName()); + } + } + private T head(Collection collection) { return collection.iterator().next(); } - }