Bug 73244 - [options] Improve "Unnecessary declaration of thrown checked exceptions"
Summary: [options] Improve "Unnecessary declaration of thrown checked exceptions"
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.4 M2   Edit
Assignee: Maxime Daniel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 181357 (view as bug list)
Depends on:
Blocks: 157155 202451
  Show dependency tree
 
Reported: 2004-09-03 09:43 EDT by Dani Megert CLA
Modified: 2007-09-18 09:33 EDT (History)
5 users (show)

See Also:
kent_johnson: review+


Attachments
Suggested fix + test cases (17.39 KB, patch)
2007-09-06 08:54 EDT, Maxime Daniel CLA
no flags Details | Diff
fix failing test (1.08 KB, patch)
2007-09-10 09:05 EDT, Benno Baumgartner CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2004-09-03 09:43:14 EDT
I20040901

There could be some additional option which allows to ignore the following cases:
- abstract methods
- methods which are overridden (scope: workspace)

Currently we have set this check to ignore because it flagged to many framework
methods.

See also eclipse.platform newsgroup message:
news://news.eclipse.org:119/cgvvab$9tl$1@eclipse.org
Comment 1 Philipe Mulet CLA 2005-04-07 08:01:42 EDT
Deferring post 3.1
Comment 2 Dani Megert CLA 2006-07-12 07:03:44 EDT
Can this be revisited for 3.3?
Comment 3 Frederic Fusier CLA 2006-09-08 12:56:25 EDT
As discussed at JDT/Summit
Comment 4 Frederic Fusier CLA 2006-09-13 07:52:48 EDT
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...
Comment 5 Frederic Fusier CLA 2006-09-13 07:53:59 EDT
forgot previous comment, wrong bug...:-(
Comment 6 Philipe Mulet CLA 2006-12-05 04:32:19 EST
removing target
Comment 7 Maxime Daniel CLA 2007-04-06 01:47:10 EDT
*** Bug 181357 has been marked as a duplicate of this bug. ***
Comment 8 Simon Archer CLA 2007-04-06 10:07:33 EDT
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.
Comment 9 Philipe Mulet CLA 2007-07-10 10:29:02 EDT
I would now favor a solution similar to our unused parameter story.
If an unused exception is documented (@throws), then we stop complaining.
Comment 10 Dani Megert CLA 2007-07-10 11:09:06 EDT
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.
Comment 11 Philipe Mulet CLA 2007-08-31 12:56:23 EDT
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.
Comment 12 Maxime Daniel CLA 2007-09-06 08:54:54 EDT
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).
Comment 13 Maxime Daniel CLA 2007-09-06 09:52:37 EDT
All tests pass.
Kent, would you please review it?
Comment 14 Dani Megert CLA 2007-09-06 10:05:16 EDT
Filed bug 202451 for the UI part of this feature.
Comment 15 Kent Johnson CLA 2007-09-07 12:01:40 EDT
Looks straight forward to me.
Comment 16 Maxime Daniel CLA 2007-09-10 03:09:03 EDT
Released for 3.4 M2.
Comment 17 Benno Baumgartner CLA 2007-09-10 09:05:44 EDT
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.
Comment 18 Maxime Daniel CLA 2007-09-10 09:43:55 EDT
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.
Comment 19 Jerome Lanneluc CLA 2007-09-18 09:33:38 EDT
Verified for 3.4M2 using I20070917-1800