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

Support Java 21 string template syntax #13830

Closed
nandorholozsnyak opened this issue Oct 4, 2023 · 12 comments · Fixed by #13991
Closed

Support Java 21 string template syntax #13830

nandorholozsnyak opened this issue Oct 4, 2023 · 12 comments · Fixed by #13991

Comments

@nandorholozsnyak
Copy link

I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

❯ javac --enable-preview --release 21 src/main/java/HelloWorld.java
Note: src/main/java/HelloWorld.java uses preview features of Java SE 21.
Note: Recompile with -Xlint:preview for details.

/var/tmp $ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">

    <module name="NewlineAtEndOfFile">
        <property name="lineSeparator" value="lf"/>
    </module>

    <module name="TreeWalker">
        <module name="UnusedImports"/>
    </module>
</module>


/var/tmp $ cat YOUR_FILE.java
public class HelloWorld {
    public static void main(String[] args) {
        int x = 10;
        int y = 20;
        String result = STR."\{x} + \{y} = \{x + y}";
        System.out.println(result);
    }
}

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-X.XX-all.jar -c config.xml YOUR_FILE.java
> java $RUN_LOCALE -jar checkstyle-10.12.4-all.jar -c checkstyle.xml src/main/java/HelloWorld.java 

Starting audit...
com.puppycrawl.tools.checkstyle.api.CheckstyleException: Exception was thrown while processing src/main/java/HelloWorld.java
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:309)
        at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:226)
        at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:415)
        at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:338)
        at com.puppycrawl.tools.checkstyle.Main.execute(Main.java:195)
        at com.puppycrawl.tools.checkstyle.Main.main(Main.java:130)
Caused by: com.puppycrawl.tools.checkstyle.api.CheckstyleException: IllegalStateException occurred while parsing file /home/xxx/VCS/own/checkstyle-java21-reproducer/src/main/java/HelloWorld.java.
        at com.puppycrawl.tools.checkstyle.JavaParser.parse(JavaParser.java:105)
        at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:152)
        at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:101)
        at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:337)
        at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:296)
        ... 5 more
Caused by: java.lang.IllegalStateException: 6:32: mismatched input '}' expecting ';'
        at com.puppycrawl.tools.checkstyle.JavaParser$CheckstyleErrorListener.syntaxError(JavaParser.java:255)
        at org.antlr.v4.runtime.ProxyErrorListener.syntaxError(ProxyErrorListener.java:41)
        at org.antlr.v4.runtime.Parser.notifyErrorListeners(Parser.java:544)
        at org.antlr.v4.runtime.DefaultErrorStrategy.reportInputMismatch(DefaultErrorStrategy.java:327)
        at org.antlr.v4.runtime.DefaultErrorStrategy.reportError(DefaultErrorStrategy.java:139)
        at com.puppycrawl.tools.checkstyle.CheckstyleParserErrorStrategy.recoverInline(CheckstyleParserErrorStrategy.java:38)
        at org.antlr.v4.runtime.Parser.match(Parser.java:208)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.blockStatement(JavaLanguageParser.java:6190)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.block(JavaLanguageParser.java:6101)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.methodBody(JavaLanguageParser.java:2940)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.methodDeclaration(JavaLanguageParser.java:2898)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.memberDeclaration(JavaLanguageParser.java:2745)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.classBodyDeclaration(JavaLanguageParser.java:2671)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.classBody(JavaLanguageParser.java:2477)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.classDeclaration(JavaLanguageParser.java:1096)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.types(JavaLanguageParser.java:753)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.typeDeclaration(JavaLanguageParser.java:667)
        at com.puppycrawl.tools.checkstyle.grammar.java.JavaLanguageParser.compilationUnit(JavaLanguageParser.java:414)
        at com.puppycrawl.tools.checkstyle.JavaParser.parse(JavaParser.java:99)
        ... 9 more
Caused by: org.antlr.v4.runtime.InputMismatchException
        ... 23 more
Checkstyle ends with 1 errors.

in place of the last 2 commands above.


Java 21 is out, and it has the new String Templates as a preview feature. I did not really go deeper but if the configuration is having TreeWalker module, with at least one entry, then the build fails with the above-mentioned error, and if it happens inside a Maven build it just fails. It fails at the following line: String result = STR."\{x} + \{y} = \{x + y}";

If the TreeWalker is not present or empty, it finishes successfuly. Will the Java 21 based preview features be supported in upcoming releases?


@rnveach rnveach added the antlr label Oct 4, 2023
@nrmancuso
Copy link
Member

Will the Java 21 based preview features be supported in upcoming releases?

We typically do add support for preview features (especially useful ones like String templates), I can take this up soon.

@nrmancuso nrmancuso self-assigned this Oct 13, 2023
@nandorholozsnyak
Copy link
Author

Will the Java 21 based preview features be supported in upcoming releases?

We typically do add support for preview features (especially useful ones like String templates), I can take this up soon.

It sounds good. Thank you

@nrmancuso
Copy link
Member

nrmancuso commented Oct 23, 2023

New grammar summary (adapted from here):

TemplateExpression:
  TemplateProcessor . TemplateArgument

TemplateProcessor:
  Expression

TemplateArgument:
  Template
  StringLiteral
  TextBlock

