Community
Participate
Working Groups
Consider the following two methods public void test() throws FileNotFoundException, IOException { throwsIOException(); } public void anotherTest() throws Exception { } There is no "unnecessary declaration of thrown exception" warnings as of Eclipse 3.1RC2 for these two methods. I can kinda understand rationale why throws FileNotFoundException is not considered redundant but throws Exception does not make any sense to me and makes this feature pretty much useless.
There is no warning for Exception because this type can potentially catch an unchecked exception. Same treatment for Throwable. I hope that this clarifies the way this warning works. Did you notice any change in the way we handle this warning between RC1 and RC2?
Thank you for the quick reply, Olivier, but I think you missed my point. The only reason a developer would care about unnecessary "throws" declarations is because such exceptions have to be dealt with by every caller of the method. The callers' code will either become polluted with unnecessary catch blocks, or it will further propagate declaration of these unnecessary exceptions. In either case, the codebase grows with the code which not only redundant but is also a likely source of programming errors. Throws Exception/Throwable is especially bad since it forces the callers to (implicitly) catch RuntimeException/Error. In other words, the goal of this warning is to help limit number of exception types that *have* to be caught by the callers, not number of exception types that *can* possibly be caught by the callers. Hope this makes sense...
Philippe, any comment?
The debate is legite, and I could agree with it. I think all we need to know at this stage (3.1RC3) is whether RC2 changed behavior for you. If not, then I would defer any change in this area. Not that it isn't a valid concern, but not critical at this stage.
Indeed, the behaviour did not change. That only makes me wonder how I missed this before. And I agree this should wait until after 3.1.
We could consider a suboption to not consider unchecked exception as being thrown by default. This option would be off by default (keep current behavior by default).
removing target
*** Bug 132420 has been marked as a duplicate of this bug. ***
Changing the title to reflect the discussion. May have to consider both unchecked exceptions and exceptions that unchecked exceptions extend (with separate messages, but same tuning devices). Also need to discuss whether we warn upon unchecked exceptions that are explicitly declared, or only upon unchecked exceptions that are explicitly declared *and not thrown* (which was the point of bug 132420).
I *often* declare throws clauses for exceptions not thrown by an API method when I anticipate a subclass may need to throw such an exception, or when I anticipate some future version of the code may need to throw such an exception (ie, I may hard code a method return value today which I eventually may want to read from a database [possibly causing an exception]). If I specifically declare such an exception, I think the default behavior should be for the compiler to assume I know what I am doing. What I was asking for in the bug which was (I think incorrectly) marked as a dup of this, is that if I specifically declare a method as throwing an *unchecked* exception, that callers not specifically catching/handling that runtime exception should issue a warning.
I see. Reopening bug 132420, with a slightly different title. Please check it matches your thoughts.
Created attachment 86913 [details] Fix + test cases Adds an option to include all exceptions in the diagnostic, disabled by default.
Kent, would you please let me know what you think?
Released for 3.4 M5.
Opened bug 216233 for UI consideration of the new option.
I have to reopen as this does not work: 1. the JavaCore constant value has two typos and hence doesn't match the option (string) defined in the internal CompilerOptions 2. test case seems to be broken or not test the public API as used by clients 3. not sure about this one but it seems that the option is not initialized for batch compiler(org.eclipse.jdt.internal.compiler.batch.Main.configure(String[])
Created attachment 87631 [details] Proposed fix (without additional tests)
Created attachment 87638 [details] Proposed fix (without additional tests)
Created attachment 87639 [details] Proposed fix (without additional tests) Sorry about the previous patches. I got tricked by the wizard.
Thanks for the patch. The change to Main is not strictly needed, and given the fact that the code comments what it is doing to some length: // override defaults: references that are embedded in javadoc are ignored // from the perspective of parameters and thrown exceptions usage I won't adopt it, but the other part is more than welcome. I released it along with a test variant that uses the API constant. I'll open a separate bug to suggest that we factorize constants or else test that they are consistent (I think I can see why we do not factorize them... but the mistake I made calls for some safeguards at least). Released (again) for 3.4 M5.
Verified for 3.4M5 using build I20080204-0010
