Repository: joernio/joern (scala)

Issue:
[Bug] [ruby] Calls Directly under Type Decl
**Describe the bug**
On the `zammad` project, there are a number of call nodes directly under a Type Decl which is invalid
```
joern> cpg.typeDecl.where(_.astOut.isCall).size
val res2: Int = 929
```

**To Reproduce**
Parse https://github.com/zammad/zammad

Pull request description:
Fixes issue where `<body>` calls from nested types were being given as direct children to type decls. The solution pushes `<body>` calls to the next method.

Another issue was that access modifier calls did not check that the argument was actually a method. Sometimes methods could be referred to via an identifier in some data structure. This PR currently emits a warning for these cases.

Resolves #5155

Patch:
diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala
index 37b81e02fca4..9fffe327236a 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForFunctionsCreator.scala
@@ -203,7 +203,10 @@ trait AstForFunctionsCreator(implicit withSchemaValidation: ValidationMode) { th
         pushAccessModifier(ModifierTypes.PUBLIC)
     }
 
-    val methodAst = astsForStatement(node.method)
+    val methodAst = node.method match {
+      case m: ProcedureDeclaration => astsForStatement(m)
+      case x                       => logger.warn(s"Unhandled method reference from AST type ${x.getClass}"); Nil
+    }
 
     popAccessModifier()
     pushAccessModifier(originalAccessModifier)
diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForTypesCreator.scala b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForTypesCreator.scala
index 950dc6055e53..9e46d0be3d48 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForTypesCreator.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForTypesCreator.scala
@@ -1,6 +1,6 @@
 package io.joern.rubysrc2cpg.astcreation
 
-import io.joern.rubysrc2cpg.astcreation.RubyIntermediateAst.*
+import io.joern.rubysrc2cpg.astcreation.RubyIntermediateAst.{TypeDeclaration, *}
 import io.joern.rubysrc2cpg.datastructures.{BlockScope, MethodScope, ModuleScope, NamespaceScope, TypeScope}
 import io.joern.rubysrc2cpg.passes.Defines
 import io.joern.x2cpg.utils.NodeBuilders.newModifierNode
@@ -143,8 +143,21 @@ trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this:
     val classBody =
       node.body.asInstanceOf[StatementList] // for now (bodyStatement is a superset of stmtList)
 
+    val statementsToForwardUpTheAst = mutable.ArrayBuffer.empty[Ast]
+    def separateStatementsFromBody(ss: List[RubyExpression]) = {
+      // There may be additional expression nodes introduced from nodes such as type decls, so we must
+      // re-distribute these back into the <body> method
+      ss.flatMap {
+        case t: TypeDeclaration =>
+          val (typeDeclAsts, other) = astsForStatement(t).partition(_.root.exists(_.isInstanceOf[NewTypeDecl]))
+          statementsToForwardUpTheAst.addAll(other)
+          typeDeclAsts
+        case n => astsForStatement(n)
+      }
+    }
+
     val classBodyAsts = {
-      val bodyAsts = classBody.statements.flatMap(astsForStatement)
+      val bodyAsts = separateStatementsFromBody(classBody.statements)
       if (scope.shouldGenerateDefaultConstructor && this.parseLevel == AstParseLevel.FULL_AST) {
         val bodyStart  = classBody.span.spanStart()
         val initBody   = StatementList(List())(bodyStart)
@@ -200,7 +213,7 @@ trait AstForTypesCreator(implicit withSchemaValidation: ValidationMode) { this:
 
     if shouldPopAdditionalScope then scope.popScope()
     popAccessModifier()
-    prefixAst :: bodyMemberCallAst :: Nil
+    prefixAst :: bodyMemberCallAst :: statementsToForwardUpTheAst.toList
   }
 
   private def astForTypeDeclBodyCall(node: TypeDeclBodyCall, typeFullName: String): Ast = {


Test patch:
diff --git a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ClassTests.scala b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ClassTests.scala
index 539d523988c1..806f04f1ce0d 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ClassTests.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/querying/ClassTests.scala
@@ -191,6 +191,29 @@ class ClassTests extends RubyCode2CpgFixture {
     aMember.lineNumber shouldBe Some(3)
   }
 
+  "`attr_reader` in a nested class should generate the correct member for the correct class" in {
+    val cpg = code("""
+        |class Foo
+        |
+        |  class Bar
+        |    attr_reader :a
+        |  end
+        |
+        |end
+        |
+        |""".stripMargin)
+
+    val List(bar)     = cpg.typeDecl.name("Bar").l
+    val List(aMember) = bar.member.name("@a").l
+
+    aMember.code shouldBe "attr_reader :a"
+    aMember.lineNumber shouldBe Some(5)
+
+    // <body> calls are pushed all the way up to the <module>
+    cpg.typeDecl.astOut.isCall.size shouldBe 0
+    cpg.call(RubyDefines.TypeDeclBody).method.dedup.name.l shouldBe List(RubyDefines.Main)
+  }
+
   "`def f(x) ... end` is represented by a METHOD inside the TYPE_DECL node" in {
     val cpg = code("""
                      |class C
@@ -1270,4 +1293,22 @@ class ClassTests extends RubyCode2CpgFixture {
       case xs => fail(s"expected 1 type, got ${xs.size}: [${xs.code.mkString(", ")}]")
     }
   }
+
+  "`private_class_method` with unhandled argument types should not render these under the type decl" in {
+    val cpg = code("""
+        |class Foo
+        |
+        |  private_class_method %i[
+        |    filter_ignore_usable
+        |    filter_key
+        |    filter_usage
+        |    parts
+        |  ]
+        |
+        |end
+        |
+        |""".stripMargin)
+
+    cpg.typeDecl("Foo").astChildren.whereNot(_.or(_.isMethod, _.isModifier, _.isTypeDecl, _.isMember)).size shouldBe 0
+  }
 }