From 05f6ed40f17305de43b4fcdac03d43602389c895 Mon Sep 17 00:00:00 2001 From: Fabian Mastenbroek Date: Thu, 19 Jul 2018 13:45:50 +0200 Subject: [PATCH] Don't generate setters for trivial private property setters This change will prevent the compiler for generating Java bytecode for private property setters that are trivial. Since Kotlin uses direct field access for private properties, it will result in the private setter never been used and since it cannot be accessed by any other class without reflection, the setter cannot be covered by code coverage tools. See https://youtrack.jetbrains.com/issue/KT-20344 for the related YouTrack issue. --- .../jetbrains/kotlin/codegen/PropertyCodegen.java | 6 ++++++ .../codegen/dumpDeclarations/classMembers.json | 2 -- .../jetbrains/kotlin/codegen/PropertyGenTest.java | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java index 0cf157089a1..be2f35dfee0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java @@ -210,6 +210,12 @@ public class PropertyCodegen { return !isDefaultAccessor; } + // Non-private properties with private setter should not be generated for trivial properties + // as the class will use direct field access instead + if (accessor != null && accessor.isSetter() && Visibilities.isPrivate(descriptor.getSetter().getVisibility())) { + return !isDefaultAccessor; + } + return true; } diff --git a/compiler/testData/codegen/dumpDeclarations/classMembers.json b/compiler/testData/codegen/dumpDeclarations/classMembers.json index e959e87e511..a821b9882c0 100644 --- a/compiler/testData/codegen/dumpDeclarations/classMembers.json +++ b/compiler/testData/codegen/dumpDeclarations/classMembers.json @@ -40,13 +40,11 @@ {"visibility": "internal", "declaration": "final fun (: kotlin.String): kotlin.Unit", "name": "setInternalVar$test_module", "desc": "(Ljava/lang/String;)V"}, {"visibility": "internal", "declaration": "final lateinit var internalVarPrivateSet: kotlin.String", "name": "internalVarPrivateSet", "desc": "Ljava/lang/String;"}, {"visibility": "internal", "declaration": "final fun (): kotlin.String", "name": "getInternalVarPrivateSet$test_module", "desc": "()Ljava/lang/String;"}, - {"visibility": "private", "declaration": "final fun (: kotlin.String): kotlin.Unit", "name": "setInternalVarPrivateSet", "desc": "(Ljava/lang/String;)V"}, {"visibility": "protected", "declaration": "final lateinit var protectedVar: kotlin.String", "name": "protectedVar", "desc": "Ljava/lang/String;"}, {"visibility": "protected", "declaration": "final fun (): kotlin.String", "name": "getProtectedVar", "desc": "()Ljava/lang/String;"}, {"visibility": "protected", "declaration": "final fun (: kotlin.String): kotlin.Unit", "name": "setProtectedVar", "desc": "(Ljava/lang/String;)V"}, {"visibility": "protected", "declaration": "final lateinit var protectedVarPrivateSet: kotlin.String", "name": "protectedVarPrivateSet", "desc": "Ljava/lang/String;"}, {"visibility": "protected", "declaration": "final fun (): kotlin.String", "name": "getProtectedVarPrivateSet", "desc": "()Ljava/lang/String;"}, - {"visibility": "private", "declaration": "final fun (: kotlin.String): kotlin.Unit", "name": "setProtectedVarPrivateSet", "desc": "(Ljava/lang/String;)V"}, {"visibility": "private", "declaration": "final lateinit var privateVar: kotlin.Any", "name": "privateVar", "desc": "Ljava/lang/Object;"}, {"visibility": "public", "declaration": "constructor ClassWithLateinit()", "name": "", "desc": "()V"} ] diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/PropertyGenTest.java b/compiler/tests/org/jetbrains/kotlin/codegen/PropertyGenTest.java index 579b45cb27f..7e318263780 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/PropertyGenTest.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/PropertyGenTest.java @@ -124,9 +124,10 @@ public class PropertyGenTest extends CodegenTestCase { getFoo.setAccessible(true); assertTrue((getFoo.getModifiers() & Modifier.PROTECTED) != 0); assertEquals(349, getFoo.invoke(instance)); - Method setFoo = findDeclaredMethodByName(aClass, "setFoo"); - setFoo.setAccessible(true); - assertTrue((setFoo.getModifiers() & Modifier.PRIVATE) != 0); + // See KT-20344 + // Method setFoo = findDeclaredMethodByName(aClass, "setFoo"); + // setFoo.setAccessible(true); + // assertTrue((setFoo.getModifiers() & Modifier.PRIVATE) != 0); Method setter = findDeclaredMethodByName(aClass, "setter"); setter.invoke(instance); assertEquals(610, getFoo.invoke(instance)); @@ -247,4 +248,12 @@ public class PropertyGenTest extends CodegenTestCase { assertNull("Property should not have a getter", findDeclaredMethodByNameOrNull(c, "getVarNoAccessors")); assertNull("Property should not have a setter", findDeclaredMethodByNameOrNull(c, "setVarNoAccessors")); } + + // Properties with trivial private setters should not generate their setters since + // the class will use direct field access instead for these properties. + public void testKt20344() throws Exception { + loadText("class Foo { public lateinit var x: String private set }"); + Class aClass = generateClass("Foo"); + assertNull("Property should not have generated setter", findDeclaredMethodByNameOrNull(aClass, "setX")); + } }