skuzzle/restrict-imports-enforcer-rule

Full compilation unit parsing errors with recent Java language features

bjmi opened this issue · 7 comments

bjmi commented

We have been using <parseFullCompilationUnit>true</parseFullCompilationUnit>(#57) smoothly for quite a while.
Now we encounter an error after converting an enum to local enum: (line 298,col 14) Parse error. Found "{", expected one of "," ";" "=" "@" "["

@Test
void enumMap() {
    enum E {
        A, B;
    }
    ...
}

The used JavaParser README states that Java language features up to version 15 are supported. It turns out we are affected by javaparser/javaparser#3990, a Java 16 language feature.

Therefore some improvements could be made:

  • It would be very useful to mention the filename in the error message to identify the cause of a build break much more easily.
  • The supported language versions can be added to the Limitation section in the readme.
  • As Java 17 LTS has been released for more than a year, are there other java parsers that support Java 17+?

What do you think?

Corr. stacktrace:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.2.1:enforce (enforce-java-imports) on project core-utils: Execution enforce-java-imports of goal org.apache.maven.plugins:maven-enforcer-plugin:3.2.1:enforce failed: (line 298,col 14) Parse error. Found "{", expected one of  "," ";" "=" "@" "["
[ERROR] Problem stacktrace :
[ERROR]   com.github.javaparser.GeneratedJavaParser.generateParseException(GeneratedJavaParser.java:13770)
[ERROR]   com.github.javaparser.GeneratedJavaParser.jj_consume_token(GeneratedJavaParser.java:13615)
[ERROR]   com.github.javaparser.GeneratedJavaParser.BlockStatement(GeneratedJavaParser.java:5707)
[ERROR]   com.github.javaparser.GeneratedJavaParser.Statements(GeneratedJavaParser.java:2673)
[ERROR]   com.github.javaparser.GeneratedJavaParser.Block(GeneratedJavaParser.java:5644)
[ERROR]   com.github.javaparser.GeneratedJavaParser.MethodDeclaration(GeneratedJavaParser.java:2074)
[ERROR]   com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceBodyDeclaration(GeneratedJavaParser.java:1675)
[ERROR]   com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceBody(GeneratedJavaParser.java:1187)
[ERROR]   com.github.javaparser.GeneratedJavaParser.ClassOrInterfaceDeclaration(GeneratedJavaParser.java:501)
[ERROR]   com.github.javaparser.GeneratedJavaParser.CompilationUnit(GeneratedJavaParser.java:152)
[ERROR]   com.github.javaparser.JavaParser.parse(JavaParser.java:125)
[ERROR]   com.github.javaparser.JavaParser.parse(JavaParser.java:231)
[ERROR]   com.github.javaparser.JavaParserAdapter.parse(JavaParserAdapter.java:79)
[ERROR]   com.github.javaparser.StaticJavaParser.parse(StaticJavaParser.java:173)
[ERROR]   de.skuzzle.enforcer.restrictimports.parser.lang.JavaCompilationUnitParser.parseCompilationUnit(JavaCompilationUnitParser.java:37)

Oh that looks very unfortunate.

As short term mitigation I could add a fall back to the "naive" analysis mode for files that can not properly be parsed.
The plugin would then emit a warning that analysis is inaccurate for certain files (and include the file name)

In the long run I believe it should/must be possible to find a parser that supports at least the latest LTS language features.

bjmi commented

I have no objection to your suggestion - sounds good.

I've implemented a draft. Will take some time to finalize this though. Result will then look like this if a file failed to parse:

[INFO] --- maven-enforcer-plugin:3.0.0:enforce (default-cli) @ fallback-on-failure ---
[WARNING] EXPERIMENTAL FEATURE enabled. You have enabled full-compilation-unit parsing. Please be aware that experimental features might get removed. Please share your feedback!
[WARNING] 
Banned imports analysis completed with warnings. Results may be inaccurate!

	Failed to parse in 'full-compilation-unit' mode. Analysis might be inaccurate:
		de/skuzzle/test/DataImporter.java

	Run the build with debug information to see error details about the warning

Analysis of 2 files took less than 1 second

[WARNING] Detected banned imports will not fail the build as the 'failBuild' flag is set to false!

If line-by-line parsing detects a failure in a file that could not be properly parsed by JavaParser, the message would look like this. (Note the little warning (!) icon)

[WARNING] Rule 0: org.apache.maven.plugins.enforcer.RestrictImports failed with message:

Banned imports detected:

	in file: de/skuzzle/test/DataImporter.java (!)
		java.util.LinkedList 	(Line: 4, Matched by: java.util.*)

Banned imports analysis completed with warnings. Results may be inaccurate!

	Failed to parse in full-compilation-unit mode. Analysis might be inaccurate:
		de/skuzzle/test/DataImporter.java

	Run the build with debug information to see error details about the warning

Analysis of 2 files took less than 1 second

The mitigation is now available in latest snapshot build: <version>2.2.0-SNAPSHOT</version>

Mitigation is now released as 2.2.0. Leaving this ticket open until I found a proper solution

In the long run I believe it should/must be possible to find a parser that supports at least the latest LTS language features.

CongoCC is a parser generator that comes with a Java parser that supports up to JDK 20 syntax. You can find more information here. Actually, if you want to try it, here is a magical incantation that should work:

   git clone https://github.com/congo-cc/congo-parser-generator.git congo
   cd congo/examples/java
   ant test
   java JParse

As I said, it is part of CongoCC, which is itself a parser generator, but the Java grammar/parser that is built in can be used perfectly well by 3rd party projects. If you have any question, though, by all means, we're eager to help. The best venue to get your questions is answered is probably our discussion forum.

That looks quite promising indeed. I will have a deeper look when I have some spare time. Or maybe someone can be found to send a PR to integrate this. This plugin's API is pretty simple and it should be quite straight forward to replace the used parser 😉