Fix getJvmName for @JvmRecord data class properties by MariusVolkhart · Pull Request #2813 · google/ksp

MariusVolkhart · 2026-02-26T17:20:54Z

@JvmRecord data classes compile to Java records, whose component accessors use bare property names (e.g. name()) rather than bean-style getters (getName()). ResolverAAImpl.getJvmName unconditionally used JvmAbi.getterName() which always added the get prefix.

Add a check for the @JvmRecord annotation alongside the existing annotation class check, since both use bare property names as accessor names.

Fixes #2812

google-cla · 2026-02-26T17:21:10Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

MariusVolkhart · 2026-02-26T17:33:23Z

CLA violation is due to Claude Code. I didn't see anything in the CONTRIBUTING.md indicating agentic AI was not allowed, and wanted to be clear on the fact that it was used as part of this change. Please advise if you want me to make changes or are unable to accept the PR as a result. Thank you.

hfmehmed · 2026-03-05T13:55:16Z

To me it seems like the tool requires all the owners of the commit (you and anthropic agent) need to sign the CLA. Obviously, the agent itself can't sign it so probably to the time being the workaround is not commit code using the agent

jaschdoc

Hey thank you for opening the PR and apologies for the slow response. I left some comments that aim to improve the test suite and readability. Cheers!

jaschdoc · 2026-03-10T10:32:18Z

+@JvmRecord
+data class TestRecordClass(val x: Int, val y: String)
+// FILE: TestLibRecordClass.kt
+@JvmRecord
+data class TestLibRecordClass(val x: Int, val y: String)


How are these classes distinct except for the name? It's better if the classes are a bit more diverse. I'm thinking an approximation of a matrix of the following:

Classes with different number of parameters

Different property names

Classes extending other types

Classes with type parameters

Classes marked as JvmRecords that override a val declared in a non-annotation interface, e.g., interface Example { val x: Int }

Classes with different names (like you already have)

Classes with var and val properties

If you have type parameters and or normal parameters and a class that extends another type, then it's also a good idea to override some parameters and add some locally, e.g.,

interface Example<A> { val x: A } data class Ext<A, B>(override val x: A, val y: B) : Example<A>

Done. Records can't extend classes or have var constructor properties, so those cells became interface supertypes (incl. the generic override shape) and a body var without a backing field — noted in the test file. Also added an aliased import, @get:JvmName, and a library-module record. Expected names verified against javap.

jaschdoc · 2026-03-10T10:33:09Z