Template:
  StringTemplate
  TextBlockTemplate

StringTemplate:
  Resembles a StringLiteral but has one or more embedded expressions,
    and can be spread over multiple lines of source code

TextBlockTemplate:
  Resembles a TextBlock but has one or more embedded expressions

Reading:

Attention: mutliline strings delimited with only single " are possible.

Example:

String time = STR."The current time is \{
    //sample comment - current time in HH:mm:ss
    DateTimeFormatter
      .ofPattern("HH:mm:ss")
      .format(LocalTime.now())
	}.";

Note the expression within the template:

    //sample comment - current time in HH:mm:ss
    DateTimeFormatter
      .ofPattern("HH:mm:ss")
      .format(LocalTime.now())

@romani @rnveach thoughts about also parsing the expressions within the template? I suppose it would be good to first get templates working in general.

Edit: it is required for us to support parsing of templated expressions, since line wrapping of single " string templates requires us to be aware of the structure of the string template.

Example:

➜  src cat Test.java 
public class Test {
    int x = 10;
    int y = 20;
    String result = STR."\{x} + \{y} = \{x + y}";
}
➜  src javac --enable-preview --release 21 Test.java
Note: Test.java uses preview features of Java SE 21.
Note: Recompile with -Xlint:preview for details.

➜  src cat Test.java                                
public class Test {
    int x = 10;
    int y = 20;
    String result = STR."\{
            x} + \{y} = \{x + y}";
}
➜  src javac --enable-preview --release 21 Test.java
Note: Test.java uses preview features of Java SE 21.
Note: Recompile with -Xlint:preview for details.


➜  src cat Test.java                                
public class Test {
    int x = 10;
    int y = 20;
    String result = STR."\{x}
            + \{y} = \{x + y}";
}
➜  src javac --enable-preview --release 21 Test.java
Test.java:4: error: string template is not well formed
    String result = STR."\{x}
                        ^
Test.java:5: error: illegal character: '\'
            + \{y} = \{x + y}";
              ^
Test.java:5: error: not a statement
            + \{y} = \{x + y}";
                ^
Test.java:5: error: ';' expected
            + \{y} = \{x + y}";
                 ^
Test.java:5: error: illegal start of type
            + \{y} = \{x + y}";
                   ^
Test.java:5: error: illegal character: '\'
            + \{y} = \{x + y}";
                     ^
Test.java:5: error: not a statement
            + \{y} = \{x + y}";
                         ^
Test.java:5: error: ';' expected
            + \{y} = \{x + y}";
                            ^
Test.java:5: error: unclosed string literal
            + \{y} = \{x + y}";
                             ^
Test.java:7: error: reached end of file while parsing
Note: Test.java uses preview features of Java SE 21.
Note: Recompile with -Xlint:preview for details.
10 errors


@romani
Copy link
Member

romani commented Oct 28, 2023

https://openjdk.org/jeps/430

STR is a public static final field that is automatically imported into every Java source file.

So it a type that has such operator-method " and """.
I highly recommend to add support for without parsing of content, as it is like nested java file :).

And in separate iteration add support of parsing it's content.

@robtimus
Copy link

Keep in mind that STR is not the only processor; there are also StringTemplate.RAW, FormatProcessor.FMT and anything returned by FormatProcessor.create in the JRE, and it's also possible to create custom implementations. Therefore, any parsing would need to do one of two things:

  • Verify that the LHS of ." or .""" implements StringTemplate.Processor
  • Don't verify anything about the LHS, just let the compiler worry about that

@romani
Copy link
Member

romani commented Oct 28, 2023

Verify that the LHS of ." or .""" implements StringTemplate.Processor

Checkstyle parser is not type aware https://checkstyle.org/writingchecks.html#Limitations . We rely on compiler to do verification of this. For us it will looks like all objects can have ." or .""".

@robtimus
Copy link

So that leaves option 2: let the compiler worry about that.

@nrmancuso nrmancuso changed the title Java 21 based String Template (Preview) analyzes kills the Checkstyle process Support Java 21 string template syntax Nov 3, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 10, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 11, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 27, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 27, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 28, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 28, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 28, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 30, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 30, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Dec 31, 2023
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 2, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 3, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 3, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 3, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 3, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 11, 2024
nrmancuso added a commit to nrmancuso/checkstyle that referenced this issue Jan 14, 2024
@mikepapadim
Copy link

hello, I just bumped into this issue as well and I wonder what is the status on that or if there is any workaround atm?
thanks

@romani
Copy link
Member

romani commented Jan 15, 2024

@mikepapadim , please build jar from #13991 and run on your sources to confirm that it works, it will speed up acceptance of such PR.

https://github.com/checkstyle/checkstyle/wiki/How-to-run-certain-phases-and-validations#how-to-generate-all-binaries-and--alljar--too

Or simply mvn install.

@nrmancuso
Copy link
Member

@mikepapadim this might be helpful until we get these PRs merged: #12542 (comment)

@mikepapadim
Copy link

Thank you for the quick replies! I'll give it a go

@mikepapadim
Copy link

@mikepapadim this might be helpful until we get these PRs merged: #12542 (comment)

Thanks! that did the trick for now

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

Successfully merging a pull request may close this issue.

6 participants