codehaus-plexus / plexus-compiler

Plexus compiler a layer on top of compilers and used by maven-compiler-plugin
https://codehaus-plexus.github.io/plexus-compiler/
34 stars 58 forks source link

Output information needed to suppress Xlint warnings #183

Open cowwoc opened 2 years ago

cowwoc commented 2 years ago

Per Robert Scholte https://issues.apache.org/jira/browse/MCOMPILER-367 can only be fixed in this plugin.

Here is the problem description:

I will attach a testcase showing the following problem.

If you compile a project that has compiler warnings, javac's output indicates which Xlint option will suppress the warning. But when compiling the same code under this plugin, this vital information is suppressed so the user has a difficult time figuring out which option to use.

Expected behavior: output should indicate which Xlint option corresponds to the warning.

Running mvn compile on the testcase results in:

[INFO] — maven-compiler-plugin:3.8.0:compile (default-compile) @ testcase — [INFO] Changes detected - recompiling the module! [INFO] Compiling 2 source files to C:\Users\Gili\Documents\3rdparty\testcase\target\classes [WARNING] /C:/Users/Gili/Documents/3rdparty/testcase/src/main/java/module-info.java:[2,32] module not found: someOtherModule I then run the following compilation:

javac -d C:\Users\Gili\Documents\3rdparty\testcase\target\classes -classpath C:\Users\Gili\Documents\3rdparty\testcase\target\classes; -sourcepath C:\Users\Gili\Documents\3rdparty\testcase\src\main\java;C:\Users\Gili\Documents\3rdparty\testcase\target\generated-sources\annotations; -s C:\Users\Gili\Documents\3rdparty\testcase\target\generated-sources\annotations -g -target 11 -source 11 -encoding UTF-8 com\company\SomeClass.java module-info.java

I get:

module-info.java:2: warning: [module] module not found: someOtherModule exports com.company to someOtherModule; ^ 1 warning Notice that the javac output includes "[module]" at the beginning of the warning message. This indicates that "-Xlint:-module" will suppress this warning. I am expecting maven-plugin-compiler to show this information in its output.

Please let me know if have any questions and/or would like me to attach the testcase here.

olamy commented 2 years ago

not sure how to fix that..... We use the result from compilation with this class https://docs.oracle.com/en/java/javase/17/docs/api/java.compiler/javax/tools/Diagnostic.html

in case of error/warning, we use the method diagnostic.getMessage(Locale.getDefault()) (https://github.com/codehaus-plexus/plexus-compiler/blob/f4d60742f1375e3a6e07d9582f92a633c1fbbef9/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavaxToolsCompiler.java#L146) in this case the method returns module not found: someOtherModule but at the same time diagnostic.toString() is returning:

/Volumes/workspace/dev/sources/plexus/plexus-compiler/plexus-compiler-its/target/it/missing-warnings/src/main/java/module-info.java:2: warning: [module] module not found: someOtherModule
    exports com.company to someOtherModule;
                           ^

but the Diagnostic class doesn't have any way to retrieve the second line.

cowwoc commented 2 years ago

@olamy I am asking for you to include [module] from the first line, not the second line. Did you mean you can't retrieve this value?

olamy commented 2 years ago

@olamy I am asking for you to include [module] from the first line, not the second line. Did you mean you can't retrieve this value?

even this is not returned by diagnostic.getMessage( Locale.getDefault() ); seems like a jdk bug to me the only way to get it is to use <forceJavacCompilerUse>true</forceJavacCompilerUse>. as it's not using the javax.tools API and simply try to parse the output.

cowwoc commented 2 years ago

even this is not returned by diagnostic.getMessage( Locale.getDefault() ); seems like a jdk bug to me

I've forwarded this question to the JDK team. I'll let you know once I hear back.

olamy commented 2 years ago

even this is not returned by diagnostic.getMessage( Locale.getDefault() ); seems like a jdk bug to me

I've forwarded this question to the JDK team. I'll let you know once I hear back.

any link?

cowwoc commented 2 years ago

any link?

Not yet. It was a private email correspondence. I'll let you know once I receive a response.

cowwoc commented 2 years ago

I just received a response from the dev team:

The lint info should always be included in the diagnostic string, assuming that the plugin uses the normal API to get the text form of a diagnostic. The format is controlled by a series of internal printf-like strings, and in all cases the lint info is provided. For this to work as expected, it's important for the client to call Diagnostic.getMessage(Locale) . If they bypass that and roll their own conversion, then that's up to them, so you might try and find out exactly how they are generating the string that does not have the lint info.

Does this help?

olamy commented 2 years ago

I just received a response from the dev team:

The lint info should always be included in the diagnostic string, assuming that the plugin uses the normal API to get the text form of a diagnostic. The format is controlled by a series of internal printf-like strings, and in all cases the lint info is provided. For this to work as expected, it's important for the client to call Diagnostic.getMessage(Locale) . If they bypass that and roll their own conversion, then that's up to them, so you might try and find out exactly how they are generating the string that does not have the lint info.

Does this help?

nope as we definitely use Diagnostic.getMessage(Locale)

https://github.com/codehaus-plexus/plexus-compiler/blob/84e2dab07521a610591ac0a878b1bccd7a28424b/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavaxToolsCompiler.java#L146

cowwoc commented 2 years ago

Here is the latest reply on my end:

Dev team will do more investigation. Will get back to you once I have some update.

Hopefully they'll be able to reproduce the problem.

cowwoc commented 2 years ago

@olamy This issue is now tracked by https://bugs.openjdk.org/browse/JDK-8292634 with an expected release date of JDK 21.

In the meantime, the unofficial recommendation (per https://bugs.openjdk.org/browse/JDK-8292634?focusedCommentId=14526780&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14526780) is to implement a workaround by parsing the output of toString().

Is this something you can do until JDK 21 is out?