Support using response type keeper with KSP#4526
Support using response type keeper with KSP#4526Goooler wants to merge 13 commits intosquare:trunkfrom
Conversation
| if (qualifiedName == "kotlin.Any") { | ||
| qualifiedName = "java.lang.Object" | ||
| } |
There was a problem hiding this comment.
Object is kept in the annotation processor, so I patched the logic for the symbol processor. Seems we have no need to do so, I can address a new PR to remove them.
There was a problem hiding this comment.
Yeah I left it in the annotation processor because it was harmless. I'm fine filtering them out if you want to do that in both.
Goooler
changed the title
|
Is KSP 2 stable? I'm pretty content being entirely outside the KSP ecosystem, and try to use it in as few places as possible. I don't want to every be forced to do a Retrofit release because a new version of Kotlin and/or KSP came out. |
|
I believe so. I didn't test the processor on KSP1 as I see KSP1 support has been dropped by https://github.com/ZacSweers/kotlin-compile-testing/releases/tag/0.10.0 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Okay I'm comfortable adding this. We actually use a KSP one internally, not the annotation processing one. I will compare to ensure this meets our needs and then merge. |
Co-authored-by: Zongle Wang <wangzongler@gmail.com>
Co-authored-by: Zongle Wang <wangzongler@gmail.com>
| @@ -0,0 +1,17 @@ | |||
| package retrofit2.keeper | |||
There was a problem hiding this comment.
Copyright header please
| @@ -0,0 +1,80 @@ | |||
| package retrofit2.keeper | |||
There was a problem hiding this comment.
Copyright header please
| w.write("# $typeName\n") | ||
| for (referencedType in referencedTypes.sorted()) { | ||
| w.write("-keep,allowoptimization,allowshrinking,allowobfuscation class $referencedType\n") | ||
| w.write("$KEEP_RULE_PREFIX $referencedType\n") | ||
| } |
There was a problem hiding this comment.
Instead of pulling out the constant, lets pull out this little block into a function that writes the whole file.
| "all-http-methods", | ||
| "nesting", | ||
| "kotlin-suspend", |
There was a problem hiding this comment.
Hmm I'm not sure we have enough tests to necessitate such a robust mechanism (as opposed to just three functions with the input source and output rules).
There was a problem hiding this comment.
I would probably do an enum that's like
enum class Generator {
AnnotationProcessor {
override fun validate(source: String, rules: String) { .. }
},
Ksp {
override fun validate(source: String, rules: String) { .. }
},
;
abstract fun validate(source: String, rules: String)
}and then make that enum the class-level test parameter. Then each method is basically just
@Test fun stuff() {
generator.validate(
source = """
|stuff
""".trimMargin(),
rules = """
|stuff
""".trimMargin(),
)
}There was a problem hiding this comment.
I put the test fixtures under resources cause they deserve the code highlighting and navigation from IDEA.
There was a problem hiding this comment.
Done. But I would still suggest using parametermized tests to reduce boilerplates.
Goooler
force-pushed
the
response-type-keeper-ksp
branch
2 times, most recently
from
918ea68 to
26fee81
Compare
Goooler
force-pushed
the
response-type-keeper-ksp
branch
from
26fee81 to
5b6c1b5
Compare
…-ksp # Conflicts: # CHANGELOG.md
…-ksp # Conflicts: # retrofit-response-type-keeper/build.gradle
Goooler
force-pushed
the
response-type-keeper-ksp
branch
from
1cb0564 to
97bf8f8
Compare
This is on my "to do" list for this month. Lots going on, but I haven't forgotten about it. |
|
Thanks for the feedback! |
3 participants
CHANGELOG.md's "Unreleased" section has been updated, if applicable.