Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RubyParser: emit ASSIGNMENT_LIKE_METHOD_IDENTIFIERS #3296

Merged
merged 3 commits into from
Jul 28, 2023
Merged

RubyParser: emit ASSIGNMENT_LIKE_METHOD_IDENTIFIERS #3296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2daeac8
RubyParser: only emit assignment-like identifiers after `DEF`
xavierpinho Jul 27, 2023
4ce76d3
scalafmt
xavierpinho Jul 27, 2023
2e3ee6f
Merge branch 'master' into xavierp/ruby-assignment-like-method-token
xavierpinho Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyLexer.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion joern-cli/frontends/rubysrc2cpg/src/main/antlr4/io/joern/rubysrc2cpg/parser/RubyParser.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ definedMethodName
;

assignmentLikeMethodIdentifier
: (CONSTANT_IDENTIFIER | LOCAL_VARIABLE_IDENTIFIER) EQ
: ASSIGNMENT_LIKE_METHOD_IDENTIFIER
;

methodName
Expand Down
22 changes: 5 additions & 17 deletions ...li/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstCreator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {
Expand Down
99 changes: 97 additions & 2 deletions ...ntends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,7 @@ class MethodDefinitionTests extends RubyParserAbstractTest {
| SimpleMethodNamePart
| DefinedMethodName
| AssignmentLikeMethodIdentifier
| foo2
| =
| foo2=
| MethodParameterPart
| (
| Parameters
Expand All @@ -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
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions ...cli/frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/RubyLexerTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

}