-
Notifications
You must be signed in to change notification settings - Fork 657
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
Allow mapping scalars to simple generic types #6158
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
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.
LGTM.
2 possible improvements:
- Make it a proper parser for Kotlin
type
(unlocksdynamic
, function types, variance, wildcards, type annotations, etc...)
2. recognize some frequent auto imports (donekotlin.
,kotlin.collections.
)
) | ||
} else { | ||
mapOf( | ||
"Date" to ScalarInfo("java.util.Date"), | ||
"URL" to ScalarInfo("kotlin.String", ExpressionAdapterInitializer("com.example.UrlAdapter")), | ||
"ID" to ScalarInfo("kotlin.Long"), | ||
"String" to ScalarInfo("kotlin.String", ExpressionAdapterInitializer("com.example.MyStringAdapter()")), | ||
"ListOfString" to ScalarInfo("kotlin.collections.List<kotlin.String?>"), |
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.
That is quite the mouthful. I guess List<String?>
doesn't work here?
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.
Just tried to make sure, but no, ClassName.bestGuess()
expects a fully qualified name. I'll see if I can add a few shortcuts for String
, List
, etc.
private fun String.withPackage(): String { | ||
return if (this in commonKotlinTypes) { | ||
"kotlin.${this}" | ||
} else if (this in commonKotlinCollectionsTypes) { | ||
"kotlin.collections.${this}" | ||
} else { | ||
this | ||
} | ||
} |
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.
👍
See #6156 and #3243. This is far from perfect but unlocks a few cases.