Community
Participate
Working Groups
I20071127-0800 -enableJavadoc, documented as 'consider references in javadoc' (implicitly, for the sake of any check that involves references), inhibits following uses of -warn:-javadoc and -warn:-allJavadoc. Released (inactive) BatchCompilerTest#206 and 207 in HEAD to illustrate the issue.
Exploring the bug further, I realized that javadoc and allJavadoc both implied enableJavadoc. This is not consistent with the user interface, which allows to separate the two concerns (that is, the check for unused parameters selectively considers @param as a reference or not). The user interface maintains one dependency: if the analysis of the javadoc is totally disabled, then @param won't influence the diagnostic at all. But the reverse relationship does not hold true. Released additional test cases #208 to 213, of which 212 and 213 are inactive, to improve our coverage. Changing the title of the bug to better reflect the diversity of situations.
Created attachment 84180 [details] Fix + test cases
Kent, pls let me know what you think.
From the patch : @@ -3057,6 +3002,69 @@ if (this.enableJavadocOn) { } else if (this.warnJavadocOn || this.warnAllJavadocOn) { } // I would seperate so its clear that the above case is if {} elseif {} // might want to pull the common options out when if either is enabled if (this.warnJavadocOn) { // most of the options are the same as the ones for All } if (this.warnAllJavadocOn) { // same options as above (almost) }
(In reply to comment #4) Thx for your comments. > From the patch : > > @@ -3057,6 +3002,69 @@ > if (this.enableJavadocOn) { > } else if (this.warnJavadocOn || this.warnAllJavadocOn) { > } > > // I would seperate so its clear that the above case is if {} elseif {} Not sure I would want to do this. Would suggest more verbose comments instead, because the intent of those lines is to act upon OPTION_DocCommentSupport, and possibly on javadoc related default options if javadoc is off (and the defaults happen to be overkill). Will propose a new patch along those lines. > // might want to pull the common options out when if either is enabled I wondered if I wanted to do this as well. The problem is that we need to mix positive and negative settings on the one hand, and that factorizing makes us make an extra test (code would then be more compact but slower). I'll play a bit with it and I'll come back with my toughts in a new patch.
Created attachment 86092 [details] Fix + test cases - wip, do not release as is This proposal better comments the first series of settings and groups the others so as to avoid code duplication. As far as existing test cases are concerned, this patch works just like the previous one, and the resulting behavior seems reasonably aligned with the documented behaviors of -warn:javadoc and -warn:allJavadoc (respectively 'invalid javadoc' and 'invalid or missing javadoc'). Interestingly enough, tough, it must be markedly different from the previous patch on other test cases (because I commented out the settings that were previously done for javadoc only). At the very least, this tells us that our test coverage is perfectible. Before making any decision, I'd appreciate the others' opinion on whether these specific settings are really needed or not (especially Kent and Olivier, according to the CVS annotations). I'll complete the test cases to match the selected behavior.
Created attachment 86291 [details] Better fix + test cases Added test cases that stick to the current definition for allJavadoc and javadoc (that is, allJavadoc implies all javadoc, and javadoc is allJavadoc minus missing javadocs). Their passing needs that the options be tweaked a bit. Discovered that (probably because of changes in default options) we didn't have it right for allJavadoc anyway. Also figured out that redundant superinterfaces were never reported in batch mode. Did not change this. Changing the corresponding option to DISABLED was not correct anyway, since it expects one of "error", "warning", "ignore", defaulting to ignore. The substractive behavior is not expected to be fixed by this patch, because it is the object of bug 210521, that I'll tackle once this bug is fixed. Kent, would you please let me know what you think?
Released for 3.4 M5.
Verified for 3.4M5 using build I20080204-0010
