Add KotlinPropertyNameAsImplicitName option #686
Conversation
k163377
force-pushed
the
feat/kotlin-name
branch
from
e97b2ce
to
5b65f56
Compare
| /** | ||
| * By enabling this feature, the property name on Kotlin will be used as the getter name. | ||
| * | ||
| * By default, the name based on the getter name on the JVM is used as the accessor name. | ||
| * This name may be different from the parameter/field name, in which case serialization results | ||
| * may be incorrect or annotations may malfunction. | ||
| * See [jackson-module-kotlin#630] for details. | ||
| * | ||
| * By enabling this feature, such malfunctions will not occur. | ||
| * | ||
| * On the other hand, enabling this option increases the amount of reflection processing, | ||
| * which may result in performance degradation for both serialization and deserialization. | ||
| * In addition, the adjustment of behavior using get:JvmName is disabled. | ||
| */ | ||
| UseKotlinPropertyNameForGetter(enabledByDefault = false); |
There was a problem hiding this comment.
@cowtowncoder
I would like to get a review as I am not sure about the option name and description.
There was a problem hiding this comment.
One quick question: is "property" first-class concept in Kotlin? If not, maybe it'd be "field", but I assume it is.
But I guess I am not sure what "use property name for getter" actually means... I mean, getter in Jackson specifically means method use to access value of a logical property to serialize. It has name specified by class file, and does not change.
Name of logical property does change tho. So maybe naming here is bit confusing, at least to me.
There was a problem hiding this comment.
is "property" first-class concept in Kotlin?
I think we can call it a first-class concept in this regard.
On Kotlin, fields and accessors are defined for properties.
https://kotlinlang.org/docs/properties.html
So maybe naming here is bit confusing, at least to me.
I don't know if this is a good name, but at least this is how I would describe the internal behavior directly.
If the emphasis is on naming on Jackson, would it be UseKotrinPropertyNameForGetterImplicitName?
However, it doesn't seem like a good name since we may not use the findImplicitPropertyName function forever.
Also, the name seems too long compared to the traditional KotlinFeature.
There was a problem hiding this comment.
I just did not understand what "for getter" means. But yes, if it changes "implicit [property] name" in context of serialization, I can see how "....ForGetterImplicitName" could work.
Does this not affect deserialization? Otherwise just something like "UseKotlinNameAsImplicitName" would make more sense to me.
There was a problem hiding this comment.
I just did not understand what "for getter" means.
I wanted to express that it does not affect parameters, fields, or setters, but only getters.
Currently we are already using names on Kotlin for parameter names, but that is not affected by this option.
Does this not affect deserialization?
If there are annotations for deserialization on the getter, it will affect the behavior, but basically it should only affect serialization.
There was a problem hiding this comment.
Right. I think referring to serialization makes more sense because getter really is physical thing (Method) used to access a property for purpose of (de)serialization. So you can't really change name of getter, but you change name of property for (de)serialization. Although, yes, obtained from getter method.
Still, ultimately this is your decision: I just offer my suggestions. :)
There was a problem hiding this comment.
Pushed for correction.
aa7217f
k163377
changed the title
Added option to use property names in Kotlin as getter names.
This resolves #630.