Bug 246236 - [javadoc] False positive in missing Javadoc tags problem detection
Summary: [javadoc] False positive in missing Javadoc tags problem detection
Status: CLOSED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT Core Triaged CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-04 11:36 EDT by Dani Megert CLA
Modified: 2020-01-30 14:54 EST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2008-09-04 11:36:20 EDT
3.4 and I20080903-1200.

If the warning for missing Javadoc tags in overriding and implementing methods is enabled then we get false positives for inherited members. Since most tags comments are automatically inherited (see [1] and [2]) no warning must be issued for those tags that have inherited comment.

The point for the additional option was to save performance, now when I check this I accept performance impact in favor of more accurate problem reporting.

[1] http://java.sun.com/j2se/1.4.2/docs/tooldocs/windows/javadoc.html#inheritingcomments

[2] http://java.sun.com/javase/6/docs/technotes/tools/windows/javadoc.html#inheritingcomments
Comment 1 Markus Keller CLA 2008-09-11 12:38:32 EDT
Documenting the overriding method with {@inheritDoc} currently makes all "Javadoc: Missing tag for..." problems go away. This "catch-all" code should be removed when this bug is fixed (since {@inheritDoc} in the main description does not affect auto inheritance of tags at all).

For @param and @return tags, you could also consider an easy fix that just avoids the "missing tags" problem in overriding methods, without checking whether one of the overridden methods already has documentation for that tag (if the tag cannot be found, then the topmost methods in the inheritance tree must already have a problem).

However, for @exception and @throws, you have to walk the inheritance tree, since unchecked exceptions (like IAE and ISE below) may or may not be documented in one of the super methods.


public class Try {
    /**
     * @param a the a
     * @return 0
     * @since 1.0
     * @throws IOException io
     * @throws IllegalArgumentException iae
     */
    public int m(int a) throws IOException {
        return 0;
    }
}

class Sub extends Try {
    /**
     */
    public int m(int a) throws IOException, IllegalArgumentException,
            IllegalStateException {
        return super.m(a);
    }
}
Comment 2 Frederic Fusier CLA 2008-09-12 04:11:06 EDT
(In reply to comment #0)
The only way for the compiler to avoid these false positive missing tag warning would be to open all the files of the superclasses until it was sure that one of the overridden defines the tag or not. First, I do not know if it would be feasible and second, the extra cost would be totally unacceptable...

There's currently a way to disable this warning in overriding and implementing methods and then avoid these false positive warning. I cannot see what we can do better than that here...

(In reply to comment #1)
I'm not sure to understand how the easy fix works for @param and @return tag...
Why if the tag is not present does it mean that the topmost methods in the inheritance tree already have a problem? Which kind of problem? Can you be a little bit more precise?
Comment 3 Dani Megert CLA 2008-09-12 04:21:01 EDT
>There's currently a way to disable this warning in overriding and implementing
>methods and then avoid these false positive warning. I cannot see what we can
>do better than that here...
It is the other way around: the warning is off by default (i.e. Ignore...). When I go and enable it (as I did) I chose better (more accurate) reporting and hence also expect (can live with) the performance impact. I guess that's why we added that switch in the first place.
Comment 4 Frederic Fusier CLA 2008-09-12 04:47:23 EDT
(In reply to comment #3)
> It is the other way around: the warning is off by default (i.e. Ignore...).
> When I go and enable it (as I did) I chose better (more accurate) reporting and
> hence also expect (can live with) the performance impact.
>
My concern on the performance impact is that it should be over the acceptable range. Would you really accept that compilation lasts twice or three times longer only to be sure that you have no false positive for missing tag warnings?
If your answer is yes, then I still strongly doubt that all users would have the same feeling...

> I guess that's why we added that switch in the first place.
> 
Not sure. My recall is that we added it because there was no other possibility when users do not want to modify their comments. Currently users who want to have missing tag warning without be polluted by overriding method can either:
1) ignore them by setting this preference
2) add a {@inheritDoc} tag
3) add a @see reference to the overridden method.

I still think we offer enough other way to avoid these false positive and I'm definitely not sure that trying to compile all the class hierarchy would give a real benefit for users...

Philippe, what's your mind on this?
Comment 5 Dani Megert CLA 2008-09-12 05:03:27 EDT
>Would you really accept that compilation lasts twice or three times
>longer only to be sure that you have no false positive for missing tag
>warnings?
>If your answer is yes, then I still strongly doubt that all users would have
>the same feeling...
The answer would be 'no'. If it is really that expensive we shouldn't do it.
Comment 6 Markus Keller CLA 2008-09-12 05:09:21 EDT
(In reply to comment #2)
> I'm not sure to understand how the easy fix works for @param and @return tag...
> Why if the tag is not present does it mean that the topmost methods in the
> inheritance tree already have a problem? Which kind of problem? Can you be a
> little bit more precise?

See the example in comment 1. When you remove the "@param a ..."  from
Try#m(int), then parameter doc for "a" is missing for both Try#m(int) and
Sub#m(int).

My point was that it would be acceptable to not raise a problem for Sub#m(int),
because if the generated doc for the overriding method Sub#m(int) does not have
doc for "a", then none of the super implementations (including the the topmost
method Try#m(int) in the example) has "@param a ..." in their Javadoc. So, when
you compile the topmost method with the same compiler options, then you will
always get a "Missing tag" problem there. It's enough to just check the topmost
methods to ensure that all parameters are documented when the system is
warning-free. Of course, this only works when you have everything in source,
but I think that's a fair assumption.
Comment 7 Markus Keller CLA 2008-09-12 05:20:24 EDT
(In reply to comment #4)
> Would you really accept that compilation lasts twice or three times longer

No, I also wouldn't accept that.

(In reply to comment #2)
> The only way for the compiler to avoid these false positive missing tag warning
> would be to open all the files of the superclasses until it was sure that one
> of the overridden defines the tag or not.

I don't think that's "the only way". IIRC, you already have a bit on a method binding that indicates whether a method overrides or implements another method. In the same pass where you determine the state of this bit, you could also remember each method's available (declared or inherited) tags (bits for @return and for each parameter, resolved types for documented exceptions). Then the lookup would not take that long any more (but use some memory, which is acceptable IMO).
Comment 8 Eclipse Genie CLA 2020-01-30 14:54:40 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.