RFR: 8071693: Introspector ignores default interface methods [v7]

[Alexey Ivanov]

On Mon, 24 Apr 2023 19:25:59 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> The `Introspector` class was never updated to include `default` methods inherited from interfaces.
>> 
>> This patch attempts to fix that omission.
>
> Archie Cobbs has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Jam lines into 80 columns.
>  - Add more scenarios to the regression test.

Marked as reviewed by aivanov (Reviewer).

test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java line 184:

> 182:           .ifPresent(name -> {
> 183:             throw new Error("property \"" + name + "\" not found in " + type);
> 184:           });

Suggestion:

        expected.stream()
                .map(PropertyDescriptor::getName)
                .filter(name -> BeanUtils.getPropertyDescriptor(type, name) == null)
                .findFirst()
                .ifPresent(name -> {
                    throw new Error("property "" + name + "" not found in " + type);
                });

I don't know if there's a common style that's followed in OpenJDK, however, aligning wrapped method calls in streams on the dot is the most common way. I prefer this style.

Indenting wrapped lines by two spaces doesn't look standard to me, the most common indentation for wrapped lines seems to be 8 spaces.

test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java line 188:

> 186:         // Gather actual properties
> 187:         final Set<PropertyDescriptor> actual = Set.of(
> 188:             BeanUtils.getPropertyDescriptors(type));

Suggestion:

        final Set<PropertyDescriptor> actual =
                Set.of(BeanUtils.getPropertyDescriptors(type));

May be easier to read? I mean wrapping the entire expression instead of breaking the expression.

test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java line 200:

> 198:               + expected.stream()
> 199:                 .map(Object::toString)
> 200:                 .collect(Collectors.joining("\n  ")));

Suggestion:

              + actual.stream()
                      .map(Object::toString)
                      .collect(Collectors.joining("\n  "))
              + "\nEXPECTED:\n  "
              + expected.stream()
                        .map(Object::toString)
                        .collect(Collectors.joining("\n  ")));

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

PR Review: https://git.openjdk.org/jdk/pull/13544#pullrequestreview-1400494157
PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176886529
PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176890225
PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176888835