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

[Sergey Bylokhov]

On Thu, 13 Feb 2025 18:25:38 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>>> It was implemented in a way to minimize the difference between different 'stopClasses' for the same object. In the example above, the next call will produce the same properties:
>>> ...
>>> Thus, the methods of the current class have some priority over those of the parent class.
>>> But if the same class has multiple setFoo(xxx) methods, the behavior will be undefined/unspecified.
>> 
>> Currently, I see from `PropertyInfo` implementation that methods are just sorted by argument's type name if arguments are not assignable from each other. So, in case `Long` vs `Number`, `Long` will be chosen (`isAssignable()` check works); in case `Float` vs `Integer`, `Float` will be chosen (sorted by type name).
>> 
>> I'm still wondering if there's  any specification (tests?) describing this. At the time it looks like the current implementation is the only doc.
>> 
>> If no docs, could you review the test cases below, please? Is it correct, or redundant, or incomplete? I will add them to the test when OK. 
>> 
>> 
>> Case 1:
>>     public interface A {
>>         default public void setParentFoo(Integer num) {
>>         }
>>         default public void setFoo(Integer num) {
>>         }
>>     }
>> 
>>     public class D implements A {
>>         public void setFoo(Number num) {
>>         }
>>         public void setLocalFoo(Long num) {
>>         }
>>         public void setLocalFoo(Float num) {
>>         }
>>     }
>> 
>> Expecting:
>> 
>> --- properties
>> public void Test$D.setFoo(java.lang.Number)
>> public void Test$D.setLocalFoo(java.lang.Float)
>> public default void Test$A.setParentFoo(java.lang.Integer)
>> --- methods
>> public void Test$D.setFoo(java.lang.Number)
>> public void Test$D.setLocalFoo(java.lang.Float)
>> public default void Test$A.setFoo(java.lang.Integer)
>> public void Test$D.setLocalFoo(java.lang.Long)
>> public default void Test$A.setParentFoo(java.lang.Integer)
>> 
>> 
>> Case 2:
>>     public class AC {
>>         public void setParentFoo(Integer num) {
>>         }
>>         public void setFoo(Integer num) {
>>         }
>>     }
>> 
>>     public class DC extends AC {
>>         public void setFoo(Number num) {
>>         }
>>         public void setLocalFoo(Long num) {
>>         }
>>         public void setLocalFoo(Float num) {
>>         }
>>     }
>> 
>> Expecting:
>> 
>> --- properties
>> public void Test$DC.setFoo(java.lang.Number)
>> public void Test$DC.setLocalFoo(java.lang.Float)
>> public void Test$AC.setParentFoo(java.lang.Integer)
>> --- methods
>> public void Test$DC.setFoo(java.lang.N...
>
>> > It was implemented in a way to minimize the difference between different 'stopClasses' for the same object. In the example above, the next call will produce the same properties:
>> > ...
>> > Thus, the methods of the current class have some priority over those of the parent class.
>> > But if the same class has multiple setFoo(xxx) methods, the behavior will be undefined/unspecified.
>> 
>> Currently, I see from `PropertyInfo` implementation that methods are just sorted by argument's type name if arguments are not assignable from each other. So, in case `Long` vs `Number`, `Long` will be chosen (`isAssignable()` check works); in case `Float` vs `Integer`, `Float` will be chosen (sorted by type name).
> 
> There is "test/jdk/java/beans/Introspector/TestMethodOrderDependence.java" which was added to prevent accidental change in the implementation, but part of the behavior is undefined/unspecified.
> 
>>If no docs, could you review the test cases below, please? Is it correct, or redundant, or incomplete? I will add them to the test when OK.
> 
> New tests are always welcome, especially for interfaces, as they are a relatively new feature.

> @mrserb If I reverse sorting order, or remove sorting at all, 2 tests are failed - OverloadedSetter and TestMethodOrderDependence. So it doesn't look an utility wrapper, as 2 tests rely on the sorting order. Or am I mistaken?

I meant to delete the next code you added in the [MethodInfo.java](https://github.com/openjdk/jdk/pull/23443/files#diff-2b692928e718b6c457a50aac719ae8b1ace8a2cb2426428fca14c67035e78080):

            if (a.isDefault() != b.isDefault()) {
                return a.isDefault() ? -1 : 1; // default methods go first
            }

and update the logic for read/write in the [PropertyInfo.java#initialize](https://github.com/openjdk/jdk/pull/23443/files#diff-ef0d6cd4a6177670eb9196da7ac4d67d1eddef222aadbfbf49075e74395fe9bbL73) to get the same result.

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

PR Comment: https://git.openjdk.org/jdk/pull/23443#issuecomment-2686762227