On Wed, 15 Mar 2023 15:48:37 GMT, Pavel Rappo <[email protected]> wrote:

> Please review a change to clean up and simplify LocalMemberTable; a container 
> to cache, classify, and provide efficient lookup for the return value of 
> `TypeElement.getEnclosedElements()`.
> 
> While the change primarily targets internals of LocalMemberTable, it also 
> affects its clients: in particular, code that handles JavaFX documentation. 
> That code does not seem to be tested well (I filed a bug for that: 
> JDK-8304170). To make sure I haven't broken anything, aside from usual 
> testing, I also cloned [OpenJFX](https://github.com/openjdk/jfx) and built 
> its documentation with javadoc before and after the change. Documentation 
> bundles were identical.

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

> 901:         }
> 902: 
> 903:         List<Element> getMembers(Name simplename, Kind kind) {

Suggest: `simpleName`

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

> 905:                 @Override
> 906:                 public String visitExecutable(ExecutableElement e, Void 
> aVoid) {
> 907:                     return e.getSimpleName() + ":" + 
> e.getParameters().size();

That was bizarre.

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

> 905:         }
> 906: 
> 907:         <T extends Element> List<T> getMembers(Name simplename, Kind 
> kind, Class<T> clazz) {

suggest: `simpleName`

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

> 912:         }
> 913: 
> 914:         List<ExecutableElement> getPropertyMethods(Name simplename) {

suggest `simpleName`

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

> 980:             VariableElement field = flist.isEmpty() ? null : 
> flist.get(0);
> 981: 
> 982:             // TODO: this code does not seem to be covered by tests well

Suggest: link to this TODO in recent new JBS entry.

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

> 1002: 
> 1003:             var setter = 
> lmt.getPropertyMethods(utils.elementUtils.getName(pUtils.getSetName(propertyMethod))).stream()
> 1004:                     // TODO: number of parameters a setter take is not 
> tested

Suggest: link to this TODO in recent new JBS entry.

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

PR: https://git.openjdk.org/jdk/pull/13044

Reply via email to