Repository: joernio/joern (scala)

Issue:
[Bug] RubyParser: Incorrect hierarchy with the methodDefinition production
**Describe the bug**
The methodDefinition production produces an incorrect parse tree for a certain permutation of methods.

**To Reproduce**
The below code produces an incorrect parse tree. The parse tree is incorrect since the method` foo2() `comes into the body of method `foo1()`
```
module SomeModule
def foo1
    return unless true
end
def foo2=(arg)
end
end
```

The below code which is the same with just the order of the method swapped generates a correct parse tree. The parse tree is correct because the methods are at the same level.
```
module SomeModule
def foo2=(arg)
end
def foo1
    return unless true
end
end
```


**Expected behavior**
Below is a correct parse tree
![Correct parse tree](https://github.com/joernio/joern/assets/119306799/8c2264e5-adf1-4069-8b57-dc9d2ec88d80)

Below is the incorrect parse tree.
![Incorrect parse tree](https://github.com/joernio/joern/assets/119306799/79d42f21-1d88-4514-af5c-f6bc4f0fb140)

1. The methods should be at the same level
2. This should be irrespective of the order in which the methods have been written in the module.

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Desktop (please complete the following information):**
 - OS
 - Joern Version 
 - Java version

**Additional context**
Add any other context about the problem here.

Pull request:
Previously, so-called assignment-like method identifiers (e.g. `foo2=` in `def foo2=(x) ... end`) were being lexed as two separate tokens (`LOCAL_VARIABLE_IDENTIFIER` and `EQ`) and handled together in a parser rule. However, with the introduction of so-called endless methods, this separation could lead to wrong parse trees, as witnessed by #3270 .

To resolve this ambiguity, we shall now gobble up the `EQ` token as part of the method name, emitting an `ASSIGNMENT_LIKE_METHOD_IDENTIFIER` token instead.

Closes #3270

Patch:
diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyLexer.g4 b/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyLexer.g4
index bd968f5d8189..e35d700bfc9c 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyLexer.g4
+++ b/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyLexer.g4
@@ -430,7 +430,7 @@ fragment LINE_TERMINATOR
 // --------------------------------------------------------
 
 SYMBOL_LITERAL
-    :   ':' SYMBOL_NAME
+    :   ':' (SYMBOL_NAME | (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) '=')
     // This check exists to prevent issuing a SYMBOL_LITERAL in whitespace-free associations, e.g. 
     //      in `foo(x:y)`, so that `:y` is not a SYMBOL_LITERAL
     // or   in `{:x=>1}`, so that `:x=` is not a SYMBOL_LITERAL
@@ -444,7 +444,6 @@ fragment SYMBOL_NAME
     |   CONSTANT_IDENTIFIER
     |   LOCAL_VARIABLE_IDENTIFIER
     |   METHOD_ONLY_IDENTIFIER
-    |   ASSIGNMENT_LIKE_METHOD_IDENTIFIER
     |   OPERATOR_METHOD_NAME
     |   KEYWORD
     // NOTE: Even though we have PLUSAT and MINUSAT in OPERATOR_METHOD_NAME, the former
@@ -483,8 +482,12 @@ fragment METHOD_ONLY_IDENTIFIER
     :   (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) ('!' | '?')
     ;
 
-fragment ASSIGNMENT_LIKE_METHOD_IDENTIFIER
-    :   (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) '='
+
+// Similarly to PLUSAT/MINUSAT, this should only occur after a DEF token.
+// Otherwise, the assignment `x=nil` would be parsed as (ASSIGNMENT_LIKE_METHOD_IDENTIFIER, NIL)
+// instead of the more appropriate (LOCAL_VARIABLE_IDENTIFIER, EQ, NIL).
+ASSIGNMENT_LIKE_METHOD_IDENTIFIER
+    :   (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) '=' {previousNonWsTokenTypeOrEOF() == DEF}?
     ;
 
 fragment IDENTIFIER_CHARACTER
diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyParser.g4 b/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyParser.g4
index b42150438904..4c297b8385ff 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyParser.g4
+++ b/joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyParser.g4
@@ -315,7 +315,7 @@ definedMethodName
     ;
 
 assignmentLikeMethodIdentifier
-    :   (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) EQ
+    :   ASSIGNMENT_LIKE_METHOD_IDENTIFIER
     ;
 
 methodName
diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstCreator.scala b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstCreator.scala
index 54728755934a..680b8f7f98ad 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstCreator.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstCreator.scala
@@ -1068,23 +1068,11 @@ class AstCreator(
     }
   }
   def astForAssignmentLikeMethodIdentifierContext(ctx: AssignmentLikeMethodIdentifierContext): Seq[Ast] = {
-    if (ctx == null) return Seq(Ast())
-
-    val terminalNode = Option(ctx.LOCAL_VARIABLE_IDENTIFIER()) match
-      case Some(value) => value
-      case None        => ctx.CONSTANT_IDENTIFIER()
-
-    val methodName = terminalNode.getText + "="
-    val callNode = NewCall()
-      .name(methodName)
-      .code(ctx.getText)
-      .methodFullName(methodName)
-      .signature("")
-      .dispatchType(DispatchTypes.STATIC_DISPATCH)
-      .typeFullName(Defines.Any)
-      .lineNumber(terminalNode.getSymbol().getLine())
-      .columnNumber(terminalNode.getSymbol().getCharPositionInLine())
-    Seq(callAst(callNode))
+    Seq(
+      callAst(
+        callNode(ctx, ctx.getText, ctx.getText, ctx.getText, DispatchTypes.STATIC_DISPATCH, Some(""), Some(Defines.Any))
+      )
+    )
   }
 
   def astForDefinedMethodNameContext(ctx: DefinedMethodNameContext): Seq[Ast] = {


Test patch:
diff --git a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala
index 3b49952306d6..b39510151316 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala
@@ -552,8 +552,7 @@ class MethodDefinitionTests extends RubyParserAbstractTest {
             |       SimpleMethodNamePart
             |        DefinedMethodName
             |         AssignmentLikeMethodIdentifier
-            |          foo2
-            |          =
+            |          foo2=
             |       MethodParameterPart
             |        (
             |        Parameters
@@ -569,6 +568,102 @@ class MethodDefinitionTests extends RubyParserAbstractTest {
             |  Separator""".stripMargin
 
       }
+
+      // This test makes sure that the `end` after `def foo2=` is not parsed as part of its definition,
+      // which could happen if `foo2=` was parsed as two separate tokens (LOCAL_VARIABLE_IDENTIFIER, EQ)
+      // instead of just ASSIGNMENT_LIKE_METHOD_IDENTIFIER.
+      // Issue report: https://github.com/joernio/joern/issues/3270
+      "its name ends in `=` and the next keyword is `end`" in {
+        val code =
+          """module SomeModule
+            |def foo1
+            |    return unless true
+            |end
+            |def foo2=(arg)
+            |end
+            |end
+            |""".stripMargin
+        printAst(_.compoundStatement(), code) shouldEqual
+          """CompoundStatement
+            | Statements
+            |  ExpressionOrCommandStatement
+            |   ExpressionExpressionOrCommand
+            |    PrimaryExpression
+            |     ModuleDefinitionPrimary
+            |      ModuleDefinition
+            |       module
+            |       WsOrNl
+            |       ClassOrModuleReference
+            |        SomeModule
+            |       WsOrNl
+            |       BodyStatement
+            |        CompoundStatement
+            |         Statements
+            |          ExpressionOrCommandStatement
+            |           ExpressionExpressionOrCommand
+            |            PrimaryExpression
+            |             MethodDefinitionPrimary
+            |              MethodDefinition
+            |               def
+            |               WsOrNl
+            |               SimpleMethodNamePart
+            |                DefinedMethodName
+            |                 MethodName
+            |                  MethodIdentifier
+            |                   foo1
+            |               MethodParameterPart
+            |               Separator
+            |               WsOrNl
+            |               BodyStatement
+            |                CompoundStatement
+            |                 Statements
+            |                  ModifierStatement
+            |                   ExpressionOrCommandStatement
+            |                    InvocationExpressionOrCommand
+            |                     ReturnArgsInvocationWithoutParentheses
+            |                      return
+            |                   unless
+            |                   WsOrNl
+            |                   ExpressionOrCommandStatement
+            |                    ExpressionExpressionOrCommand
+            |                     PrimaryExpression
+            |                      VariableReferencePrimary
+            |                       PseudoVariableIdentifierVariableReference
+            |                        TruePseudoVariableIdentifier
+            |                         true
+            |                 Separators
+            |                  Separator
+            |               end
+            |          Separators
+            |           Separator
+            |          ExpressionOrCommandStatement
+            |           ExpressionExpressionOrCommand
+            |            PrimaryExpression
+            |             MethodDefinitionPrimary
+            |              MethodDefinition
+            |               def
+            |               WsOrNl
+            |               SimpleMethodNamePart
+            |                DefinedMethodName
+            |                 AssignmentLikeMethodIdentifier
+            |                  foo2=
+            |               MethodParameterPart
+            |                (
+            |                Parameters
+            |                 Parameter
+            |                  MandatoryParameter
+            |                   arg
+            |                )
+            |               Separator
+            |               BodyStatement
+            |                CompoundStatement
+            |               end
+            |         Separators
+            |          Separator
+            |       end
+            | Separators
+            |  Separator""".stripMargin
+      }
     }
   }
 
diff --git a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/RubyLexerTests.scala b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/RubyLexerTests.scala
index 512775f5c7b4..82d22328048b 100644
--- a/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/RubyLexerTests.scala
+++ b/joern-cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/RubyLexerTests.scala
@@ -123,6 +123,11 @@ class RubyLexerTests extends AnyFlatSpec with Matchers {
     all(eg.map(tokenize)) shouldBe Seq(SYMBOL_LITERAL, EOF)
   }
 
+  "Assignment-like-named symbols" should "be recognized as such" in {
+    val eg = Seq(":X=", ":xyz=")
+    all(eg.map(tokenize)) shouldBe Seq(SYMBOL_LITERAL, EOF)
+  }
+
   "Local variable identifiers" should "be recognized as such" in {
     val eg = Seq("i", "x1", "old_value", "_internal", "_while")
     all(eg.map(tokenize)) shouldBe Seq(LOCAL_VARIABLE_IDENTIFIER, EOF)
@@ -662,4 +667,10 @@ class RubyLexerTests extends AnyFlatSpec with Matchers {
     val eg = Seq("$0", "$10", "$2", "$3")
     all(eg.map(tokenize)) shouldBe Seq(GLOBAL_VARIABLE_IDENTIFIER, EOF)
   }
+
+  "Assignment-like method identifiers" should "be recognized as such" in {
+    val eg = Seq("def x=", "def X=")
+    all(eg.map(tokenize)) shouldBe Seq(DEF, WS, ASSIGNMENT_LIKE_METHOD_IDENTIFIER, EOF)
+  }
+
 }