On Thu, 4 Feb 2021 13:18:09 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

> This change improves support for the `@hidden` tag to suppress documentation 
> for specific elements, especially in the context of classes, interfaces and 
> inheritance/method overriding.
> 
> The important changes are in `Utils` and in `VisibleMemberTable`. (There is 
> also a number of non-related small code cleanup changes, sorry about that, I 
> know it makes review a bit harder but I couldn't resist.)
> 
> In `Utils` the most important change are:
> 
>  - Consider types as "undocumented enclosures" if they are marked with a 
> `@hidden` tag
>  - Check for `@hidden` tags even in non-included elements as they may be 
> included via undocumented enclosures
>  - To counter the above change, only run doclint on elements that are either 
> included or contained in an included type, as we don't want to report 
> warnings or errors for non-included code. 
>  
> In `VisibleMemberTable` there is a subtle change to not consider an 
> overriding method as a "simple override" if the overridden method is hidden 
> or otherwise invisible but in turn is a non-simple override of a method 
> further up the inheritance chain. This resulted in methods which should have 
> been documented as declared locally to be documented as declared in 
> supertypes. I also did a bit of renaming in `VisibleMemberTable` to make the 
> purpose of things a bit clearer.
> 
> Other than that, most of the changes consist of added calls to 
> `utils.hasHiddenTag(Element)`, usually with the purpose of not generating 
> links to things that are not there.

Two primary areas:

1. I think the changes to Utils to determine if a tag is hidden can be 
improved/simplified
2. I'm trying to understand the changes to `VisibleMemberTable`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
 line 2803:

> 2801:             return true;
> 2802:         }
> 2803:         // Run doclint on non-incuded members of included type elements.

typo `incuded`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
 line 2731:

> 2729:                 if (shouldRunDocLint(element)) {
> 2730:                     configuration.runDocLint(path);
> 2731:                 }

I'm disappointed there is so much disruption to the code because of the 
enhanced treatment of `@hidden`.Maybe it's necessary, but I was 
hoping/expecting that we could add a separate code path for the specific 
purpose of seeing if a non-selected(?) class has a `@hidden` tag. 

For example, leave all the code for block tags and doc comment tree alone 
unchanged, and then provide `isHidden(Element)`  by just using 
`getDocCommentInfo`, which gives cached access to the `DocCommentTree` then 
scan the block tags for a `HIDDEN` tree.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java
 line 261:

> 259:         OverridingMethodInfo found = overriddenMethodTable.get(e);
> 260:         if (found != null && found.simpleOverride) {
> 261:             return found.overridden;

changing from `overrider` to `overridden` seems like a pretty big change, and 
not just a terminology cleanup.

Is this related to the change you described in the introduction?

-------------

Changes requested by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2406

Reply via email to