+class JvmNameRecordProcessor : AbstractTestProcessor() {
+    val results = mutableListOf<String>()
+    override fun toResult(): List<String> {
+        return results


This should be sorted to ensure results are reproducible.

Done — results are sorted now.

jaschdoc · 2026-03-10T11:33:35Z

+        if (containingClass?.classKind == ClassKind.ANNOTATION_CLASS ||
+            containingClass != null && containingClass.annotations.any {
+                it.annotationType.resolve().declaration.qualifiedName?.asString() == "kotlin.jvm.JvmRecord"
+            }
+        ) {
+            return name
+        }


I think we can improve the readability of this by extracting some variables so it reads

if (isAnnotationClass || isJvmRecord) { return name }

Additionally, we can shortcircuit the expensive type resolution by checking for the short name first in the lambda:

any { it.shortName.asString() == "JvmRecord" || it.annotationType.resolve().declaration.qualifiedName?.asString() == "kotlin.jvm.Record }

Lastly, maybe we can rename name to propertyName so it's clearer what the name. I know you didn't change it, but let's improve it.

Done, with one tweak: comparing source short names breaks on import aliases (import kotlin.jvm.JvmRecord as JR), so the check compares ClassIds on the analysis-api symbol instead — same as explictJvmName does for @JvmName just above in util.kt.

jaschdoc · 2026-03-10T11:34:32Z

+import org.junit.jupiter.api.parallel.ExecutionMode
+
+@Execution(ExecutionMode.SAME_THREAD)
+class KSPAA17Test : AbstractKSPAATest() {


Did you try adding the test to the existing test suite?

Yes — a // JVM_TARGET: 17 directive in the testData collides with the hardcoded default in AbstractKSPTest ("Too many values passed to JVM_TARGET: [1.8, 17]"); that's why the separate class existed. Removed the hardcoded default so file directives work, and the test now lives in KSPAATest. Isolated in its own commit — easy to drop if you'd rather keep this PR minimal.

jaschdoc · 2026-03-10T11:46:50Z

+                    cls.getAllProperties().map {
+                        "(${it.getter?.let { resolver.getJvmName(it) }}, " +
+                            "${it.setter?.let { resolver.getJvmName(it) }})"
+                    }.toList().joinToString()


It's more stable if you use resolver.getSymbolsWithAnnotation("kotlin.jvm.JvmRecord") and filter for class declaration instances.
It might also be a bit cleaner if you flatmap over getAllProperties and then return a list of size at most two, that contains the getter and setter names which you can the call joinToString on, e.g.,

getAllProperties().flatMap { // create a list that contains the jvm getter name if it exists and the setter name if it exists }

The names should probably also be qualified / prefixed with the class names.

Done. One wrinkle: getSymbolsWithAnnotation only returns current-compilation symbols, so the library-module record is looked up by name. That path exposed a real bug — @JvmRecord isn't retained in class files, so library records were reported as getW. Fixed in a separate commit by recognizing the java.lang.Record supertype instead.

jaschdoc · 2026-06-12T10:03:23Z

@MariusVolkhart, just pinging to see if you still want to contribute :)

@JvmRecord data classes compile to Java records, whose component accessors use bare property names (e.g. name()) rather than bean-style getters (getName()). ResolverAAImpl.getJvmName unconditionally used JvmAbi.getterName() which always added the get prefix. Add a check for the @JvmRecord annotation alongside the existing annotation class check, since both use bare property names as accessor names. Fixes google#2812

The KSPAA17Test class existed only to run one test at JVM target 17, because a // JVM_TARGET: directive in a testData file collides with the hardcoded JVM_TARGET default in AbstractKSPTest: the test framework merges default directives with file-level ones instead of overriding, failing with "Too many values passed to JVM_TARGET: [1.8, 17]". Remove the hardcoded default and fall back to JvmTarget.DEFAULT at the point of consumption in AbstractKSPAATest, which is the same value the directive used to inject. Any test can now pick its JVM target with a one-line directive instead of a dedicated test class, and the record test moves into KSPAATest alongside the other jvmName tests.

@get

The @JvmRecord annotation is not retained in class files, so the annotation-based check never matches a record class consumed from a dependency jar, and accessors were reported with the bean-style get prefix. The analysis API exposes java.lang.Record as a supertype of deserialized record classes, so recognize library records by that supertype, keeping the annotation check for source classes where the implicit supertype is not present. Covering this required two test-infrastructure fixes: library-module compilation now forwards the module's JVM target (records need 16+), and the compiler invocation now fails the test on a non-OK exit code instead of silently producing no class files, which previously surfaced as misleading downstream resolution failures. The testData also gains an @get:JvmName record property documenting that an explicit JvmName takes precedence over record accessor naming.

MariusVolkhart requested review from jaschdoc and ting-yuan as code owners February 26, 2026 17:20

MariusVolkhart force-pushed the mv/record branch from 6cd15c8 to 86affbb Compare February 26, 2026 17:40

MariusVolkhart force-pushed the mv/record branch from 86affbb to 112045f Compare March 5, 2026 15:01

jaschdoc reviewed Mar 10, 2026

View reviewed changes

jaschdoc added awaiting author response and removed awaiting contributor response labels Mar 27, 2026

MariusVolkhart requested a review from troelsbjerre as a code owner June 12, 2026 17:27

MariusVolkhart added 4 commits June 12, 2026 13:31

fixup! Fix getJvmName for @JvmRecord data class properties

43df4e0

MariusVolkhart force-pushed the mv/record branch from 410b0f9 to dbcb5df Compare June 12, 2026 17:52

Conversation

MariusVolkhart commented Feb 26, 2026

Uh oh!

google-cla Bot commented Feb 26, 2026

Uh oh!

MariusVolkhart commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hfmehmed commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaschdoc left a comment

Choose a reason for hiding this comment

Uh oh!

jaschdoc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MariusVolkhart Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaschdoc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MariusVolkhart Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

jaschdoc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MariusVolkhart Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaschdoc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MariusVolkhart Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaschdoc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MariusVolkhart Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaschdoc commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MariusVolkhart commented Feb 26, 2026 •

edited

Loading

hfmehmed commented Mar 5, 2026 •

edited

Loading

MariusVolkhart Jun 12, 2026 •

edited

Loading

MariusVolkhart Jun 12, 2026 •

edited

Loading

MariusVolkhart Jun 12, 2026 •

edited

Loading

MariusVolkhart Jun 12, 2026 •

edited

Loading