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
Fix mapping with Kotlin default arguments #1302
Conversation
Add support for mapping to Kotlin objects with default parameters when not all arguments are specified in DefaultInputObjectMapper.
if (parameter.isOptional) { | ||
continue | ||
} else if (parameter.type.isMarkedNullable) { | ||
parametersByName[parameter] = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This preserves the existing behavior (as exercised in the tests), but I am not sure this is desirable. It essentially means that we fill in nulls for classes like this when the argument isn't explicitly supplied:
data class Foo(val nullableParam: String?)
I would think that if that were the desired behavior, the user would write the class like:
data class Foo(val nullableParam: String? = null)
Anyway, I've left it here so as to not break the existing behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this has been a point of contention in the codegen plugin as well. But I agree with the decision to preserve existing behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
if (parameter.isOptional) { | ||
continue | ||
} else if (parameter.type.isMarkedNullable) { | ||
parametersByName[parameter] = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this has been a point of contention in the codegen plugin as well. But I agree with the decision to preserve existing behavior.
Add support for mapping to Kotlin objects with default parameters when not all arguments are specified in DefaultInputObjectMapper.