Skip to content

Commit

Permalink
fix combination of lombok.Builder with Introspected without builder (#…
Browse files Browse the repository at this point in the history
…11347)

Currently users have to manually defined @introspected(builder=..) when using Lombok. This change adds native processing of Lombok builder. Not a huge fan of the direct references to Lombok annotation names but I don't see many other options.

Fixes #11344
  • Loading branch information
graemerocher authored Nov 22, 2024
1 parent 5c718a6 commit 9f7af32
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,8 @@ private void writeIntrospectionClass(ClassWriterOutputVisitor classWriterOutputV
dispatchWriter.buildGetTargetMethodByIndex(classWriter);
buildFindIndexedProperty(classWriter);
buildGetIndexedProperties(classWriter);
boolean hasBuilder = annotationMetadata != null && annotationMetadata.isPresent(Introspected.class, "builder");
boolean hasBuilder = annotationMetadata != null &&
(annotationMetadata.isPresent(Introspected.class, "builder") || annotationMetadata.hasDeclaredAnnotation("lombok.Builder"));
if (defaultConstructor != null) {
writeInstantiateMethod(classWriter, defaultConstructor, "instantiate");
// in case invoked directly or via instantiateUnsafe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public class IntrospectedTypeElementVisitor implements TypeElementVisitor<Object
* The position of the visitor.
*/
public static final int POSITION = -100;
private static final String ANN_LOMBOK_BUILDER = "lombok.Builder";

private final Map<String, BeanIntrospectionWriter> writers = new LinkedHashMap<>(10);

Expand Down Expand Up @@ -180,62 +181,101 @@ private void processBuilderDefinition(ClassElement element, VisitorContext conte
AnnotationClassValue<?> builderClass = builder.annotationClassValue("builderClass").orElse(null);
String[] writePrefixes = builder.getAnnotation("accessorStyle", AccessorsStyle.class)
.map(a -> a.stringValues("writePrefixes")).orElse(new String[]{""});
if (builderMethod != null) {
MethodElement methodElement = element
.getEnclosedElement(ElementQuery.ALL_METHODS.onlyStatic()
.filter(m -> m.getName().equals(builderMethod) && !m.getGenericReturnType().isVoid())
.onlyAccessible(element))
.orElse(null);
if (methodElement != null) {
ClassElement returnType = methodElement.getGenericReturnType();
if (returnType.isPublic() || returnType.getPackageName().equals(element.getPackageName())) {
AnnotationValueBuilder<Introspected> replaceIntrospected = AnnotationValue.builder(introspected, RetentionPolicy.RUNTIME);
replaceIntrospected.member("builderClass", new AnnotationClassValue<>(returnType.getName()));
element.annotate(replaceIntrospected.build());
AnnotationMetadata methodMetadata = methodElement.getMethodAnnotationMetadata().getTargetAnnotationMetadata();

handleBuilder(
element,
context,
creatorMethod,
writePrefixes,
methodElement,
null,
returnType,
methodMetadata,
index,
targetPackage
);
} else {
context.fail("Builder return type is not public. The method must be static and accessible.", methodElement);
}
} else {
context.fail("Method specified by builderMethod not found. The method must be static and accessible.", element);
}
} else if (builderClass != null) {
ClassElement builderClassElement = context.getClassElement(builderClass.getName()).orElse(null);
if (builderClassElement != null) {
processBuilderDefinition(
element,
context,
introspected,
index,
targetPackage,
builderMethod,
creatorMethod,
writePrefixes,
builderClass
);
} else if (element.hasDeclaredAnnotation(ANN_LOMBOK_BUILDER)) {
AnnotationValue<Annotation> lombokBuilder = element.getAnnotation(ANN_LOMBOK_BUILDER);
String builderMethod = lombokBuilder.stringValue("builderMethodName").orElse("builder");
MethodElement methodElement = element
.getEnclosedElement(ElementQuery.ALL_METHODS.onlyStatic()
.filter(m -> m.getName().equals(builderMethod) && !m.getGenericReturnType().isVoid())
.onlyAccessible(element))
.orElse(null);
if (methodElement == null) {
// Lombok processing not done yet, try again in the next round.
throw new ElementPostponedToNextRoundException(element);
}
String creatorMethod = lombokBuilder.stringValue("buildMethodName").orElse("build");
String[] writePrefixes = lombokBuilder.stringValue("setterPrefix").map(sp -> new String[] { sp }).orElse(new String[]{""});
processBuilderDefinition(
element,
context,
introspected,
index,
targetPackage,
builderMethod,
creatorMethod,
writePrefixes,
null
);
}
}

private void processBuilderDefinition(ClassElement element, VisitorContext context, AnnotationValue<Introspected> introspected, int index, String targetPackage, String builderMethod, String creatorMethod, String[] writePrefixes, AnnotationClassValue<?> builderClass) {
if (builderMethod != null) {
MethodElement methodElement = element
.getEnclosedElement(ElementQuery.ALL_METHODS.onlyStatic()
.filter(m -> m.getName().equals(builderMethod) && !m.getGenericReturnType().isVoid())
.onlyAccessible(element))
.orElse(null);
if (methodElement != null) {
ClassElement returnType = methodElement.getGenericReturnType();
if (returnType.isPublic() || returnType.getPackageName().equals(element.getPackageName())) {
AnnotationValueBuilder<Introspected> replaceIntrospected = AnnotationValue.builder(introspected, RetentionPolicy.RUNTIME);
replaceIntrospected.member("builderClass", new AnnotationClassValue<>(builderClassElement.getName()));
replaceIntrospected.member("builderClass", new AnnotationClassValue<>(returnType.getName()));
element.annotate(replaceIntrospected.build());
AnnotationMetadata methodMetadata = methodElement.getMethodAnnotationMetadata().getTargetAnnotationMetadata();

handleBuilder(
element,
context,
creatorMethod,
writePrefixes,
builderClassElement.getPrimaryConstructor().orElse(null),
builderClassElement.getDefaultConstructor().orElse(null),
builderClassElement,
builderClassElement.getTargetAnnotationMetadata(),
methodElement,
null,
returnType,
methodMetadata,
index,
targetPackage);
targetPackage
);
} else {
context.fail("Builder class not found on compilation classpath: " + builderClass.getName(), element);
context.fail("Builder return type is not public. The method must be static and accessible.", methodElement);
}
} else {
context.fail("When specifying the 'builder' member of @Introspected you must supply either a builderClass or builderMethod", element);
context.fail("Method " + builderMethod + "() specified by builderMethod not found. The method must be static and accessible.", element);
}
} else if (builderClass != null) {
ClassElement builderClassElement = context.getClassElement(builderClass.getName()).orElse(null);
if (builderClassElement != null) {
AnnotationValueBuilder<Introspected> replaceIntrospected = AnnotationValue.builder(introspected, RetentionPolicy.RUNTIME);
replaceIntrospected.member("builderClass", new AnnotationClassValue<>(builderClassElement.getName()));
element.annotate(replaceIntrospected.build());

handleBuilder(
element,
context,
creatorMethod,
writePrefixes,
builderClassElement.getPrimaryConstructor().orElse(null),
builderClassElement.getDefaultConstructor().orElse(null),
builderClassElement,
builderClassElement.getTargetAnnotationMetadata(),
index,
targetPackage);
} else {
context.fail("Builder class not found on compilation classpath: " + builderClass.getName(), element);
}
} else {
context.fail("When specifying the 'builder' member of @Introspected you must supply either a builderClass or builderMethod", element);
}
}

Expand Down Expand Up @@ -380,13 +420,18 @@ private void processElement(boolean metadata,
List<PropertyElement> beanProperties = ce.getBeanProperties(propertyElementQuery).stream()
.filter(p -> !p.isExcluded())
.toList();
Optional<MethodElement> constructorElement = ce.getPrimaryConstructor();
constructorElement.ifPresent(constructorEl -> {
if (ArrayUtils.isNotEmpty(constructorEl.getParameters())) {
writer.visitConstructor(constructorEl);
}
});
ce.getDefaultConstructor().ifPresent(writer::visitDefaultConstructor);
// unfortunately sometimes we don't see the Lombok transformations
// so assume if the class is annotated with Lombok builder we cannot
// access the constructor.
if (!ce.hasDeclaredAnnotation(ANN_LOMBOK_BUILDER)) {
Optional<MethodElement> constructorElement = ce.getPrimaryConstructor();
constructorElement.ifPresent(constructorEl -> {
if (ArrayUtils.isNotEmpty(constructorEl.getParameters())) {
writer.visitConstructor(constructorEl);
}
});
ce.getDefaultConstructor().ifPresent(writer::visitDefaultConstructor);
}

for (PropertyElement beanProperty : beanProperties) {
if (beanProperty.isExcluded()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@
import io.micronaut.inject.ast.MemberElement;
import io.micronaut.inject.ast.MethodElement;
import io.micronaut.inject.processing.ProcessingException;
import io.micronaut.inject.visitor.ElementPostponedToNextRoundException;
import io.micronaut.inject.visitor.TypeElementVisitor;
import io.micronaut.inject.visitor.VisitorContext;
import io.micronaut.inject.writer.AbstractBeanDefinitionBuilder;

import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.RoundEnvironment;
import javax.annotation.processing.SupportedOptions;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;
import java.io.IOException;
Expand Down Expand Up @@ -276,6 +278,15 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
error(originatingElement.element(), e.getMessage());
} catch (PostponeToNextRoundException e) {
postponedTypes.put(javaClassElement.getCanonicalName(), e.getErrorElement());
} catch (ElementPostponedToNextRoundException e) {
Object nativeType = e.getOriginatingElement().getNativeType();
if (nativeType instanceof JavaNativeElement jne) {
Element element = jne.element();
postponedTypes.put(javaClassElement.getCanonicalName(), element);
} else {
// should never happen.
throw e;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,11 @@ private IntrospectionBuilderData getBuilderData() {

AnnotationValue<Introspected.IntrospectionBuilder> builderAnn = getAnnotationMetadata().findAnnotation(Introspected.class)
.flatMap(a -> a.getAnnotation("builder", Introspected.IntrospectionBuilder.class)).orElse(null);
if (builderAnn != null) {
Class<?> builderClass = getAnnotationMetadata().classValue(Introspected.class, "builderClass").orElse(null);
if (builderClass != null) {
Class<?> builderClass = getAnnotationMetadata().classValue(Introspected.class, "builderClass").orElse(null);
if (builderAnn != null || builderClass != null) {
if (builderClass == null) {
throw new IntrospectionException("Introspection defines invalid builder member for type: " + getBeanType());
} else {
BeanIntrospection<Object> builderIntrospection = (BeanIntrospection<Object>) BeanIntrospection.getIntrospection(builderClass);
Collection<BeanMethod<Object, Object>> beanMethods = builderIntrospection.getBeanMethods();

Expand Down Expand Up @@ -520,8 +522,6 @@ private IntrospectionBuilderData getBuilderData() {
arguments.toArray(Argument.ZERO_ARGUMENTS)
);
}
} else {
throw new IntrospectionException("Introspection defines invalid builder member for type: " + getBeanType());
}
} else {
int constructorLength = constructorArguments.length;
Expand Down
1 change: 1 addition & 0 deletions test-suite/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,4 @@ test {
// Prevent scanning classes with missing classes
exclude '**/classnotfound/**'
}

Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.micronaut.test.lombok;

import static org.junit.jupiter.api.Assertions.assertEquals;

import io.micronaut.core.beans.BeanIntrospection;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
Expand All @@ -17,7 +19,19 @@ void testLombokBuilder() {
RobotEntity robotEntity = builder.with("name", "foo")
.build();

Assertions.assertEquals("foo", robotEntity.getName());
assertEquals("foo", robotEntity.getName());
}

@Test
void testLombokBuilder2() {
BeanIntrospection.Builder<MyEntity> builder = BeanIntrospection.getIntrospection(MyEntity.class)
.builder();
MyEntity.MyEntityBuilder builder1 = MyEntity.builder();
builder.with("name", "foo");
builder.with("id", "123");
MyEntity myEntity = builder.build();
assertEquals("foo", myEntity.getName());
assertEquals("123", myEntity.getId());
}

@Test
Expand All @@ -29,7 +43,7 @@ void testLombokBuilderWithInnerClasses() {
SimpleEntity simpleEntity = builder.with("id", id)
.build();

Assertions.assertEquals(id, simpleEntity.getId());
assertEquals(id, simpleEntity.getId());

BeanIntrospection<SimpleEntity.CompartmentCreationTimeIndexPrefix> innerClassIntrospection =
BeanIntrospection.getIntrospection(SimpleEntity.CompartmentCreationTimeIndexPrefix.class);
Expand All @@ -42,7 +56,7 @@ void testLombokBuilderWithInnerClasses() {
SimpleEntity.CompartmentCreationTimeIndexPrefix innerClassEntity =
innerClassBuilder.with("compartmentId", "c1").with("timeCreated", current).build();

Assertions.assertEquals("c1", innerClassEntity.getCompartmentId());
Assertions.assertEquals(current, innerClassEntity.getTimeCreated());
assertEquals("c1", innerClassEntity.getCompartmentId());
assertEquals(current, innerClassEntity.getTimeCreated());
}
}
19 changes: 19 additions & 0 deletions test-suite/src/test/java/io/micronaut/test/lombok/MyEntity.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.micronaut.test.lombok;


import io.micronaut.core.annotation.Introspected;
import lombok.Builder;
import lombok.Value;

@Introspected
@Value
@Builder
public class MyEntity {
public static final String NAME_INDEX = "name";

@lombok.NonNull
String id;

@lombok.NonNull
String name;
}

0 comments on commit 9f7af32

Please sign in to comment.