Skip to content

#159 support fluent setters with super types (e.g. lomboks @SuperBuilder) #161

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.

Sign up for GitHub

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

Jump to bottom
Merged
filiphr merged 4 commits into from
Mar 24, 2024
Merged

#159 support fluent setters with super types (e.g. lomboks @SuperBuilder) #161

filiphr merged 4 commits into from
Mar 24, 2024

Conversation

@thunderhook
Copy link
Contributor

@thunderhook thunderhook commented Nov 18, 2023 •
edited by filiphr
Loading

I hope this change has no unwanted side effects. It checks whether a fluent setters return type also can be assigned to one of its super types to support classes like this:

public static abstract class TargetBuilder<C extends Target, B extends TargetBuilder<C, B>> {

            private String testName;

            public B testName(String testName) {
                this.testName = testName;
                return self();
            }
            // ...
}

Fixes #159

@thunderhook thunderhook requested a review from filiphr November 18, 2023 22:13
@filiphr
Copy link
Member

filiphr commented Nov 26, 2023

Check my comment in #159 (comment). Is the test code working even properly with the MapStruct processor. We don't really support @SuperBuilder from Lombok, so the fix here will be a bit misleading.

@thunderhook thunderhook mentioned this pull request Feb 11, 2024
Closed
@thunderhook
Copy link
Contributor Author

thunderhook commented Mar 14, 2024

@filiphr Could you please have another look? I think it is now valid to support super types like this in the IDE plugin.
The example using in the test has the same structure as provided in mapstruct/mapstruct#3524, see https://github.com/mapstruct/mapstruct/pull/3542/files

filiphr
filiphr reviewed Mar 16, 2024
View reviewed changes
Copy link
Member

@filiphr filiphr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @thunderhook, I've added some small comments

src/main/java/org/mapstruct/intellij/util/MapstructUtil.java Outdated
Comment on lines 224 to 225
return Arrays.stream( returnType.getSuperTypes() )
.anyMatch( superType -> isAssignableFrom( psiType, superType ) );
Copy link
Member

@filiphr filiphr Mar 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to create a stream and then use anyMatch. Let's just iterate through the returnType.getSuperTypes() and do a check in the for loop.

src/main/java/org/mapstruct/intellij/util/MapstructUtil.java Outdated

private static boolean isAssignableFromReturnTypeOrSuperTypes(PsiType psiType, @Nullable PsiType returnType) {

if ( returnType == null ) {
Copy link
Member

@filiphr filiphr Mar 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The returnType cannot be null because in isFluentSetter we are checking method.getReturnType() != null.

@thunderhook
Copy link
Contributor Author

thunderhook commented Mar 17, 2024

@filiphr Thanks for the feedback. I have made the requested changes.

@filiphr filiphr merged commit 27c6cfc into main Mar 24, 2024
@filiphr filiphr deleted the 159 branch March 24, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@filiphr filiphr filiphr left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Code completion doesn't work with lombok @SuperBuilder annotation

3 participants

@thunderhook @filiphr