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

Fix JimplePrinter escaping to work like (old) Soot #647

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Fix JimplePrinter escaping to work like (old) Soot #647

wants to merge 14 commits into from

Conversation

swissiety
Copy link
Collaborator

@swissiety swissiety commented Jul 15, 2023 •
edited
Loading

  • boilerplate/startegy
  • add the keyword list to LegacyJimplePrinter to escape a ClassType properly
  • test it

closes #452, #622

@swissiety swissiety added the good first issue Good for newcomers label Jul 15, 2023
@swissiety swissiety changed the title Fix JimplePrinter escaping like (old) Soot Fix JimplePrinter escaping to work like (old) Soot Jul 15, 2023
@swissiety swissiety requested a review from stschott July 15, 2023 11:47
@codecov
Copy link

codecov bot commented Jul 15, 2023 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (71d5508) 63.86% compared to head (ea6e6fc) 64.47%.

❗ Current head ea6e6fc differs from pull request most recent head 2d2a222. Consider uploading reports for the commit 2d2a222 to get more accurate results

Files Patch % Lines
.../sootup/core/util/printer/LegacyJimplePrinter.java 88.88% 1 Missing and 1 partial ⚠️
.../sootup/core/util/printer/AbstractStmtPrinter.java 0.00% 1 Missing ⚠️
...n/java/sootup/core/util/printer/JimplePrinter.java 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@              Coverage Diff              @@
##             develop     #647      +/-   ##
=============================================
+ Coverage      63.86%   64.47%   +0.61%     
+ Complexity      3405     3311      -94     
=============================================
  Files            314      311       -3     
  Lines          15097    14773     -324     
  Branches        2566     2454     -112     
=============================================
- Hits            9641     9525     -116     
+ Misses          4549     4363     -186     
+ Partials         907      885      -22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stschott
Copy link
Collaborator

Old Soot escaped keywords with single quotes.
I added a test case that should be passing, if we change the escaping to use single quotes instead of double quotes

@swissiety
Copy link
Collaborator Author

did it? the StringUtil.getQuotedStringOf https://github.com/soot-oss/soot/blob/1ad74494974165e8b5f2286c90f218a00eadc243/src/main/java/soot/util/StringTools.java#L60 did add double quotes?

@swissiety
remove weird \r\n handling depending of current line.seperator system…
0bf24f3 
… property - escape when it occurs - which will only depending on the os.
@stschott
Copy link
Collaborator

did it? the StringUtil.getQuotedStringOf https://github.com/soot-oss/soot/blob/1ad74494974165e8b5f2286c90f218a00eadc243/src/main/java/soot/util/StringTools.java#L60 did add double quotes?

Yes, it does. I tested it with the old Soot. It is escaped to 'from'.

@swissiety
set escape symbol to single quote;
76edbd6 
unwindowsing the test i.e. remove \r from \n
@swissiety
make srcType final
cd7349b
@swissiety
improve stringbuilder
6bf4daa
@swissiety
add jimple to file util
c588372
@swissiety
Merge branch 'develop' into fixLegacyJimplePrinter
a4640a3 
# Conflicts:
#	sootup.java.core/src/test/java/sootup/java/core/printer/LegacyJimplePrinterTest.java
#	sootup.jimple.parser/src/main/java/sootup/jimple/parser/JimpleAnalysisInputLocation.java
@swissiety
cleanup modifierprint
2d2a222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stschott stschott Awaiting requested review from stschott

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add old-soot escaping for LegacyJimplePrinter
2 participants
@swissiety @stschott