Skip to content

Java: Add PrintAst#4261

Merged
joefarebrother merged 11 commits intogithub:mainfrom
joefarebrother:printAST-java
Sep 16, 2020
Merged

Java: Add PrintAst#4261
joefarebrother merged 11 commits intogithub:mainfrom
joefarebrother:printAST-java

Conversation

@joefarebrother
Copy link
Contributor

@joefarebrother joefarebrother commented Sep 14, 2020

Resolves #3955

@joefarebrother joefarebrother requested a review from a team September 14, 2020 12:17
@github-actions github-actions bot added the Java label Sep 14, 2020
aibaars
aibaars reviewed Sep 14, 2020
View reviewed changes
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look through the code. Looks good in general.

java/ql/src/semmle/code/java/PrintAst.ql
@@ -0,0 +1,20 @@
/**
* @name Print AST
Copy link
Contributor

@aibaars aibaars Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed for anything?

Copy link
Contributor Author

@joefarebrother joefarebrother Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the file that the qlref of the test cases points to

Copy link
Contributor

@aibaars aibaars Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes of course. Quite useful indeed.

@joefarebrother
Copy link
Contributor Author

joefarebrother commented Sep 15, 2020

I believe the failing test is due to some semmle-code tests depending on the old PrintAst.qll.
Would the best course of action be to rename my PrintAst.qll, so the name can be used as a deprecated alias of PrettyPrintAst.qll?

aschackmull
aschackmull reviewed Sep 15, 2020
View reviewed changes
@aschackmull
Copy link
Contributor

aschackmull commented Sep 15, 2020

Would the best course of action be to rename my PrintAst.qll, so the name can be used as a deprecated alias of PrettyPrintAst.qll?

I think the cleanest is to get the rename of the old PrintAst.qll done in a separate PR, or two PRs really, as you'll need one PR in this repo and one in the semmle-code repo and then merge the two synchronously.

@joefarebrother joefarebrother mentioned this pull request Sep 15, 2020
Merged
@joefarebrother joefarebrother merged commit 4f70af5 into github:main Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aschackmull aschackmull aschackmull approved these changes

+1 more reviewer

@aibaars aibaars aibaars left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Java

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement PrintAST for Java

3 participants

@joefarebrother @aschackmull @aibaars

Comments