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

fixed(prettyprint): parentheses are missing when pretty-pretting some ternary expressions #927

Merged
merged 3 commits into from
Nov 3, 2016
Merged

Conversation

arnobl
Copy link
Contributor

@arnobl arnobl commented Nov 3, 2016

The current prettyprint of ternary expressions (CtConditional) adds parentheses around the condition only when the parent is an assignment. The added test show that parentheses are also required on variable declaration.

The test shows that the mandatory parentheses that originally surround the condition are removed when pretty-printing the spooned code. This leads to ambiguous code that does not compile.
The fix adds the parentheses when the parent is a variable declaration.

Maybe the parentheses should be always added.

CtStatement compile = snippet.compile();

// Pretty-printing the Spooned code snippet and compiling the resulting code.
launcher = new Launcher();
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you need a new Launcher?

launcher = new Launcher();
snippet = launcher.getFactory().Code().createCodeSnippetStatement(compile.toString());
// If it throws an exception, the printed code is not valid.
snippet.compile();
Copy link
Collaborator

Choose a reason for hiding this comment

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

add an assert assertEquals(compile, snippet.compile())?

@monperrus monperrus merged commit e44ef7b into INRIA:master Nov 3, 2016
@monperrus
Copy link
Collaborator

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants