Repository: fraunhofer-aisec/cpg
Issue: Field `annotations` must not be null, breaks OGM

Commit 7d56eda487e8ec408b13036581d08b5e9800ff3e breaks the object graph mapper (OGM) of Codyze (and possibly Neo4J) and makes CPG structure ambiguous.

Collection fields of `Node`s must not be null.

Fields of classes in the CPG structure will result in either _edges_ or _properties_  of a node. The clearest way is to make that explicit by using the `@Relationship` annotation to enforce an edge. If that is not present, the OGM will investigate the value of the respective field and see if it is a `Node` type or Collection of `Node`s. This requires the field to be non-null, otherwise the OGM will consider it a property on the one hand, but not be able to provide type information on the other hand.

In its current form, Codyze will run into many `IllegalStateException`s.

Changed files:
src/main/java/de/fraunhofer/aisec/cpg/graph/MethodDeclaration.java, src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java, src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/CXXLanguageFrontend.java, src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java

diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/CXXLanguageFrontend.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/CXXLanguageFrontend.java
index 4f1e2bd0e..5cfe36af0 100644
--- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/CXXLanguageFrontend.java
+++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/CXXLanguageFrontend.java
@@ -372,7 +372,7 @@ public class CXXLanguageFrontend extends LanguageFrontend {
   public void processAttributes(@NonNull Node node, @NonNull IASTAttributeOwner owner) {
     if (this.config.processAnnotations) {
       // set attributes
-      node.setAnnotations(handleAttributes(owner));
+      node.addAnnotations(handleAttributes(owner));
     }
   }
 
diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java
index 816a2a126..7a6efb08b 100644
--- a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java
+++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java
@@ -55,7 +55,7 @@ public class ConstructorDeclaration extends MethodDeclaration {
     c.setBody(methodDeclaration.getBody());
     c.setLocation(methodDeclaration.getLocation());
     c.setParameters(methodDeclaration.getParameters());
-    c.setAnnotations(methodDeclaration.getAnnotations());
+    c.addAnnotations(methodDeclaration.getAnnotations());
     c.setIsDefinition(methodDeclaration.isDefinition());
 
     if (!c.isDefinition()) {
diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/MethodDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/MethodDeclaration.java
index 20361ebbd..f1f8456d0 100644
--- a/src/main/java/de/fraunhofer/aisec/cpg/graph/MethodDeclaration.java
+++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/MethodDeclaration.java
@@ -59,7 +59,7 @@ public class MethodDeclaration extends FunctionDeclaration {
     md.setParameters(functionDeclaration.getParameters());
     md.setBody(functionDeclaration.getBody());
     md.setType(functionDeclaration.getType());
-    md.setAnnotations(functionDeclaration.getAnnotations());
+    md.addAnnotations(functionDeclaration.getAnnotations());
     md.setRecordDeclaration(recordDeclaration);
     md.setIsDefinition(functionDeclaration.isDefinition());
 
@@ -80,6 +80,7 @@ public class MethodDeclaration extends FunctionDeclaration {
     this.isStatic = isStatic;
   }
 
+  @Nullable
   public RecordDeclaration getRecordDeclaration() {
     return recordDeclaration;
   }
diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java
index 4a5ff14fd..e46897645 100644
--- a/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java
+++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java
@@ -29,11 +29,7 @@ package de.fraunhofer.aisec.cpg.graph;
 import de.fraunhofer.aisec.cpg.helpers.LocationConverter;
 import de.fraunhofer.aisec.cpg.processing.IVisitable;
 import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
 import org.checkerframework.checker.nullness.qual.NonNull;
@@ -74,6 +70,7 @@ public class Node extends IVisitable<Node> {
   protected String file;
 
   /** Incoming control flow edges. */
+  @NonNull
   @Relationship(value = "EOG", direction = "INCOMING")
   protected List<Node> prevEOG = new ArrayList<>();
 
@@ -87,13 +84,15 @@ public class Node extends IVisitable<Node> {
   @Relationship(value = "CFG", direction = "OUTGOING")
   protected List<Node> nextCFG = new ArrayList<>();
 
+  @NonNull
   @Relationship(value = "DFG", direction = "INCOMING")
   protected Set<Node> prevDFG = new HashSet<>();
 
+  @NonNull
   @Relationship(value = "DFG")
   protected Set<Node> nextDFG = new HashSet<>();
 
-  protected Set<TypedefDeclaration> typedefs = new HashSet<>();
+  @NonNull protected Set<TypedefDeclaration> typedefs = new HashSet<>();
 
   /**
    * If a node is marked as being a dummy, it means that it was created artificially and does not
@@ -116,7 +115,7 @@ public class Node extends IVisitable<Node> {
 
   /** List of annotations associated with that node. */
   @SubGraph("AST")
-  protected List<Annotation> annotations;
+  protected List<Annotation> annotations = new ArrayList<>();
 
   public Long getId() {
     return id;
@@ -156,11 +155,12 @@ public class Node extends IVisitable<Node> {
     this.location = location;
   }
 
+  @NonNull
   public List<Node> getPrevEOG() {
     return this.prevEOG;
   }
 
-  public void setPrevEOG(List<Node> prevEOG) {
+  public void setPrevEOG(@NonNull List<Node> prevEOG) {
     this.prevEOG = prevEOG;
   }
 
@@ -178,11 +178,12 @@ public class Node extends IVisitable<Node> {
     return this.nextCFG;
   }
 
+  @NonNull
   public Set<Node> getNextDFG() {
     return nextDFG;
   }
 
-  public void setNextDFG(Set<Node> nextDFG) {
+  public void setNextDFG(@NonNull Set<Node> nextDFG) {
     this.nextDFG = nextDFG;
   }
 
@@ -198,11 +199,12 @@ public class Node extends IVisitable<Node> {
     }
   }
 
+  @NonNull
   public Set<Node> getPrevDFG() {
     return prevDFG;
   }
 
-  public void setPrevDFG(Set<Node> prevDFG) {
+  public void setPrevDFG(@NonNull Set<Node> prevDFG) {
     this.prevDFG = prevDFG;
   }
 
@@ -222,11 +224,12 @@ public class Node extends IVisitable<Node> {
     this.typedefs.add(typedef);
   }
 
+  @NonNull
   public Set<TypedefDeclaration> getTypedefs() {
     return typedefs;
   }
 
-  public void setTypedefs(Set<TypedefDeclaration> typedefs) {
+  public void setTypedefs(@NonNull Set<TypedefDeclaration> typedefs) {
     this.typedefs = typedefs;
   }
 
@@ -259,6 +262,15 @@ public class Node extends IVisitable<Node> {
     return this.implicit;
   }
 
+  @NonNull
+  public List<Annotation> getAnnotations() {
+    return annotations;
+  }
+
+  public void addAnnotations(@NonNull Collection<Annotation> annotations) {
+    this.annotations.addAll(annotations);
+  }
+
   /**
    * If a node should be removed from the graph, just removing it from the AST is not enough (see
    * issue #60). It will most probably be referenced somewhere via DFG or EOG edges. Thus, if it
@@ -326,12 +338,4 @@ public class Node extends IVisitable<Node> {
   public int hashCode() {
     return Objects.hash(name, this.getClass());
   }
-
-  public void setAnnotations(List<Annotation> annotations) {
-    this.annotations = annotations;
-  }
-
-  public List<Annotation> getAnnotations() {
-    return annotations;
-  }
 }