-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add support for Lombok #92
base: main
Are you sure you want to change the base?
Conversation
@filiphr Can I get some review? |
Hey @KENNYSOFT, thanks for the PR, I've been a bit busy and haven't had time to look into it yet. Will look into it once I have some time. On first glance I do not like the required dependency on the Lombok plugin, we are importing |
9cf86bf
to
5762b8d
Compare
@KENNYSOFT what happens if the Lombok plugin is not available? I see that |
OK, I thought the main concerns about the class
I've tested with the (bundled) Lombok plugin disabled, it works without any crashes.
I'll soon attach the Minimal Reproducible Example. Still do not have any idea about the test code. |
Minimal Reproducible Examplebuild.gradleplugins {
id 'java'
id 'io.freefair.lombok' version '8.0.1'
}
java {
toolchain {
languageVersion = JavaLanguageVersion.of(17)
}
}
repositories {
mavenCentral()
}
dependencies {
implementation 'org.mapstruct:mapstruct:1.5.4.Final'
annotationProcessor 'org.mapstruct:mapstruct-processor:1.5.4.Final'
} Content.javaimport lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import java.time.LocalDateTime;
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
@Getter
public class Content {
private Long no;
private String title;
private String description;
private LocalDateTime createdAt;
} ContentCreateRequest.javapublic record ContentCreateRequest(String title, String description) {
} ContentRequestMapper.javaimport org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.ReportingPolicy;
@Mapper(unmappedSourcePolicy = ReportingPolicy.ERROR, unmappedTargetPolicy = ReportingPolicy.ERROR)
public interface ContentRequestMapper {
@Mapping(target = "no", expression = "java(null)")
@Mapping(target = "createdAt", expression = "java(null)")
Content toEntity(ContentCreateRequest request);
} ResultWith Lombok extension: Without: |
Perhaps it works because the plugin is bundled and thus the class is always there. I'll try it out and see what happens. My main concern was that there will be some kind of an exception when we do the instance of check or when the IDE boots with the MapStruct plugin. |
@KENNYSOFT I am not sure how you tested this, but when I try this out with the Lombok plugin disabled on IntelliJ IDEA 2023.1.1 Preview (Ultimate Edition) then I get:
However, I think that I found a nice and neat way to support this. Have a look at my commit. I also adapted some other places in order to properly go to the field. Let me know what you think |
Yeah, cool for extracting, but I cannot find that what behavior is changed by TestDtoOnlyGetter.javaimport lombok.Getter;
@Getter
public class TestDtoOnlyGetter {
private String nameg;
} TestDtoWithBuilder.javaimport lombok.Builder;
@Builder
public class TestDtoWithBuilder {
private String nameb;
} TestDtoWithSetter.javaimport lombok.Setter;
@Setter
public class TestDtoWithSetter {
private String names;
} TestMapper.javaimport org.mapstruct.Mapper;
import org.mapstruct.Mapping;
@Mapper
public interface TestMapper {
@Mapping(target = "nameb", source = "name")
TestDtoWithBuilder resolveTargetBuilder(String name);
@Mapping(target = "names", source = "name")
TestDtoWithSetter resolveTargetSetter(String name);
@Mapping(target = "names", source = "dto.nameg")
TestDtoWithSetter resolveSourceGetter(TestDtoOnlyGetter dto);
}
|
Good point @KENNYSOFT. I was testing this initially with IntelliJ 2021.2 and there it didn't work. It only started working when I did that resolvement. However, I tried it now on 2023.1.1 Preview and everything works properly. Even when the parameter comes from the constructor. What exactly were your initial changes fixing @KENNYSOFT? |
Add support for Lombok; especially
@AllArgsConstructor
and@RequiredArgsConstructor
which makes we can omit the constructor parameter, so the reference to not work.Also fixes: if some constructor is
protected
(e.g.,@NoArgsConstructor(access = AccessLevel.PROTECTED)
from Lombok), it also should be ignored since the mapper would not extend the target class.Want to make some tests, but cannot find related examples in this repository. Still looking around to search for some good examples.
Notes:
Since this plugin builds against IntelliJ IDEA 2020.1 which does not bundle the Lombok plugin, the dependency declaration should specify a version of it. I've used
0.34-2020.1
and0.34-2020.2
which is the latest version respectively for the CI. Configurable by set environment variableLOMBOK_PLUGIN_VERSION
.By the way, I thought the plugin is bundled since IntelliJ IDEA 2020.3 but it seems broken. It succeeded with 2020.3.4 but not with 2020.3, so in this case, I just set it to
0.32-EAP
which is the only version supports IntelliJ IDEA 2020.3. Please let me know if you mind. (But I don't know the clear solution until now 😉)Note that 'Lombook' is not a typo, the plugin id really states.
Like Kotlin, the Lombok plugin is optional. The dependency is only for the
instanceof
check, so even no extra configuration is needed. (But the file is added since the platform requires existence.)IntelliJ Platform Plugin documentation guides to name the additional plugin descriptor files as "myPluginId-$NAME$.xml" so I've followed the standard.
Reference: https://plugins.jetbrains.com/docs/intellij/plugin-dependencies.html#optional-plugin-dependencies
BONUS: Fixed plugin repository in README, added
ideaVersion
2021.3 to the CI matrix.