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.

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

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

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

Conversation

swissiety
Copy link
Collaborator

@swissiety swissiety commented Jul 15, 2023

  • 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

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

… 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'.

unwindowsing the test i.e. remove \r from \n
# Conflicts:
#	sootup.java.core/src/test/java/sootup/java/core/printer/LegacyJimplePrinterTest.java
#	sootup.jimple.parser/src/main/java/sootup/jimple/parser/JimpleAnalysisInputLocation.java
@stschott stschott marked this pull request as draft September 5, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add old-soot escaping for LegacyJimplePrinter
2 participants