Merge pull request #9155 from smowton/smowton/fix/field-initializer-flow

Kotlin: Fix initializer field flow by extracting field finality

Author: smowton

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

@@ -778,19 +778,22 @@ open class KotlinFileExtractor(
         with("field", f) {
            DeclarationStackAdjuster(f).use {
                declarationStack.push(f)
-               return extractField(useField(f), f.name.asString(), f.type, parentId, tw.getLocation(f), f.visibility, f, isExternalDeclaration(f))
+               return extractField(useField(f), f.name.asString(), f.type, parentId, tw.getLocation(f), f.visibility, f, isExternalDeclaration(f), f.isFinal)
            }
         }
     }
 
 
-    private fun extractField(id: Label<out DbField>, name: String, type: IrType, parentId: Label<out DbReftype>, locId: Label<DbLocation>, visibility: DescriptorVisibility, errorElement: IrElement, isExternalDeclaration: Boolean): Label<out DbField> {
+    private fun extractField(id: Label<out DbField>, name: String, type: IrType, parentId: Label<out DbReftype>, locId: Label<DbLocation>, visibility: DescriptorVisibility, errorElement: IrElement, isExternalDeclaration: Boolean, isFinal: Boolean): Label<out DbField> {
         val t = useType(type)
         tw.writeFields(id, name, t.javaResult.id, parentId, id)
         tw.writeFieldsKotlinType(id, t.kotlinResult.id)
         tw.writeHasLocation(id, locId)
 
         extractVisibility(errorElement, id, visibility)
+        if (isFinal) {
+            addModifiers(id, "final")
+        }
 
         if (!isExternalDeclaration) {
             val fieldDeclarationId = tw.getFreshIdLabel<DbFielddecl>()
@@ -2949,12 +2952,12 @@ open class KotlinFileExtractor(
 
             // only one of the following can be non-null:
             if (dispatchReceiver != null) {
-                extractField(dispatchFieldId!!, "<dispatchReceiver>", receiverType!!, classId, locId, DescriptorVisibilities.PRIVATE, callableReferenceExpr, false)
+                extractField(dispatchFieldId!!, "<dispatchReceiver>", receiverType!!, classId, locId, DescriptorVisibilities.PRIVATE, callableReferenceExpr, isExternalDeclaration = false, isFinal = true)
                 extractParameterToFieldAssignmentInConstructor("<dispatchReceiver>", dispatchReceiver.type, dispatchFieldId, 0, firstAssignmentStmtIdx)
             }
 
             if (extensionReceiver != null) {
-                extractField(extensionFieldId!!, "<extensionReceiver>", receiverType!!, classId, locId, DescriptorVisibilities.PRIVATE, callableReferenceExpr, false)
+                extractField(extensionFieldId!!, "<extensionReceiver>", receiverType!!, classId, locId, DescriptorVisibilities.PRIVATE, callableReferenceExpr, isExternalDeclaration = false, isFinal = true)
                 extractParameterToFieldAssignmentInConstructor( "<extensionReceiver>", extensionReceiver.type, extensionFieldId, 0 + extensionParameterIndex, firstAssignmentStmtIdx + extensionParameterIndex)
             }
         }
@@ -4005,7 +4008,7 @@ open class KotlinFileExtractor(
 
                     // add field
                     val fieldId = tw.getFreshIdLabel<DbField>()
-                    extractField(fieldId, "<fn>", functionType, classId, locId, DescriptorVisibilities.PRIVATE, e, false)
+                    extractField(fieldId, "<fn>", functionType, classId, locId, DescriptorVisibilities.PRIVATE, e, isExternalDeclaration = false, isFinal = true)
 
                     // adjust constructor
                     helper.extractParameterToFieldAssignmentInConstructor("<fn>", functionType, fieldId, 0, 1)

java/ql/test/kotlin/library-tests/field-initializer-flow/test.expected

@@ -0,0 +1,4 @@
+isFinalField
+| test.kt:3:3:3:18 | x |
+#select
+| test.kt:3:3:3:18 | this.x | test.kt:6:10:6:10 | getX(...) |

java/ql/test/kotlin/library-tests/field-initializer-flow/test.kt

@@ -0,0 +1,11 @@
+class Test {
+
+  val x = "Source"
+
+  fun test() {
+    sink(x)
+  }
+
+  fun sink(s: String) { }
+
+}

java/ql/test/kotlin/library-tests/field-initializer-flow/test.ql

@@ -0,0 +1,22 @@
+import java
+import semmle.code.java.dataflow.DataFlow
+
+class Config extends DataFlow::Configuration {
+  Config() { this = "Config" }
+
+  override predicate isSource(DataFlow::Node n) {
+    n.asExpr().(CompileTimeConstantExpr).getStringValue() = "Source"
+  }
+
+  override predicate isSink(DataFlow::Node n) {
+    n.asExpr().(Argument).getCall().getCallee().getName() = "sink"
+  }
+}
+
+query predicate isFinalField(Field f) {
+  exists(FieldDeclaration f2 | f = f2.getAField()) and f.isFinal()
+}
+
+from DataFlow::Node source, DataFlow::Node sink
+where any(Config c).hasFlow(source, sink)
+select source, sink

java/ql/test/kotlin/library-tests/modifiers/modifiers.expected

@@ -1,20 +1,25 @@
 | modifiers.kt:1:1:25:1 | X | Class | public |
 | modifiers.kt:1:6:25:1 | X | Constructor | public |
+| modifiers.kt:2:5:2:21 | a | Field | final |
 | modifiers.kt:2:5:2:21 | a | Field | private |
 | modifiers.kt:2:5:2:21 | a | Property | private |
 | modifiers.kt:2:13:2:21 | getA$private | Method | private |
+| modifiers.kt:3:5:3:23 | b | Field | final |
 | modifiers.kt:3:5:3:23 | b | Field | private |
 | modifiers.kt:3:5:3:23 | b | Property | protected |
 | modifiers.kt:3:15:3:23 | getB | Method | protected |
+| modifiers.kt:4:5:4:22 | c | Field | final |
 | modifiers.kt:4:5:4:22 | c | Field | private |
 | modifiers.kt:4:5:4:22 | c | Property | internal |
 | modifiers.kt:4:14:4:22 | getC | Method | internal |
+| modifiers.kt:5:5:5:34 | d | Field | final |
 | modifiers.kt:5:5:5:34 | d | Field | private |
 | modifiers.kt:5:5:5:34 | d | Property | public |
 | modifiers.kt:5:5:5:34 | getD | Method | public |
 | modifiers.kt:7:5:9:5 | Nested | Class | final |
 | modifiers.kt:7:5:9:5 | Nested | Class | protected |
 | modifiers.kt:7:15:9:5 | Nested | Constructor | public |
+| modifiers.kt:8:9:8:29 | e | Field | final |
 | modifiers.kt:8:9:8:29 | e | Field | private |
 | modifiers.kt:8:9:8:29 | e | Property | public |
 | modifiers.kt:8:16:8:29 | getE | Method | public |

java/ql/test/kotlin/library-tests/variables/variables.expected

@@ -3,7 +3,11 @@ isFinal
 | variables.kt:8:9:8:26 | int local2 | non-final |
 | variables.kt:10:9:10:26 | int local3 | final |
 compileTimeConstant
+| variables.kt:3:5:3:21 | prop |
+| variables.kt:3:5:3:21 | this.prop |
 | variables.kt:7:17:7:22 | local1 |
+| variables.kt:15:1:15:21 | VariablesKt.topLevel |
+| variables.kt:15:1:15:21 | VariablesKt.topLevel |
 #select
 | variables.kt:3:5:3:21 | prop | int | variables.kt:3:21:3:21 | 1 |
 | variables.kt:5:20:5:29 | param | int | file://:0:0:0:0 | <none> |