Community
Participate
Working Groups
org.eclipse.jdt.internal.compiler.apt.dispatch.BatchFilerImpl.getResource(Location, CharSequence, CharSequence) can throw a NullPointerException in case the resource does not exist. This happens because org.eclipse.jdt.internal.compiler.apt.util.EclipseFileManager.getFileForInput(Location, String, String) returns null in this case and no null check is made on the return value before invoking toUri(). javac throws a FileNotFoundException in this case so this patch introduces the same behavior. I could provide a test case if neede.
Created attachment 189915 [details] patch that throws FileNotFoundException
Thanks for finding that and for providing the patch! Filer.getResource is documented to throw an IOException if the file cannot be found, so I agree with the proposed change. Olivier, I wonder if instead of catching the null and converting it to a FileNotFoundException in the dispatcher, we should just throw FileNotFoundException directly from EclipseFileManager.getFileForInput()? I don't have the source code handy right now but I'll try to look at this late tonight or tomorrow unless you get a chance sooner.
A test case would be welcome. Walter, you also need to set the iplog property and the copyrights/contribution of this file need to be updated. We should indeed investigate if the exception should not be thrown by the file manager directly. It would be good to check what javac is doing in this case.
Walter, Can you take care of this for M6 (next week) or should I do it ?
I think I can get it in for M6. I want to do a little research first to see if we should throw an exception from getFileForInput().
The file manager that is used by BatchFilerImpl may be passed in by the compiler and may even come from an outside implementation via the Tools (JSR199) API. The JavaFileManager.getFileForInput() method is documented by Sun/Oracle as "might return null if the file does not exist" - how's that for precise API documentation? "Might"?? So it seems we cannot rely on the file manager to throw an exception, and we need to throw it ourselves inside BatchFilerImpl, as in Philippe's original suggestion.
This should be released with a regression test.
Yep, I agree. I didn't do it last night because I didn't have time to write the test... will try again tonight.
Created attachment 190382 [details] sample processor A really simple sample processor that triggers the bug.
Created attachment 190383 [details] sample processor user a really simple sample processor user
I added two files. A really simple processor and user that should trigger the issue. They work fine when running in Eclipse JDT but should throw a NullPointerException when running the batch compiler. I hope this helps you write a regression test.
Ok, I'll prepare a patch with a regression test and I'll request a review from Walter.
(In reply to comment #0) > javac throws a FileNotFoundException in this case so this patch introduces the > same behavior. Philippe, what version of javac have you tried ? When I try it with a jdk6 version, the file not found exception is not thrown. If I try with a JDK7 (b131), then it works.
Created attachment 190406 [details] Proposed fix + regression test Patch that contains the fix and a regression test. The system compiler is not tested as I could not get the test to pass on a JDK6 VM. It works on a JDK7, but this is not the VM used for the Eclipse tests. Walter, please review. Philippe, I would like to understand how you got this to work with javac.
Created attachment 190408 [details] Proposed fix + regression test Same patch removing an unused import
I will review tonight & release for M6 warmup build if it looks good.
Committed to HEAD; will be released in M6.
Oliver you're right. I went over the code with jdk6 again and the the following line messenger.printMessage(Diagnostic.Kind.ERROR, "no exception raised"); is definitely reached. Sorry I didn't test the sample code more throughly.
(In reply to comment #18) > Oliver you're right. I went over the code with jdk6 again and the the following > line > messenger.printMessage(Diagnostic.Kind.ERROR, "no exception raised"); > is definitely reached. Sorry I didn't test the sample code more throughly. Ok, thanks for confirming. This means that the fix is right (we are fixing a bug), but we might end up being out-of-sync with the way javac behaves. In this case, I think it is perfectly valid.
