On Tue, 11 Feb 2025 17:36:56 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Roman Marchenko has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java
>>
>> Co-authored-by: Aleksandr Zvegintsev
>> <77687766+azveg...@users.noreply.github.com>
>
>> > Or, most likely, you can skip adding the default method in
>> > MethodInfo.get() if non-default methods are already added.
>>
>> As I commented in JDK-8347826, I think the issue surely can be fixed in
>> different ways. Currently, all methods are put into a list, are sorted, and
>> then methods with the same signature are filtered out in
>> `Introspector.addMethod()` basing on the
>
>> From my point of view, the method list wasn't actually stable (after
>> JDK-8071693), because it didn't take into account if a method is default or
>> not, so `default` is located in the aorted list depending on a name of a
>> returned type, what causes a wrong method list processing in
>> `Introspector.addMethod()`, and exactly what I'm trying to fix here.
>
> But the current sorting is good enouth to reproduced the bugs similar to the
> current one.
>
>> > Is the next output expected()?
>> > `public default void Test$A.setFoo(java.lang.Integer)`
>>
>> It looks OK, it was properly chosen by `PropertyInfo` logic, or am I wrong?
>> If it's OK, I will add it to the test cases.
>
> Then why the property is based on the implemented interface? For classes the
> method from the current class will be selected.
> `public default void Test$A.setFoo(java.lang.Integer)` vs `public void
> Test$DC.setFoo(java.lang.Number)`
>
>> As far as I see, `Parent3` actually has 2 methods falling into
>> `com.sun.beans.introspect.MethodInfo.get()`:
>>
>> * `java.beans.MethodDescriptor[name=getValue; method=public abstract
>> java.lang.Runnable Parent3.getValueX()] isSynthetic()=false`
>>
>> * `java.beans.MethodDescriptor[name=getValue; method=public default
>> java.lang.Object Parent3.getValueX()] isSynthetic()=true`
>>
>>
>> While processing the `Parent3` class (in case we call
>> `Introspector.getBeanInfo(Parent3.class)`), the default synthetic method is
>> filtered out in `java.beans.Introspector.addMethod()` as synthetic. While
>> processing the derived `Child`, it takes the default method regardless if
>> it's synthetic or not, as JDK-8071693 implemented. However I'm not sure if
>> the synthetic `default java.lang.Object Parent3.getValueX()` method should
>> exist at all in the list. I think this issue is out of scope of this PR, and
>> doesn't relate to the proposed fix, so I'd suggest to process it separately.
>
>
> But it is not filtered out for Child class:
>
> import java.beans.*;
> import java.lang.reflect.Method;
> import java.util.Arrays;
>
> interface Parent1<T> {
> T getValue();
> }
>
> interface Parent2 {
> Runnable getValue();
> }
>
> interface Parent3 extends Parent2, Par...
@mrserb Sorry, I updated my last comment above, so please take a look again,
if you already read it.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23443#issuecomment-2652943753
