Community
Participate
Working Groups
This is a fup of bug 100278. In bug 100278, we went too far and we are now warning for all unchecked exceptions which is not a useful option. We should change the option back to the original request, i.e. control whether we warn if a throws clause contains Throwable and/or Exception and no exception (checked or explicit unchecked) is actually thrown in the method body.
Created attachment 91483 [details] Fix - Renames option and changes semantics, aligns tests
Kent, would you please let me know what you think?
Released for 3.4 M6.
In fact, JDT UI already surfaced the COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_UNCHECKED_EXCEPTIONS option. Talked to Martin, and I'll reintroduce it as deprecated, without any bearing on the compiler's behavior, for one week or so. I'll open a separate bug on JDT UI to take care of the introduction of the new option in replacement of COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_UNCHECKED_EXCEPTIONS. Must still complete the option change in M6 cycle.
Temporarily reintroduced COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_UNCHECKED_EXCEPTIONS as deprecated, with no effect on compiler behavior. Opened bug 221467 against UI for the needed changes there. Will remove COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_UNCHECKED_EXCEPTIONS as soon as possible.
Can you clarify the spec in JavaCore: That's what's currently there: * <p>When enabled, the compiler will report neither * {@link java.lang.Throwable} nor {@link java.lang.Exception} as unused * declared thrown exceptions. When disabled, all declared thrown checked * exceptions will be considered. I don't understand what the second sentence means. I thought, the normal behaviour is to never consider unchecked exceptions as usused. With the new option, 'Throwable' and 'Exception' are looked at and a.) always considered unused or b.) only when there's no code that could possibly throw an exception (like an empty body) or c.) hen there's no code that explicitly throws a either checked or unchecked exception.
The fact is, Throwable and Exception are checked exceptions. The former (pre-bug 100278) behavior of the compiler exempted them because, these are the two only checked exceptions that may catch unchecked exceptions (which means that reporting that they are declared but never - explicitly - thrown somewhat overlooks the fact that they may in fact stand here for unchecked exceptions that the code may throw without showing it). What we do now (with this bug) is that we allow the user to decide that he prefers not to exempt them from the diagnostic (by moving the new COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_EXEMPT_EXCEPTION_AND_THROWABLE option from default - enabled - to disabled). The second sentence hence says that when the option is disabled, all declared thrown checked exceptions will be considered.
Thanks for the good explanation: Can you improve the second sentence? It would be helpful to have it describe what happens to 'Exception' and 'Throwable' when the option is disabled. 'When the option is disabled, 'Exception' and 'Throwable' will be marked unused when... '
Suggestion would be: amend the doc for COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION into: * <p>When enabled, the compiler will issue an error or a warning when a * method or a constructor is declaring a checked exception as thrown, * but never actually raises it in its body. (the following lines explain that other options further tune this) amend the doc for COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_EXEMPT_EXCEPTION_AND_THROWABLE into: * <p>When enabled, the compiler will issue an error or a warning when a * method or a constructor is declaring a checked exception as thrown, * but never actually raises it in its body, except if the said * exception is {@link java.lang.Throwable} or {@link * java.lang.Exception}. When disabled, the compiler will issue an * error or a warning when a method or a constructor is declaring a * checked exception as thrown, but never actually raises it in its * body, including if the said exception is {@link java.lang.Throwable} or {@link * java.lang.Exception} (the following line says that the error level is set by COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION, which is our standard way of documenting dependent options) Please let me know what you think. I'll amend the doc when I'll get rid of the deprecated option.
I think the addition to 'COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION' is good. For COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION, just improve the second sentence: I'm asking because I'm wondering how the compiler treats potentially thrown unchecked exceptions. Is it: a.) 'When the option is disabled, 'Exception' and 'Throwable' will be marked unused when no statement in the body explicitly declares to throw a checked or unchecked exception. Note that unchecked exception are not required to be declared explicitly. However, the compiler only analyzes explicitly declared unchecked exceptions.' or b.) 'When the option is disabled, 'Exception' and 'Throwable' will be marked unused when no statement in the body specifies to throw a checked exception' or c.) 'When the option is disabled, 'Exception' and 'Throwable' will be marked unused when its guaranteed that no statement in the body can throw a checked exception or unchecked exception'
Would be a) semantics (except that the wording you propose is much focused on message sends, whereas throw statements will be considered as well, and catch clauses absorb some exceptions). Will think more about it and come up with a better proposition then, since the question you ask bears on all exceptions (what happens when an exception that extends another one is thrown?) and probes for a better doc for COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION as well.
Feature wise, is it important that we have a.)? Or wouldn't be b.) simpler to explain and also more useful?
In fact, this is not quite a). The general rule is throwing an exception that extends the one that is declared clears the warning out. It just happens that, for Throwable and Exception, there exists runtime exception that apply in these circumstances. If we want to keep the design consistent, then I believe we should not change the behavior. I've released an improved (or at least, I hope so) documentation for the options bunch, the one for COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_EXEMPT_EXCEPTION_AND_THROWABLE now being: /** * Compiler option ID: Reporting Unused Declared Thrown Exception Exempts Exception And Throwable. * <p>When enabled, the compiler will issue an error or a warning when a * method or a constructor is declaring a checked exception else than * {@link java.lang.Throwable} or {@link java.lang.Exception} as thrown, * but its body actually raises neither that exception, nor any other * exception extending it. When disabled, the compiler will issue an * error or a warning when a method or a constructor is declaring a * checked exception (including {@link java.lang.Throwable} and * {@link java.lang.Exception}) as thrown, but its body actually raises * neither that exception, nor any other exception extending it. * <p>The severity of the unused declared thrown exception problem is * controlled with option * {@link #COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION}. * <p>This diagnostic is further tuned by options * {@link #COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_DOC_COMMENT_REFERENCE} * and {@link #COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_WHEN_OVERRIDING}. * <dl> * <dt>Option id:</dt><dd><code>"org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownExceptionExemptExceptionAndThrowable"</code></dd> * <dt>Possible values:</dt><dd><code>{ "enabled", "disabled" }</code></dd> * <dt>Default:</dt><dd><code>"enabled"</code></dd> * </dl> * @since 3.4 * @category CompilerOptionID */ Please let me know when I can delete the deprecated constant.
The spec is much better now, thanks. The complexity is hidden in the work raise. I wouldn't mind if it would at least hint at a difference between runtime and checked exceptions. Something like: 'Note that the analysis treats runtime exception like checked exceptions, and only explicitly thrown or declared runtime exception are used for the analysis'. We must not forget to add this to the help as well, maybe even with some examples. The one sentence description on the preference page can not capture that. You can remove the old constant now,
You're welcome. I think we are focusing just a bit too much on the unchecked/checked controversy now because we've been through it recently. It is difficult to strike the right balance between clarity and conciseness. What I suggest is the following: when we upgrade the help, we attempt to provide more verbose explanations and include the unchecked/checked difference discussion there. Then we could reconsider whether or not the javadoc needs further improvement. I'll remove the deprecated constant right away.
Removed COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_UNCHECKED_EXCEPTIONS. Released for 3.4 M6 (again).
Verified for 3.4M6 using build I20080324-1300
