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

[Archie Cobbs]

On Tue, 25 Apr 2023 18:31:01 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> 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.
>
> 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.

Thanks - fixed in b92726a923b.

> 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.

Thanks - fixed in b92726a923b.

> 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  ")));

Thanks - fixed in b92726a923b.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176901780
PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176901938
PR Review Comment: https://git.openjdk.org/jdk/pull/13544#discussion_r1176901865