RFR: 8304146: Refactor VisibleMemberTable (LocalMemberTable)

Jonathan Gibbons jjg at openjdk.org
Wed Mar 15 17:59:27 UTC 2023


On Wed, 15 Mar 2023 15:48:37 GMT, Pavel Rappo <prappo at openjdk.org> 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


More information about the javadoc-dev mailing list