-
Notifications
You must be signed in to change notification settings - Fork 372
[ruby] Add Support for ERB files #5447
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
6608084
[ruby] Add support for basic ERB files (#5404)
AndreiDreyer 545f74d
[ruby] rework erb support. Added templateOutRaw and templateOutEscape…
AndreiDreyer e1d651b
[ruby] Add .html.erb files as config files for ruby
AndreiDreyer 82a37a2
review comments
AndreiDreyer e9401dc
Fixed ERB lowering implementation. Updated test expectations
AndreiDreyer e8813ca
Updated comments
AndreiDreyer 03c79d9
remove test
AndreiDreyer 0db1370
upgrade ruby-ast-gen version
AndreiDreyer e63a443
upgrade ruby-ast-gen version, remove unnecessary whitespace
AndreiDreyer 799c7a6
[ruby] update code string and tests for joern__buffer changes to self…
AndreiDreyer 903ea4d
[ruby] update ERB tests to reflect no line numbers being set from rub…
AndreiDreyer f9227ef
[ruby] remove self parameter from lambda, capture from outside instead
AndreiDreyer bf9c788
[ruby] Add empty expression for if statement with no expressions in b…
AndreiDreyer 415d890
[ruby] Only look for the closest self in the outerscope in closure body
AndreiDreyer 0b1983e
[ruby] capture self variable
AndreiDreyer 7b74888
fix: resolve multiple self identifier refOuts bug
78d6684
chore: add return type
ee4a305
refactor: scalafmt run
f22bd19
fix: resolve incorrect closure binding refOut bug
9c6ae12
feat: use joern__buffer_append
4923261
chore: scalafmt
b75c092
fix: recognise joern_inner_buffer appends as erb calls
73101ee
fix: retain joern__buffer_append args' receiver ast
9c9bed7
improvement: turn lookupSelfInOuterScope into wrapper
721e174
improvement: override span instead of using 2 parameter lists
fe6f43d
refactor: remove debug lines in test
cc6f13b
improvement: move typeFullName to outer scope
926e851
improvement: use buffer_append constant
504464f
improvement: use Option.when
1650c2f
improvement: remove case for self
45a684f
improvement: remove ERB call special case
30fc06a
improvement: use camelCase for ERB buffer symbols and methods
1778915
refactor: scalafmt
dd5db48
cleanup: remove unused variable
f19c5ab
improvement: rename isErbCall
de1453a
improvement: avoid unnecessary call
5319cc8
refactor: refactor block
39edae8
fix: don't generate receiver AST if isStatic
f0fdf28
fix: add tmp assignments to tests
0b8bd4d
Revert "fix: add tmp assignments to tests"
d524e45
Revert "fix: don't generate receiver AST if isStatic"
5c2a7a4
chore: use Option.unless
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 2 additions & 2 deletions
4
joern-cli/frontends/rubysrc2cpg/src/main/resources/application.conf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| rubysrc2cpg { | ||
| ruby_ast_gen_version: "0.33.0" | ||
| ruby_ast_gen_version: "0.58.0" | ||
| joern_type_stubs_version: "0.6.0" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole
if-cascade should also simplify to a single callcallAst(call, argumentAsts, base = Option(baseAst), receiver = receiverAstOpt)- after adjusting the condition used for creatingreceiverAstOptThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that before and found that
.copy(receiverEdges = Nil)was done to remove the receiver edges coming from thebaseAst.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we do the same as with the receiver? Conditionally create it only in case we want to actually make use of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the creation of the receiver stores some state somewhere that's then used in the creation of
baseAst. I'm getting a couple of failed tests if I don't create the receiver whenisStaticis true. The difference I spotted is thatbaseAsthas more nodes in it whenastForFieldAccessis not called for the receiver. I'm not 100% sure what state it is that's at play here but I'm busy debugging to try figure that out.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might have something to do with this
TODO->joern/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/astcreation/AstForExpressionsCreator.scala
Lines 345 to 363 in 39947b2
handleTmpGengets called during the creation of the receiver (astForFieldAccess) and triggers this flow. It's then also triggered when thebaseAstis created. I don't understand enough yet to know why these side-effects exist yet the result ofastForFieldAccessisn't used whenisStaticis true :/.I'll see if I can figure out how to make this better. I have a call with @ml86 later and will probably ask for some guidance on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ml86 on closer inspection, I think this implementation is okay. The way the method
createMemberCallwas doing its operations was actually incorrect. It was resulting in sometmpassignments being omitted. You can have a look at my diff here to double check if what I did makes sense: https://github.com/joernio/joern/pull/5447/files/5319cc841d17e36954830f09782ecbf2438c99f7..f0fdf28d92ae5f751eedb93d76379c9d078e159b