Cleanup MetaData and FuzzyC File creation (#1066)

fabsx00 · web-flow · commit 40bdfd6d4e29 · 2020-12-30T16:21:57.000+01:00
* Move `POLICY_DIRECTORIES` into enhancements.json

* MetaData-block cleanup

* Do not create file nodes in fuzzyc frontend

* Do not run `FilenameCompat` for C CPGs

* Rename `FileLinker` to `FileCreationPass` and move

* Minor cleanups
diff --git a/fuzzyc2cpg-tests/src/test/scala/io/shiftleft/fuzzyc2cpg/querying/MetaDataNodeTests.scala b/fuzzyc2cpg-tests/src/test/scala/io/shiftleft/fuzzyc2cpg/querying/MetaDataNodeTests.scala
@@ -0,0 +1,23 @@
+package io.shiftleft.fuzzyc2cpg.querying
+
+import io.shiftleft.fuzzyc2cpg.testfixtures.FuzzyCCodeToCpgSuite
+import io.shiftleft.semanticcpg.language._
+
+class MetaDataNodeTests extends FuzzyCCodeToCpgSuite {
+
+  override val code: String =
+    """
+      |
+      |""".stripMargin
+
+  "should contain exactly one node with all mandatory fields set" in {
+    cpg.metaData.l match {
+      case List(x) =>
+        x.language shouldBe "C"
+        x.version shouldBe "0.1"
+        x.overlays shouldBe List("semanticcpg")
+      case _ => fail
+    }
+  }
+
+}
diff --git a/fuzzyc2cpg-tests/src/test/scala/io/shiftleft/fuzzyc2cpg/testfixtures/FuzzyCCodeToCpgSuite.scala b/fuzzyc2cpg-tests/src/test/scala/io/shiftleft/fuzzyc2cpg/testfixtures/FuzzyCCodeToCpgSuite.scala
@@ -1,7 +1,6 @@
 package io.shiftleft.fuzzyc2cpg.testfixtures
 
 import java.io.File
-
 import io.shiftleft.codepropertygraph.Cpg
 import io.shiftleft.fuzzyc2cpg.FuzzyC2Cpg
 import io.shiftleft.semanticcpg.testfixtures.{CodeToCpgFixture, LanguageFrontend}
diff --git a/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/AstCreationPass.scala b/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/AstCreationPass.scala
@@ -1,7 +1,7 @@
 package io.shiftleft.fuzzyc2cpg.passes
 
 import io.shiftleft.codepropertygraph.Cpg
-import io.shiftleft.codepropertygraph.generated.{EdgeTypes, nodes}
+import io.shiftleft.codepropertygraph.generated.nodes
 import io.shiftleft.fuzzyc2cpg.passes.astcreation.{AntlrCModuleParserDriver, AstVisitor}
 import io.shiftleft.fuzzyc2cpg.{Defines, Global}
 import io.shiftleft.passes.{DiffGraph, IntervalKeyPool, ParallelCpgPass}
@@ -23,26 +23,20 @@ class AstCreationPass(filenames: List[String], cpg: Cpg, keyPool: IntervalKeyPoo
 
     val diffGraph = DiffGraph.newBuilder
     val absolutePath = new java.io.File(filename).toPath.toAbsolutePath.normalize().toString
-    val fileNode = nodes.NewFile(name = absolutePath, order = 0)
-    diffGraph.addNode(fileNode)
     val namespaceBlock = nodes.NewNamespaceBlock(
       name = Defines.globalNamespaceName,
-      fullName = CMetaDataPass.getGlobalNamespaceBlockFullName(Some(fileNode.name))
+      fullName = CMetaDataPass.getGlobalNamespaceBlockFullName(Some(absolutePath)),
+      filename = absolutePath
     )
-    diffGraph.addNode(fileNode)
     diffGraph.addNode(namespaceBlock)
-    diffGraph.addEdge(fileNode, namespaceBlock, EdgeTypes.AST)
-
-    val driver = createDriver(fileNode, namespaceBlock)
+    val driver = createDriver(namespaceBlock)
     tryToParse(driver, filename, diffGraph)
   }
 
-  private def createDriver(fileNode: nodes.NewFile,
-                           namespaceBlock: nodes.NewNamespaceBlock): AntlrCModuleParserDriver = {
+  private def createDriver(namespaceBlock: nodes.NewNamespaceBlock): AntlrCModuleParserDriver = {
     val driver = new AntlrCModuleParserDriver()
     val astVisitor = new AstVisitor(driver, namespaceBlock, global)
     driver.addObserver(astVisitor)
-    driver.setFileNode(fileNode)
     driver
   }
 
diff --git a/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/CMetaDataPass.scala b/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/CMetaDataPass.scala
@@ -13,7 +13,7 @@ import io.shiftleft.passes.{CpgPass, DiffGraph, KeyPool}
 class CMetaDataPass(cpg: Cpg, keyPool: Option[KeyPool] = None) extends CpgPass(cpg, keyPool = keyPool) {
   override def run(): Iterator[DiffGraph] = {
     def addMetaDataNode(diffGraph: DiffGraph.Builder): Unit = {
-      val metaNode = nodes.NewMetaData(language = Languages.C)
+      val metaNode = nodes.NewMetaData(language = Languages.C, version = "0.1")
       diffGraph.addNode(metaNode)
     }
 
diff --git a/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/astcreation/AntlrParserDriver.java b/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/astcreation/AntlrParserDriver.java
@@ -53,17 +53,13 @@ abstract public class AntlrParserDriver {
     private CommonParserContext context = null;
     public DiffGraph.Builder cpg;
     private final List<AntlrParserDriverObserver> observers = new ArrayList<>();
-    private NewFile fileNode;
+
     private Parser antlrParser;
 
     public AntlrParserDriver() {
         super();
     }
 
-    public void setFileNode(NewFile fileNode) {
-        this.fileNode = fileNode;
-    }
-
     public abstract ParseTree parseTokenStreamImpl(TokenSubStream tokens);
 
     public abstract Lexer createLexer(CharStream input);
@@ -95,10 +91,10 @@ private void handleHiddenTokens(String filename) {
             String text = token.getText();
             NewComment commentNode = new NewComment(
                     new Some<>(line),
-                    text
+                    text,
+                    filename
             );
             cpg.addNode(commentNode);
-            cpg.addEdge(fileNode, commentNode, EdgeTypes.AST, List$.MODULE$.empty());
         }
     }
 
diff --git a/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/astcreation/AstCreator.scala b/fuzzyc2cpg/src/main/scala/io/shiftleft/fuzzyc2cpg/passes/astcreation/AstCreator.scala
@@ -116,6 +116,7 @@ private[astcreation] class AstCreator(diffGraph: DiffGraph.Builder,
       lineNumberEnd = location.endLine,
       columnNumberEnd = location.endPos,
       signature = signature,
+      filename = namespaceBlock.filename
     )
 
     addAndConnectAsAstChild(method)
@@ -628,7 +629,8 @@ private[astcreation] class AstCreator(diffGraph: DiffGraph.Builder,
         name = identifierDecl.getName.getEscapedCodeStr,
         fullName = identifierDecl.getName.getEscapedCodeStr,
         isExternal = false,
-        aliasTypeFullName = Some(registerType(declTypeName))
+        aliasTypeFullName = Some(registerType(declTypeName)),
+        filename = namespaceBlock.filename
       )
       diffGraph.addNode(aliasTypeDecl)
       connectAstChild(aliasTypeDecl)
@@ -785,7 +787,8 @@ private[astcreation] class AstCreator(diffGraph: DiffGraph.Builder,
       name = name,
       fullName = name,
       isExternal = false,
-      inheritsFromTypeFullName = baseClassList
+      inheritsFromTypeFullName = baseClassList,
+      filename = namespaceBlock.filename
     )
 
     diffGraph.addNode(typeDecl)
diff --git a/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/CpgTestFixture.scala b/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/CpgTestFixture.scala
@@ -4,7 +4,7 @@ import io.shiftleft.codepropertygraph.Cpg
 import io.shiftleft.fuzzyc2cpg.passes.{AstCreationPass, CMetaDataPass, StubRemovalPass}
 import io.shiftleft.passes.IntervalKeyPool
 import io.shiftleft.semanticcpg.language._
-import io.shiftleft.semanticcpg.passes.CfgCreationPass
+import io.shiftleft.semanticcpg.passes.{CfgCreationPass, FileCreationPass}
 import io.shiftleft.x2cpg.SourceFiles
 import overflowdb.traversal.TraversalSource
 
@@ -22,6 +22,7 @@ case class CpgTestFixture(projectName: String) {
     new CfgCreationPass(cpg, cfgKeyPool).createAndApply()
   }
   new StubRemovalPass(cpg).createAndApply()
+  new FileCreationPass(cpg).createAndApply()
 
   def traversalSource = TraversalSource(cpg.graph)
 
diff --git a/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/ProgramStructureTests.scala b/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/ProgramStructureTests.scala
@@ -1,7 +1,5 @@
 package io.shiftleft.fuzzyc2cpg
 
-import java.nio.file.Paths
-
 import io.shiftleft.codepropertygraph.generated.NodeKeys
 import io.shiftleft.proto.cpg.Cpg.CpgStruct.Node.NodeType
 import org.scalatest.matchers.should.Matchers
@@ -23,32 +21,6 @@ class ProgramStructureTests extends AnyWordSpec with Matchers {
       namespaceBlocks.size shouldBe 1
     }
 
-    "contain one file node" in {
-      val fileName = fixture.traversalSource
-        .label(NodeType.FILE.toString)
-        .property(NodeKeys.NAME)
-        .headOption
-      fileName.isDefined shouldBe true
-      fileName.head should not contain ".."
-
-      // Construct a platform-independent path.
-      val path = Paths
-        .get("src", "test", "resources", "testcode", "structure", "structure.c")
-        .toString
-
-      fileName.head should not be path
-      fileName.head should endWith(path)
-    }
-
-    "contain AST edge from file node to namespace block" in {
-      val nodes = fixture.traversalSource
-        .label(NodeType.FILE.toString)
-        .out("AST")
-        .hasLabel(NodeType.NAMESPACE_BLOCK.toString)
-        .l
-      nodes.size shouldBe 1
-    }
-
     "contain type-decl node" in {
       val nodes = fixture.traversalSource.label(NodeType.TYPE_DECL.toString).l
       nodes.size should be > 0
diff --git a/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/passes/AstCreationPassTests.scala b/fuzzyc2cpg/src/test/scala/io/shiftleft/fuzzyc2cpg/passes/AstCreationPassTests.scala
@@ -22,10 +22,6 @@ class AstCreationPassTests extends AnyWordSpec with Matchers {
       new AstCreationPass(filenames, cpg, new IntervalKeyPool(1, 1000))
         .createAndApply()
 
-      "create one File node per file name with absolute path in `name`" in {
-        cpg.file.name.toSet shouldBe expectedFilenameFields.toSet
-      }
-
       "create one NamespaceBlock per file" in {
         val expectedNamespaceFullNames = expectedFilenameFields.map(f => s"$f:<global>").toSet
         cpg.namespaceBlock.fullName.toSet shouldBe expectedNamespaceFullNames
diff --git a/schema/src/main/resources/schemas/base.json b/schema/src/main/resources/schemas/base.json
@@ -9,7 +9,6 @@
         { "id": 19, "name" : "LANGUAGE", "comment" : "The programming language this graph originates from", "valueType" : "string", "cardinality" : "one"},
         { "id" : 13, "name": "VERSION", "comment" : "A version, given as a string", "valueType" : "string", "cardinality" : "one"},
 	{ "id" : 118, "name" : "OVERLAYS", "comment" : "Names of overlays applied to this graph, in order of application", "valueType" : "string", "cardinality" : "list"},
-	{ "id" : 119, "name" : "POLICY_DIRECTORIES", "comment": "Sub directories of the policy directory that should be loaded when processing the CPG", "valueType" : "string", "cardinality" : "list"},
 	{ "id" : 120, "name" : "HASH", "comment" : "Hash value of the artifact that this CPG is built from.", "valueType": "string", "cardinality" : "zeroOrOne"},
 
         // Properties that indicate where the code can be found
@@ -68,7 +67,7 @@
         {
             "id" : 39,
             "name" : "META_DATA",
-            "keys" : ["LANGUAGE", "VERSION", "OVERLAYS", "POLICY_DIRECTORIES", "HASH"],
+            "keys" : ["LANGUAGE", "VERSION", "OVERLAYS", "HASH"],
             "comment" : "Node to save meta data about the graph on its properties. Exactly one node of this type per graph",
             "outEdges" : []
         },
diff --git a/schema/src/main/resources/schemas/enhancements.json b/schema/src/main/resources/schemas/enhancements.json
@@ -3,8 +3,9 @@
     // note: these should *NOT* be written by the language frontend.
 
     "nodeKeys" : [
-        {"id" : 8, "name": "VALUE", "comment" : "Tag value", "valueType" : "string", "cardinality" : "one"},
-        {"id" : 1002, "name": "IS_METHOD_NEVER_OVERRIDDEN", "comment" : "True if the referenced method is never overridden by the subclasses and false otherwise", "valueType" : "boolean", "cardinality" : "zeroOrOne"}
+      {"id" : 8, "name": "VALUE", "comment" : "Tag value", "valueType" : "string", "cardinality" : "one"},
+      {"id" : 1002, "name": "IS_METHOD_NEVER_OVERRIDDEN", "comment" : "True if the referenced method is never overridden by the subclasses and false otherwise", "valueType" : "boolean", "cardinality" : "zeroOrOne"},
+      { "id" : 119, "name" : "POLICY_DIRECTORIES", "comment": "Sub directories of the policy directory that should be loaded when processing the CPG", "valueType" : "string", "cardinality" : "list"}
     ],
 
     "edgeKeys" : [
@@ -241,7 +242,11 @@
             ]},
             {"edgeName": "EVAL_TYPE", "inNodes": [{"name": "TYPE"}]}
           ]
-        }
+        },
+      {
+	"name" : "META_DATA",
+	"keys": ["POLICY_DIRECTORIES"]
+      }
     ],
 
     "edgeTypes" : [
diff --git a/schema/src/main/resources/schemas/source-specific.json b/schema/src/main/resources/schemas/source-specific.json
@@ -1,13 +1,16 @@
 {
     "nodeTypes" : [
 	{"id" : 511, "name" : "COMMENT",
-	 "keys" : ["LINE_NUMBER", "CODE"],
+	 "keys" : ["LINE_NUMBER", "CODE", "FILENAME"],
 	 "comment" : "A comment",
-	 "outEdges" : []
+	 "outEdges" : [
+	   {"edgeName": "SOURCE_FILE", "inNodes": [{"name": "COMMENT"}]}
+	 ]
 	},
 
 	{"name" : "FILE",
 	 "outEdges" : [
+	   // TODO: remove AST edge here
 	   {"edgeName": "AST", "inNodes": [{"name": "COMMENT"}]}
 	 ]
 	}
diff --git a/semanticcpg/src/main/scala/io/shiftleft/semanticcpg/layers/Scpg.scala b/semanticcpg/src/main/scala/io/shiftleft/semanticcpg/layers/Scpg.scala
@@ -4,7 +4,7 @@ import io.shiftleft.codepropertygraph.Cpg
 import io.shiftleft.codepropertygraph.generated.{Languages, NodeTypes}
 import io.shiftleft.passes.CpgPassBase
 import io.shiftleft.semanticcpg.language._
-import io.shiftleft.semanticcpg.passes.BindingMethodOverridesPass
+import io.shiftleft.semanticcpg.passes.{BindingMethodOverridesPass, FileCreationPass}
 import io.shiftleft.semanticcpg.passes.cfgdominator.CfgDominatorPass
 import io.shiftleft.semanticcpg.passes.codepencegraph.CdgPass
 import io.shiftleft.semanticcpg.passes.compat.argumentcompat.ArgumentCompat
@@ -14,7 +14,7 @@ import io.shiftleft.semanticcpg.passes.containsedges.ContainsEdgePass
 import io.shiftleft.semanticcpg.passes.languagespecific.fuzzyc.{MethodStubCreator, TypeDeclStubCreator}
 import io.shiftleft.semanticcpg.passes.linking.calllinker.CallLinker
 import io.shiftleft.semanticcpg.passes.linking.capturinglinker.CapturingLinker
-import io.shiftleft.semanticcpg.passes.linking.filecompat.{FileLinker, FileNameCompat}
+import io.shiftleft.semanticcpg.passes.linking.filecompat.FileNameCompat
 import io.shiftleft.semanticcpg.passes.linking.linker.Linker
 import io.shiftleft.semanticcpg.passes.linking.memberaccesslinker.MemberAccessLinker
 import io.shiftleft.semanticcpg.passes.methoddecorations.MethodDecoratorPass
@@ -58,7 +58,7 @@ class Scpg(optionsUnused: LayerCreatorOptions = null) extends LayerCreator {
           new CapturingLinker(cpg),
           new Linker(cpg),
           new FileNameCompat(cpg),
-          new FileLinker(cpg),
+          new FileCreationPass(cpg),
           new BindingTableCompat(cpg),
           new BindingMethodOverridesPass(cpg),
           new CallLinker(cpg),
@@ -77,8 +77,7 @@ class Scpg(optionsUnused: LayerCreatorOptions = null) extends LayerCreator {
           new MethodDecoratorPass(cpg),
           new CapturingLinker(cpg),
           new Linker(cpg),
-          new FileNameCompat(cpg),
-          new FileLinker(cpg),
+          new FileCreationPass(cpg),
           new BindingTableCompat(cpg),
           new BindingMethodOverridesPass(cpg),
           new CallLinker(cpg),
@@ -97,7 +96,7 @@ class Scpg(optionsUnused: LayerCreatorOptions = null) extends LayerCreator {
           new CapturingLinker(cpg),
           new Linker(cpg),
           new FileNameCompat(cpg),
-          new FileLinker(cpg),
+          new FileCreationPass(cpg),
           new BindingTableCompat(cpg),
           new BindingMethodOverridesPass(cpg),
           new CallLinker(cpg),
@@ -117,7 +116,7 @@ class Scpg(optionsUnused: LayerCreatorOptions = null) extends LayerCreator {
           new CapturingLinker(cpg),
           new Linker(cpg),
           new FileNameCompat(cpg),
-          new FileLinker(cpg),
+          new FileCreationPass(cpg),
           new ContainsEdgePass(cpg),
           new MethodExternalDecoratorPass(cpg),
           new CfgDominatorPass(cpg),
diff --git a/semanticcpg/src/main/scala/io/shiftleft/semanticcpg/passes/FileCreationPass.scala b/semanticcpg/src/main/scala/io/shiftleft/semanticcpg/passes/FileCreationPass.scala
@@ -1,7 +1,7 @@
-package io.shiftleft.semanticcpg.passes.linking.filecompat
+package io.shiftleft.semanticcpg.passes
 
 import io.shiftleft.codepropertygraph.Cpg
-import io.shiftleft.codepropertygraph.generated.{EdgeTypes, NodeTypes, nodes}
+import io.shiftleft.codepropertygraph.generated.{EdgeTypes, NodeKeyNames, NodeTypes, nodes}
 import io.shiftleft.passes.{CpgPass, DiffGraph}
 import io.shiftleft.semanticcpg.language._
 import io.shiftleft.semanticcpg.language.types.structure.File
@@ -10,13 +10,10 @@ import io.shiftleft.semanticcpg.passes.linking.linker.Linker
 import scala.collection.mutable
 
 /**
-  * Links NAMESPACE_BLOCKs, TYPE_DECLs and METHODs to their FILEs via
-  * SOURCE_FILE edges.
-  *
-  * This pass should come after FILENAME properties have been recovered,
-  * that is, after FileNameCompat.
+  * For all nodes with FILENAME fields, create corresponding FILE nodes
+  * and connect node with FILE node via outgoing SOURCE_FILE edges.
   */
-class FileLinker(cpg: Cpg) extends CpgPass(cpg) {
+class FileCreationPass(cpg: Cpg) extends CpgPass(cpg) {
   override def run(): Iterator[DiffGraph] = {
     val dstGraph = DiffGraph.newBuilder
 
@@ -45,12 +42,13 @@ class FileLinker(cpg: Cpg) extends CpgPass(cpg) {
       srcLabels = List(
         NodeTypes.NAMESPACE_BLOCK,
         NodeTypes.TYPE_DECL,
-        NodeTypes.METHOD
+        NodeTypes.METHOD,
+        NodeTypes.COMMENT
       ),
       dstNodeLabel = NodeTypes.FILE,
       edgeType = EdgeTypes.SOURCE_FILE,
       dstNodeMap = originalFileNameToNode,
-      dstFullNameKey = "FILENAME",
+      dstFullNameKey = NodeKeyNames.FILENAME,
       dstGraph,
       Some(createFileIfDoesNotExist)
     )

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ import io.shiftleft.passes.{CpgPass, DiffGraph, KeyPool}`
`13`	`13`	`class CMetaDataPass(cpg: Cpg, keyPool: Option[KeyPool] = None) extends CpgPass(cpg, keyPool = keyPool) {`
`14`	`14`	`override def run(): Iterator[DiffGraph] = {`
`15`	`15`	`def addMetaDataNode(diffGraph: DiffGraph.Builder): Unit = {`
`16`		`- val metaNode = nodes.NewMetaData(language = Languages.C)`
	`16`	`+ val metaNode = nodes.NewMetaData(language = Languages.C, version = "0.1")`
`17`	`17`	`diffGraph.addNode(metaNode)`
`18`	`18`	`}`
`19`	`19`