On Thu, 13 Feb 2025 18:25:38 GMT, Sergey Bylokhov <s...@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
