Kotlin: don't extract private setters of external classes

Previously these would get extracted unlike other private methods even if the class was a standard library or other external class. This could cause inconsistencies because if we also compiled the class from source we could end up deciding different names for the property's setter: setXyz$private when seen from source, and setXyz without a
suffix when seen as an external .class file. Avoiding extracting these functions from the external perspective both restores consistency with other kinds of method and avoids these consistency problems.

Author: smowton

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -134,7 +134,7 @@ open class KotlinFileExtractor(
                 is IrProperty -> {
                     val parentId = useDeclarationParent(declaration.parent, false)?.cast<DbReftype>()
                     if (parentId != null) {
-                        extractProperty(declaration, parentId, extractBackingField = true, extractFunctionBodies = extractFunctionBodies, null, listOf())
+                        extractProperty(declaration, parentId, extractBackingField = true, extractFunctionBodies = extractFunctionBodies, extractPrivateMembers = extractPrivateMembers, null, listOf())
                     }
                     Unit
                 }
@@ -364,7 +364,7 @@ open class KotlinFileExtractor(
                 if (shouldExtractDecl(it, false)) {
                     when(it) {
                         is IrFunction -> extractFunction(it, id, extractBody = false, extractMethodAndParameterTypeAccesses = false, typeParamSubstitution, argsIncludingOuterClasses)
-                        is IrProperty -> extractProperty(it, id, extractBackingField = false, extractFunctionBodies = false, typeParamSubstitution, argsIncludingOuterClasses)
+                        is IrProperty -> extractProperty(it, id, extractBackingField = false, extractFunctionBodies = false, extractPrivateMembers = false, typeParamSubstitution, argsIncludingOuterClasses)
                         else -> {}
                     }
                 }
@@ -955,7 +955,7 @@ open class KotlinFileExtractor(
         return id
     }
 
-    private fun extractProperty(p: IrProperty, parentId: Label<out DbReftype>, extractBackingField: Boolean, extractFunctionBodies: Boolean, typeSubstitution: TypeSubstitution?, classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?) {
+    private fun extractProperty(p: IrProperty, parentId: Label<out DbReftype>, extractBackingField: Boolean, extractFunctionBodies: Boolean, extractPrivateMembers: Boolean, typeSubstitution: TypeSubstitution?, classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?) {
         with("property", p) {
             if (isFake(p)) return
 
@@ -970,21 +970,25 @@ open class KotlinFileExtractor(
                 val getter = p.getter
                 val setter = p.setter
 
-                if (getter != null) {
+                if (getter == null) {
+                    if (p.modality != Modality.FINAL || !isExternalDeclaration(p)) {
+                        logger.warnElement("IrProperty without a getter", p)
+                    }
+                } else if (shouldExtractDecl(getter, extractPrivateMembers)) {
                     val getterId = extractFunction(getter, parentId, extractBody = extractFunctionBodies, extractMethodAndParameterTypeAccesses = extractFunctionBodies, typeSubstitution, classTypeArgsIncludingOuterClasses)?.cast<DbMethod>()
                     if (getterId != null) {
                         tw.writeKtPropertyGetters(id, getterId)
                         if (getter.origin == IrDeclarationOrigin.DELEGATED_PROPERTY_ACCESSOR) {
                             tw.writeCompiler_generated(getterId, CompilerGeneratedKinds.DELEGATED_PROPERTY_GETTER.kind)
                         }
                     }
-                } else {
-                    if (p.modality != Modality.FINAL || !isExternalDeclaration(p)) {
-                        logger.warnElement("IrProperty without a getter", p)
-                    }
                 }
 
-                if (setter != null) {
+                if (setter == null) {
+                    if (p.isVar && !isExternalDeclaration(p)) {
+                        logger.warnElement("isVar property without a setter", p)
+                    }
+                } else if (shouldExtractDecl(setter, extractPrivateMembers)) {
                     if (!p.isVar) {
                         logger.warnElement("!isVar property with a setter", p)
                     }
@@ -995,10 +999,6 @@ open class KotlinFileExtractor(
                             tw.writeCompiler_generated(setterId, CompilerGeneratedKinds.DELEGATED_PROPERTY_SETTER.kind)
                         }
                     }
-                } else {
-                    if (p.isVar && !isExternalDeclaration(p)) {
-                        logger.warnElement("isVar property without a setter", p)
-                    }
                 }
 
                 if (bf != null && extractBackingField) {

java/ql/integration-tests/posix-only/kotlin/private_property_accessors/hasprops.kt

@@ -0,0 +1,9 @@
+class HasProps {
+
+  var accessorsPublic = 1
+
+  var setterPrivate = 3
+    private set
+
+}
+

java/ql/integration-tests/posix-only/kotlin/private_property_accessors/test.expected

@@ -0,0 +1,7 @@
+| hasprops.kt:3:3:3:25 | getAccessorsPublic |
+| hasprops.kt:3:3:3:25 | setAccessorsPublic |
+| hasprops.kt:5:3:6:15 | getSetterPrivate |
+| hasprops.kt:6:13:6:15 | setSetterPrivate$private |
+| usesprops.kt:1:1:9:1 | user |
+| usesprops.kt:3:3:3:58 | useGetters |
+| usesprops.kt:5:3:7:3 | useSetter |

java/ql/integration-tests/posix-only/kotlin/private_property_accessors/test.py

@@ -0,0 +1,3 @@
+from create_database_utils import *
+
+run_codeql_database_create(["kotlinc hasprops.kt", "kotlinc usesprops.kt -cp ."], lang="java")

java/ql/integration-tests/posix-only/kotlin/private_property_accessors/test.ql

@@ -0,0 +1,5 @@
+import java
+
+from Method m
+where m.fromSource()
+select m

java/ql/integration-tests/posix-only/kotlin/private_property_accessors/usesprops.kt

@@ -0,0 +1,9 @@
+fun user(hp: HasProps) {
+
+  fun useGetters() = hp.accessorsPublic + hp.setterPrivate
+
+  fun useSetter(x: Int) {
+    hp.accessorsPublic = x
+  }
+
+}