Refactor and improve error matching #346
Conversation
kriegaex
force-pushed
the
refactor-and-improve-error-matching
branch
4 times, most recently
from
518fc05
to
026fd94
Compare
|
@olamy, I have split some and squashed other commits for the diffs per commit to be more focused and more easily digestable. Now, I am actually quite happy with the succession of commits and the PR overall. |
| // javac.properties-> javac.msg.proc.annotation.uncaught.exception | ||
| // (en JDK-8, ja JDK-8, zh_CN JDK-8, en JDK-21, ja JDK-21, zh_CN JDK-21, de JDK-21) | ||
| protected static final String[] ANNOTATION_PROCESSING_ERROR_HEADERS = { | ||
| "\n\nAn annotation processor threw an uncaught exception.\nConsult the following stack trace for details.\n\n", |
There was a problem hiding this comment.
Do new line separators should be a system depend?
There was a problem hiding this comment.
No, this is exactly how they are defined in the JDK properties across all JDK versions. There are other places in the code where we parse log lines and then re-print them with OS-dependent line breaks. Also the tests consider this.
|
I afraid that it can depend on JDK version ... and maybe on vendor |
|
@slawekjaranowski, of course it depends on JDK version, which is why I have properties for multiple JDK versions (e.g. 8, 21 or 8, 9, 21), where I found differences. The parametrised tests check all variants already. Feel free to write as many ITs as you wish. I will not. Reminding you again, however, for the third time or so, that it is difficult to reproduce most errors on all JDK versions in an IT, because as they get fixed, this can change, even between two separate update versions of the same JDK major. I can assure you that I reproduced each single error, which is how I managed to get sample stack traces for the tests in the first place. That was more tedious than to actually write the code, because I had to juggle many JDK versions and locales and in some cases even install specific, outdated JDK update versions just to reproduce an error in real life, because later it was already fixed in the same JDK. Compare the before vs. after situations: Before, there were
If you are afraid, that this PR makes anything worse, just do not merge it. But stop trying to burden me with work that should have been done by others in the past already. Much of that work I did in this PR, but new ITs are out of scope for me, because the existing ITs are so minimal, we could add at least two dozen new ones for everything that currently only has UT coverage. How about some Mojohaus developers getting active there? Not everything can be done by waiting for external contributors doing it. I want to give you my little finger, not the whole hand or arm. So try not to be too greedy. |
Add a few more helper methods, factor out error message constants into an inner class, get rid of some deprecated API usage, add or improve comments, remove excessive blank line usage. The scope of these changes is not the whole JavacCompiler class, but just the 'parseModern*' methods I am about to improve some more in subsequent commits. The functionality is unchanged for now, it really is a classical refactoring.
- Add more error message prefixes to class JavacCompiler.Messages - New method JavacCompiler.getTextStartingWithPrefix handles multi-line Java properties with placeholders and match them correctly in javac log output - Add test verifying that for slightly modified, non-matching error headers at least the stack traces are still recognised and added as error messages, despite the headers missing in those cases
"The system is out of resources. Consult the following stack trace for details."
"An input/output error occurred. Consult the following stack trace for details."
"A plugin threw an uncaught exception. Consult the following stack trace for details."
Before, "warning: " prefixes were already removed. This change brings both warnings and errors in sync with regard to message processing. Some tests had to be slightly adjusted to reflect the now cleaner error messages.
kriegaex
force-pushed
the
refactor-and-improve-error-matching
branch
from
0577d01
to
f4541e8
Compare
|
Can anybody else look at it? |
This PR supersedes the remaining part of #342 which was not part of #343, because the original description does not fit the content of that PR anymore.
Refactor javac output parsing code for better readability
Add a few more helper methods, factor out error message constants into an inner class, get rid of some deprecated API usage, add or improve comments, remove excessive blank line usage. The scope of these changes is not the whole JavacCompiler class, but just the
parseModern*methods I am about to improve some more in subsequent commits.The functionality is unchanged for now, it really is a classical refactoring. But the next commit depends on and builds on top of it:
Improve forked javac error matching accuracy and flexibility
JavacCompiler.MessagesJavacCompiler.getTextStartingWithPrefixhandles multi-line Java properties with placeholders and match them correctly in javac log outputRecognise new javac error header types
javac.msg.resource- "The system is out of resources."javac.msg.io- "An input/output error occurred."javac.msg.plugin.uncaught.exception- "A plugin threw an uncaught exception."@olamy, as you have seen at least one commit before, maybe at your convenience after the holidays you want to review this PR. Until then, I wish you some happy relaxing time off. 🙂