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

feature: add support for pretty-printing Java 15 text blocks #3664

Merged
merged 24 commits into from
Nov 4, 2020

Conversation

LakshyAAAgrawal
Copy link
Contributor

Implement CtTextBlock class and add factories
Closes #3605

@LakshyAAAgrawal
Copy link
Contributor Author

Todo:

  • Modify JDTBasedSpoonCompiler to create TextBlock when parsed literal is TextBlock.
  • Modify existing tests to check creation CtTextBlock
  • Write additional tests

@LakshyAAAgrawal
Copy link
Contributor Author

LakshyAAAgrawal commented Oct 24, 2020

The package org.eclipse.jdt.internal.compiler.ast does provide TextBlock class as part of the package.
However, org.eclipse.jdt.internal.compiler.ASTVisitor class, which is the base class for JDTTreeBuilder, the visit and endvisit methods are only defined for StringLiteral class in JDT AST, but not for TextBlock. I tried to retrieve the token source for it too from the StringLiteral, however, it just gives the string content, and not the source token. If the source token was provided, we could simply detect 3 consecutive (") in the beginning and confirm it being a TextBlock.

However, org.eclipse.jdt.core.dom.ASTVisitor.java does provide visitor for TextBlock. This has not been extended by any class in the spoon project.

I am taking a look at how to solve this..

@MartinWitt
Copy link
Collaborator

MartinWitt commented Oct 25, 2020

If the source token was provided, we could simply detect 3 consecutive (") in the beginning and confirm it being a TextBlock.

JLS 3.10.6 stats the grammar for textblocks " " " {TextBlockWhiteSpace} LineTerminator {TextBlockCharacter} " " " and TextBlockWhiteSpace == Whitespace. So the regex """\s*\n.*""" should do the trick. Maybe it helps you

@LakshyAAAgrawal
Copy link
Contributor Author

LakshyAAAgrawal commented Oct 25, 2020

If the source token was provided, we could simply detect 3 consecutive (") in the beginning and confirm it being a TextBlock.

JLS 3.10.6 stats the grammar for textblocks " " " {TextBlockWhiteSpace} LineTerminator {TextBlockCharacter} " " " and TextBlockWhiteSpace == Whitespace. So the regex """\s*\n.*""" should do the trick. Maybe it helps you

Thank you very much for the reply. Actually my question had been that I am not able to get the token source from a call to getSource in the first place. For example, if the textblock was:

String s = """
        Test String""";

Then calling stringLiteral.source returns the string content - "Test String" instead of the token source """"\n Test String"""".

But the issue has been resolved now in the latest commit. The stringLiteral that was being passed to the visitor was actually an instanceof TextBlock from jdt ast, and I checked it using instanceof.

@LakshyAAAgrawal LakshyAAAgrawal changed the title WIP: feature: Implement CtTextBlock for Java 15 support Review: feature: Implement CtTextBlock for Java 15 support Nov 1, 2020
Copy link
Collaborator

@monperrus monperrus left a comment

Choose a reason for hiding this comment

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

Really cool PR! LGTM but a minor comment to fix.

public CtTextBlock createTextBlock(String value) {
CtTextBlock textblock = factory.Core().createTextBlock();
textblock.setValue(value);
// TODO set textblock.setType
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix or remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for pointing out. I have made some changes. Kindly give them a look.

@LakshyAAAgrawal
Copy link
Contributor Author

I believe that with this PR, Java 15 support in Spoon is complete. Relevant changes in the webpage/documentation can be made either now, or upon the next release.

@monperrus monperrus changed the title Review: feature: Implement CtTextBlock for Java 15 support feature: add support for pretty-printing Java 15 text blocks Nov 4, 2020
@monperrus monperrus merged commit 06ad7f0 into INRIA:master Nov 4, 2020
@monperrus
Copy link
Collaborator

Thanks @LakshyAAAgrawal impressive work.

@LakshyAAAgrawal
Copy link
Contributor Author

Thanks a lot for merging @monperrus

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.

add support for Java 15
3 participants