Fix assignability analysis for prematurely read fields

DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java

@@ -0,0 +1,45 @@
+/* BSD 2-Clause License - see OPAL/LICENSE for details. */
+package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples;
+
+import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField;
+
+/**
+ * The default value of the field x is assigned to another field n during construction and as
+ * a result seen with two different values.
+ */
+public class PrematurelyReadOfFinalField {
+
+    @AssignableField("Field n is assigned with different values.")
+    static int n = 5;
+
+    public static void main(String[] args) {
+        System.out.println("Value A.X before constructor:" + PrematurelyReadOfFinalField.n);
+        C c = new C();
+        System.out.println("Value A.X after constructor:" + PrematurelyReadOfFinalField.n);
+        System.out.println("Value C.x after constructor:" + c.x );
+    }
+
+}
+class B {
+
+    B() {
+        PrematurelyReadOfFinalField.n = ((C) this).x;
+    }
+
+    void b(C c) {
+        PrematurelyReadOfFinalField.n = c.x;
+    }
+
+}
+
+class C extends B{
+
+    @AssignableField("Is seen with two different values during construction.")
+    public final int x;
+
+    C() {
+        super();
+        //this.b(this);
+        x = 3;
+    }
+}

DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java

@@ -0,0 +1,23 @@
+/* BSD 2-Clause License - see OPAL/LICENSE for details. */
+package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples;
+
+import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField;
+
+/**
+ * This test case simulates the fact that the this object escapes in the constructor before (final) fields
+ * are assigned.
+ */
+public class ThisEscapesDuringConstruction {
+
+   @AssignableField("The this object escapes in the constructor before the field is assigned.")
+   final int n;
+
+    public ThisEscapesDuringConstruction(){
+        C2.m(this);
+        n = 7;
+    }
+}
+
+class C2{
+    public static void m(ThisEscapesDuringConstruction c){}
+}

DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ValueReadBeforeAssignment.java

@@ -0,0 +1,25 @@
+/* BSD 2-Clause License - see OPAL/LICENSE for details. */
+package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples;
+
+import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField;
+
+/**
+ * The value of the field x is read with its default value (0)
+ * in the constructor before assignment and assigned to a public field.
+ * Thus, the value can be accessed from everywhere.
+ */
+public class ValueReadBeforeAssignment {
+    @AssignableField("Field value is read before assignment.")
+    private int x;
+    @AssignableField("Field y is public and not final.")
+    public int y;
+
+    public ValueReadBeforeAssignment() {
+        y = x;
+        x = 42;
+    }
+
+    public ValueReadBeforeAssignment foo() {
+        return new ValueReadBeforeAssignment();
+    }
+}

OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala

@@ -128,29 +128,34 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis {
 
         implicit val state: AnalysisState = createState(field)
 
-        if (field.isFinal)
-            return Result(field, NonAssignable);
-        else
-            state.fieldAssignability = EffectivelyNonAssignable
+        val fieldName = field.name
+        println(fieldName)
+
+        state.fieldAssignability =
+            if (field.isFinal)
+                NonAssignable
+            else
+                EffectivelyNonAssignable
 
-        if (field.isPublic)
+        if (field.isPublic && !field.isFinal)
             return Result(field, Assignable);
 
         val thisType = field.classFile.thisType
 
-        if (field.isPublic) {
+        if (field.isPublic && !field.isFinal) {
             if (typeExtensibility(ObjectType.Object).isYesOrUnknown) {
                 return Result(field, Assignable);
             }
-        } else if (field.isProtected) {
+        } else if (field.isProtected && !field.isFinal) {
             if (typeExtensibility(thisType).isYesOrUnknown) {
                 return Result(field, Assignable);
             }
             if (!closedPackages(thisType.packageName)) {
                 return Result(field, Assignable);
             }
         }
-        if (field.isPackagePrivate) {
+
+        if (field.isPackagePrivate && !field.isFinal) {
             if (!closedPackages(thisType.packageName)) {
                 return Result(field, Assignable);
             }

OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala

@@ -92,18 +92,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject)
         pc:            PC,
         receiver:      AccessReceiver
     )(implicit state: AnalysisState): Boolean = {
+
         val field = state.field
         val method = definedMethod.definedMethod
         val stmts = taCode.stmts
         val receiverVar = receiver.map(uVarForDefSites(_, taCode.pcToIndex))
 
         val index = taCode.pcToIndex(pc)
-        if (method.isInitializer) {
-            if (field.isStatic) {
-                method.isConstructor
-            } else {
-                receiverVar.isDefined && receiverVar.get.definedBy != SelfReferenceParameter
-            }
+        if (method.isInitializer && method.classFile == field.classFile) {
+            field.isStatic && method.isConstructor ||
+            receiverVar.isDefined && receiverVar.get.definedBy != SelfReferenceParameter ||
+            checkWriteDominance(definedMethod, taCode, receiverVar, index)
         } else {
             if (field.isStatic || receiverVar.isDefined && receiverVar.get.definedBy == SelfReferenceParameter) {
                 // We consider lazy initialization if there is only single write
@@ -152,24 +151,45 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject)
         if (writesInMethod.distinctBy(_._2).size > 1)
             return true; // Field is written in multiple locations, thus must be assignable
 
-        // If we have no information about the receiver, we soundly return
-        if (receiverVar.isEmpty)
+        // If we have no information about the receiver, we soundly return true
+        // However, a static field has no receiver
+        if (receiverVar.isEmpty && !state.field.isStatic)
             return true;
 
-        val assignedValueObject = receiverVar.get
-        if (assignedValueObject.definedBy.exists(_ < 0))
+        val assignedValueObject =
+            if (index > 0 && stmts(index).isPutStatic) {
+                stmts(index).asPutStatic.value.asVar
+            } else
+                receiverVar.get
+
+        // When there are more than 1 definitionsite, we soundly return true
+        if (assignedValueObject.definedBy.size != 1)
+            return true;
+
+        val definitionSite = assignedValueObject.definedBy.head
+
+        if (definitionSite < -1 ||
+            (definitionSite == -1 && !definedMethod.definedMethod.isConstructor)
+        )
             return true;
 
-        val assignedValueObjectVar = stmts(assignedValueObject.definedBy.head).asAssignment.targetVar.asVar
+        val uses = if (definitionSite == -1)
+            taCode.params.thisParameter.useSites
+        else {
+            val assignedValueObjectVar = stmts(definitionSite).asAssignment.targetVar.asVar
+            if (assignedValueObjectVar != null)
+                assignedValueObjectVar.usedBy
+            else IntTrieSet.empty
+        }
 
         val fieldWriteInMethodIndex = taCode.pcToIndex(writesInMethod.head._2)
-        if (assignedValueObjectVar != null && !assignedValueObjectVar.usedBy.forall { index =>
+        if (!uses.forall { index =>
                 val stmt = stmts(index)
 
                 fieldWriteInMethodIndex == index || // The value is itself written to another object
                     // IMPROVE: Can we use field access information to care about reflective accesses here?
                     stmt.isPutField && stmt.asPutField.name != state.field.name ||
-                    stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar ||
+                    //  stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar ||
                     stmt.isMethodCall && stmt.asMethodCall.name == "<init>" ||
                     // CHECK do we really need the taCode here?
                     dominates(fieldWriteInMethodIndex, index, taCode)
@@ -256,15 +276,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject)
                 fieldReadAccessInformation.numIndirectAccesses - seenIndirectAccesses
             ).exists { readAccess =>
                 val method = contextProvider.contextFromId(readAccess._1).method
-                (writeAccess._1 eq method) && {
-                    val taCode = state.tacDependees(method.asDefinedMethod).ub.tac.get
 
-                    if (readAccess._3.isDefined && readAccess._3.get._2.forall(isFormalParameter)) {
-                        false
-                    } else {
-                        !dominates(writeAccess._4, taCode.pcToIndex(readAccess._2), taCode)
+                method.definedMethod.classFile != state.field.classFile ||
+                    (writeAccess._1 eq method) && {
+                        val taCode = state.tacDependees(method.asDefinedMethod).ub.tac.get
+
+                        if (readAccess._3.isDefined && readAccess._3.get._2.forall(isFormalParameter)) {
+                            false
+                        } else {
+                            !dominates(writeAccess._4, taCode.pcToIndex(readAccess._2), taCode)
+                        }
                     }
-                }
             }
         }
     }