Bugzilla – Full Text Bug Listing
| Summary: | [options] Improve "Unnecessary declaration of thrown checked exceptions" | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||||
| Component: | Core | Assignee: | Maxime Daniel <maxime_daniel> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | jerome_lanneluc, kent_johnson, mlists, philippe_mulet, sja.eclipse | ||||||
| Version: | 3.0 | Flags: | kent_johnson:
review+
|
||||||
| Target Milestone: | 3.4 M2 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 157155, 202451 | ||||||||
| Attachments: |
|
||||||||
|
Description
Dani Megert
Deferring post 3.1 Can this be revisited for 3.3? As discussed at JDT/Summit There are several bugs currently opened against this functionality, I will add all of them as prerequesite of this bug and remove them from plan to have only one input about this improvement... forgot previous comment, wrong bug...:-( removing target *** Bug 181357 has been marked as a duplicate of this bug. *** Is this likely to make it into 3.3? I'm mostly interested in the scenario I describe in bug 181357, which has been marked as a duplicate of this bug. I would now favor a solution similar to our unused parameter story. If an unused exception is documented (@throws), then we stop complaining. We can do it the same way but it will suffer the exact same limitations: 1. Eclipse can auto-generate the Javadoc skeleton hence disables the warning as the method is generated (e.g. by using code assist or a quick fix/assist). 2. the compiler's power is not fully exploited i.e. it could help me more: it would be very helpful to let the compiler check the hierarchy (workspace scope would be fine for me). This would help in many situations, e.g. when writing new (API) code where you still know all your clients and hence can avoid unnecessary throws declarations. For 3.4, we want to support the documenting approach. Building a type hierarchy on the fly for any of these is just going to make compiler 100x slower. This is rather a separate tool issue. Most generated comments in overriding methods usually simply refer to top method doc, so are not going to be hiding these warnings. Mostly the top method is going to allow to silence the offending warning when defining its API contract. Created attachment 77801 [details]
Suggested fix + test cases
This proposal imitates what @param does for parameters, which results into the following:
- it introduces a new option OPTION_ReportUnusedDeclaredThrownExceptionIncludeDocCommentReference, with its associated reportUnusedDeclaredThrownExceptionIncludeDocCommentReference, COMPILER_PB_UNUSED_DECLARED_THROWN_EXCEPTION_INCLUDE_DOC_COMMENT_REFERENCE and org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownExceptionIncludeDocCommentReference fields and values;
- that option is ENABLED by default (which changes the behavior on some sources, but is aligned with @param, which must have also introduced the same type of behavior change when its matching option appeared);
- as illustrated in test cases, we only consider exact matches (that is, an @throws of a checked exception that is a direct or indirect superclass of a declared checked exception that is not thrown does not clear the warning).
All tests pass. Kent, would you please review it? Filed bug 202451 for the UI part of this feature. Looks straight forward to me. Released for 3.4 M2. Created attachment 77978 [details] fix failing test (In reply to comment #13) > All tests pass. Well, not really: org.eclipse.jdt.ui.tests.quickfix.LocalCorrectionsQuickFixTest.testUnnecessaryThrownException3() does not. Fixed in HEAD. Sorry for that, and thanks for fixing it. I felt that following the design of what we did for @param was better than keeping the default behavior unchanged (and hence a mismatch between @param and @throws). I still believe it is better for consistency sake, but I should have checked with impacted teams first. Verified for 3.4M2 using I20070917-1800 |