Skip to content
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

Compatibility with non-JavaBeans-style property accessors #1601

Open
danielsonjh opened this issue Sep 1, 2018 · 5 comments
Open

Compatibility with non-JavaBeans-style property accessors #1601

danielsonjh opened this issue Sep 1, 2018 · 5 comments
Labels
enhancement up-for-grabs Issues targeted for community contribution

Comments

@danielsonjh
Copy link

The following docs from AutoValue and Immutables seem to indicate that both libraries do not recommend using the JavaBeans-style property accessors (getProperty(), isBoolean(), etc)
https://github.com/google/auto/blob/master/value/userguide/index.md
https://immutables.github.io/

Both libraries do support JavaBeans-style accessors to some degree however
https://github.com/google/auto/blob/master/value/userguide/howto.md#beans
https://immutables.github.io/style.html

In Mapstruct 1.3.0.Beta1 only JavaBeans-style properties are supported.
To support this new convention, Mapstruct will need an option to treat all methods as potential properties.
With no differentiation between standard methods and property getters, it seems that Mapstruct either needs:

  1. Mapped classes to only have property getters as methods.
  2. An annotation to allow Mapstruct to ignore certain methods when not using JavaBeans-style conventions.

Option 1 is up to the developer, and for Option 2, there is already an annotation that can do this, @Mapping(target = "somemethod", ignore = true)

@filiphr
Copy link
Member

filiphr commented Sep 14, 2018

@danielsonjh I think that someone else requested this as well, but I am not sure if it was within an issue.

As you noticed Option 2 is already in there. As for Option 1 it can be achieved by using a custom AccessorNamingStrategy and it is not that difficult to write for your own use. I don't see why such an opt in strategy won't be acceptable for us (if someone wants to contribute it). The only thing is that one needs to be really careful as then all methods would be considered as properties.

@filiphr filiphr added enhancement up-for-grabs Issues targeted for community contribution labels Sep 14, 2018
@sjaakd
Copy link
Contributor

sjaakd commented Sep 22, 2018

and Immutables already works out-of-the box with fluent-accessors, right? #1608 reports something similar for Lombok..

@filiphr
Copy link
Member

filiphr commented Sep 22, 2018

Everything works out-of-the box with fluent setters. There are no fluent getters actually. As fluent implies that you get the same object back and for getters you get something else.

@debug-desperado
Copy link

With immutables I think it makes the most sense to consider all methods as properties, and then have the user exclude derived, lazy, and check methods if they cause problems (@Value.Derived, @Value.Lazy, @Value.Check). Those won't be on the builders, but I'm not sure how MapStruct could use that information.

Looking briefly over the code, if ImmutablesAccessorNamingStrategy extended CustomAccessorNamingStrategy instead of DefaultAccessorNamingStrategy, wouldn't that have this effect?

@filiphr
Copy link
Member

filiphr commented Aug 18, 2019

@debug-desperado CustomAccessorNamingStrategy is within the tests. In any case even if that is done it won't have the effect that you explained.

As I said in #1601 (comment). This one is marked as up for grabs, so if someone from the community is willing to give it a go we are more than happy to help out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement up-for-grabs Issues targeted for community contribution
Projects
None yet
Development

No branches or pull requests

4 participants