[rubysrc2cpg] Parser and ast changes for empty right hand side of association #3226
Conversation
...frontends/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/parser/MethodDefinitionTests.scala
Outdated
Show resolved
Hide resolved
...s/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/passes/ast/SimpleAstCreationPassTest.scala
Outdated
Show resolved
Hide resolved
...s/rubysrc2cpg/src/test/scala/io/joern/rubysrc2cpg/passes/ast/SimpleAstCreationPassTest.scala
Outdated
Show resolved
Hide resolved
|
@xavierpinho , addressed your review. |
| cpg.call.size shouldBe 2 | ||
| val List(callNode, operatorNode) = cpg.call.l | ||
| callNode.name shouldBe "foo" | ||
| operatorNode.name shouldBe "<operator>.activeRecordAssociation" |
There was a problem hiding this comment.
Out of this PR's responsibility, but might as well take advantage of a sample at hands.
I think there shouldn't be an operator here. The sample foo(bar:) should mean the same as foo(bar: bar), in which case there wouldn't be an operator, but rather a call to bar.
Do you have a different opinion?
There was a problem hiding this comment.
foo(bar:bar) would have an operator as well, right? Imo, it helps when there is something present on the RHS.
There was a problem hiding this comment.
foo(bar:bar)would have an operator as well, right? Imo, it helps when there is something present on the RHS.
Not sure. Would need to see how keyword parameters are handled in other frontends. As a first guess, I'd say it shouldn't have, as we are only mapping the parameter name bar with the call bar, not really creating another call.
There was a problem hiding this comment.
Not sure about this.
|
@xavierpinho, if there are no blockers, can you please merge? |
There are merge conflicts. Please rebase or merge with master and we should be good |
|
@xavierpinho Merged master. |
…ruby/parser-3189
…oern into ruby/parser-3189
@xavierpinho, can you please review?