Skip to content

Conversation

@Goooler
Copy link
Contributor

@Goooler Goooler commented Oct 11, 2025


  • CHANGELOG.md's "Unreleased" section has been updated, if applicable.

Comment on lines +63 to +65
if (qualifiedName == "kotlin.Any") {
qualifiedName = "java.lang.Object"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Goooler changed the title Response type keeper ksp Support using response type keeper with KSP Oct 11, 2025
@JakeWharton
Copy link
Collaborator

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.

@Goooler
Copy link
Contributor Author

Goooler commented Oct 15, 2025

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

@chri1017

This comment has been minimized.

@JakeWharton

This comment has been minimized.

@Goooler
Copy link
Contributor Author

Goooler commented Oct 23, 2025

@JakeWharton
Copy link
Collaborator

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.

JakeWharton and others added 2 commits October 23, 2025 23:47
@@ -0,0 +1,17 @@
package retrofit2.keeper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright header please

@@ -0,0 +1,80 @@
package retrofit2.keeper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright header please

Comment on lines 73 to 76
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")
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of pulling out the constant, lets pull out this little block into a function that writes the whole file.

Comment on lines 46 to 48
"all-http-methods",
"nesting",
"kotlin-suspend",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(),
  )
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the test fixtures under resources cause they deserve the code highlighting and navigation from IDEA.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. But I would still suggest using parametermized tests to reduce boilerplates.

@Goooler Goooler force-pushed the response-type-keeper-ksp branch 2 times, most recently from 918ea68 to 26fee81 Compare October 24, 2025 04:56
@Goooler Goooler force-pushed the response-type-keeper-ksp branch from 26fee81 to 5b6c1b5 Compare October 27, 2025 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KSP support for retrofit-response-type-keeper

3 participants