-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Lombok Annotations #1297
Support Lombok Annotations #1297
Comments
Hi @hetzijzo, Thanks for reporting the issue we'll take a look. |
Unfortunately, we cannot fix this issue until support for Lombok annotations and types are added to the rewrite framework. |
Good morning @pway99, is there any estimate to have the problem with the Lombok library solved? |
Good Morning @josemariavillar, It's a tricky problem that will require a significant effort; at this time, we do not have an estimate. |
I'm trying to figure out to find a workaround. The case is the following tree of recipes:
I have tried the following workarounds with little success
Are there any other options? |
…es having a null or unknown type. (issue #1297)
Hello @hetzijzo, That's a good question, I am not aware of a good workaround for Lombok issues at this time. That said in the spirit of do no harm each recipe should do its best to guard against making a breaking change due to invalid type information. I took another look at the Are you able to provide some more information or create an issue for the |
…es having a null or unknown type. (issue #1297)
reverted accidental commit to the wrong branch |
I have the following two Java files in a project import lombok.data;
@Data
class MyObject {
String someField;
} import static org.mockito.Mockito.when;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@ExtendWith(MockitoExtension.class)
class MyObjectTest {
@Mock
MyObject myObject;
@Test
void test() {
when(myObject.getSomeField()).thenReturn("testValue");
......
}
} Now when I run the following command it will lead to a unwanted deleted static import of
The execution of openrewrite leads to the following change:
|
Hi @hetzijzo, Thanks for the additional info on the Please don't hesitate to share any other issues you may find :) |
Hi @hetzijzo ! I guess full support for Lombok is some way off, but perhaps there's something we can do already to prevent the imports from being removed; do you have a minimal example where you see imports being removed? That way we can try to capture that in a JUnit test and iterate more quickly. |
Running the following command
On the folliwing Java file: package nl.fictive.bsp.capability.serviceinzicht.service.client.notificatiebeheer;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import ch.qos.logback.classic.Level;
import java.util.Date;
import java.util.UUID;
import nl.fictive.bsp.capability.serviceinzicht.service.application.mortgageinquiry.cleaning.CleanupInquiryEvent;
import nl.fictive.bsp.capability.serviceinzicht.util.logging.MemoryAppender;
import nl.fictive.bsp.capability.notificatiebeheer.api.NotificatiesVersturenApi;
import nl.fictive.bsp.capability.notificatiebeheer.api.model.Notificatie;
import nl.fictive.bsp.core.identity.Bank;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.client.RestClientException;
import org.wildfly.common.Assert;
@ExtendWith(MockitoExtension.class)
public class NotificatiebeheerServiceClientTest {
@InjectMocks private NotificatiebeheerServiceClient client;
@Mock private NotificatiesVersturenApi notificatiesVersturenApi;
@Captor ArgumentCaptor<Notificatie> notificatie;
@Test
public void notifyByMail_ExceptionThrown() {
// Arrange
final MemoryAppender memoryAppender = new MemoryAppender(NotificatiebeheerServiceClient.class);
UUID uuid = UUID.randomUUID();
CleanupInquiryEvent event =
CleanupInquiryEvent.builder()
.batchRun(new Date())
.message("TestInquiryEvent")
.successful(true)
.inquiryId(uuid)
.build();
doThrow(new RestClientException("test error message"))
.when(notificatiesVersturenApi)
.addNotificatie(eq(Bank.SOME.name().toLowerCase()), any(Notificatie.class));
ReflectionTestUtils.setField(client, "emailAddress", "[email protected]");
// Act
client.notifyByMail(event);
// Assert
assertEquals(1, memoryAppender.countEventsForLogger("NotificatiebeheerServiceClient"));
Assert.assertTrue(
memoryAppender.contains(
"Something went wrong while sending e-mail notification to [email protected]",
Level.ERROR));
}
@Test
public void notifyByMail() {
// Arrange
UUID uuid = UUID.randomUUID();
CleanupInquiryEvent event =
CleanupInquiryEvent.builder()
.batchRun(new Date())
.message("TestInquiryEvent")
.successful(true)
.inquiryId(uuid)
.build();
// Act
client.notifyByMail(event);
// Assert
verify(notificatiesVersturenApi).addNotificatie(eq("SOME"), notificatie.capture());
assertTrue(notificatie.getValue().getNotificatieTypes().contains("EMAIL"));
assertTrue(
notificatie.getValue().getOnderwerp().equals("Mortgage Inquiry Cleanup Scheduler Warning"));
assertTrue(
notificatie
.getValue()
.getInhoud()
.startsWith("Voor beheer van"));
assertTrue(notificatie.getValue().getInhoud().contains(uuid.toString()));
assertTrue(notificatie.getValue().getInhoud().contains("SCHEDULED_CLEANUP_INQUIRY"));
assertTrue(notificatie.getValue().getInhoud().contains("TestInquiryEvent"));
}
} Results with the following in the log: [WARNING] Changes have been made to src\test\java\nl\fictive\bsp\capability\serviceinzicht\service\client\notificatiebeheer\NotificatiebeheerServiceClientTest.java by:
[WARNING] nl.fictive.bsp.UpgradeBspCoreLatest
[WARNING] nl.fictive.bsp.UpgradeBspCore7
[WARNING] nl.fictive.bsp.UpgradeBspCore6
[WARNING] org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_6
[WARNING] org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_5
[WARNING] org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_4
[WARNING] org.openrewrite.java.testing.junit5.JUnit5BestPractices
[WARNING] org.openrewrite.java.testing.junit5.StaticImports
[WARNING] org.openrewrite.java.UseStaticImport: {methodPattern=org.junit.jupiter.api.Assertions *(..)}
[WARNING] org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic And the file is changed like this: package nl.fictive.bsp.capability.serviceinzicht.service.client.notificatiebeheer;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import ch.qos.logback.classic.Level;
import java.util.Date;
import java.util.UUID;
import nl.fictive.bsp.capability.serviceinzicht.service.application.mortgageinquiry.cleaning.CleanupInquiryEvent;
import nl.fictive.bsp.capability.serviceinzicht.util.logging.MemoryAppender;
import nl.fictive.bsp.capability.notificatiebeheer.api.NotificatiesVersturenApi;
import nl.fictive.bsp.capability.notificatiebeheer.api.model.Notificatie;
import nl.fictive.bsp.core.identity.Bank;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.client.RestClientException;
import org.wildfly.common.Assert;
@ExtendWith(MockitoExtension.class)
class NotificatiebeheerServiceClientTest {
@InjectMocks
private NotificatiebeheerServiceClient client;
@Mock
private NotificatiesVersturenApi notificatiesVersturenApi;
@Captor
ArgumentCaptor<Notificatie> notificatie;
@Test
void notifyByMail_ExceptionThrown() {
// Arrange
final MemoryAppender memoryAppender = new MemoryAppender(NotificatiebeheerServiceClient.class);
UUID uuid = UUID.randomUUID();
CleanupInquiryEvent event =
CleanupInquiryEvent.builder()
.batchRun(new Date())
.message("TestInquiryEvent")
.successful(true)
.inquiryId(uuid)
.build();
doThrow(new RestClientException("test error message"))
.when(notificatiesVersturenApi)
.addNotificatie(eq(Bank.SOME.name().toLowerCase()), any(Notificatie.class));
ReflectionTestUtils.setField(client, "emailAddress", "[email protected]");
// Act
client.notifyByMail(event);
// Assert
assertEquals(1, memoryAppender.countEventsForLogger("NotificatiebeheerServiceClient"));
Assert.assertTrue(
memoryAppender.contains(
"Something went wrong while sending e-mail notification to [email protected]",
Level.ERROR));
}
@Test
void notifyByMail() {
// Arrange
UUID uuid = UUID.randomUUID();
CleanupInquiryEvent event =
CleanupInquiryEvent.builder()
.batchRun(new Date())
.message("TestInquiryEvent")
.successful(true)
.inquiryId(uuid)
.build();
// Act
client.notifyByMail(event);
// Assert
verify(notificatiesVersturenApi).addNotificatie(eq("SOME"), notificatie.capture());
assertTrue(notificatie.getValue().getNotificatieTypes().contains("EMAIL"));
assertTrue(
notificatie.getValue().getOnderwerp().equals("Mortgage Inquiry Cleanup Scheduler Warning"));
assertTrue(
notificatie
.getValue()
.getInhoud()
.startsWith("Voor beheer van some-service-inzicht"));
assertTrue(notificatie.getValue().getInhoud().contains(uuid.toString()));
assertTrue(notificatie.getValue().getInhoud().contains("SCHEDULED_CLEANUP_INQUIRY"));
assertTrue(notificatie.getValue().getInhoud().contains("TestInquiryEvent"));
}
} It misses the import of |
@timtebeek I hope this helps a little |
And just to be sure; the release this morning hasn't helped right? |
I tried the command with all the latest components released this morning with the same result. |
@hetzijzo I wonder if this example has anything to do with Lombok. In the provided example class there are no Lombok annotations (or imports). The only unusual thing I can see is that Can you check if the problem is due to this import? Is the full source code (with all dependencies) available on GitHub or can you provide a reproducer? That would really help to get this issue fixed. |
It's been a long time coming but I think I have an approach that will work for this: |
After further experimentation we've identified a number of shortcomings in the current lombok support. Here's roughly what still needs to happen:
|
To give a bit of a status update; Anshuman has been instrumental in pushing this forward; trying things out and pushing fixes to Lombok as seen here:
What's left now is to add support for Java 8 and 21, and then enable that by default, before we can close out this issue. 🤞🏻 |
org.openrewrite.java.cleanup.ExplicitInitialization
andorg.openrewrite.java.cleanup.UseDiamondOperator
removes the argument type when it's a lombok.val variable.val products = new ArrayList<Product>();
val products = new ArrayList<>();
which is incorrect and doesn't compile anymore.
The text was updated successfully, but these errors were encountered: