RFR: 8347826: Introspector shows wrong method list after 8071693 [v4]

[Roman Marchenko]

On Tue, 11 Feb 2025 17:36:56 GMT, Sergey Bylokhov <serb at 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+azvegint at 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