Re: JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hi Joe, all,
the logic looks good to me.
In the tests I'm wondering whether to include an annotated wildcard
bound. There is:
307 public @AnnotType(11) Set<@AnnotType(13) ? extends Number>
fooNumberSet2() {return null;}
but nothing like
Set fooNumberSet2() {return null;}
I wouldn't expect problems for this, but a test would exercise some of
the code that gets added.
Best,
cu, WMD.
On Wed, Oct 10, 2018 at 2:40 AM Joel Borggrén-Franck
wrote:
>
> Hej Joe,
>
> New version looks good!
>
> Thanks for the explanations, makes sense to me.
>
> Cheers
> /Joel
>
> On Wed, 10 Oct 2018 at 08:27, joe darcy wrote:
>>
>> Hi Joel,
>>
>> Thanks for the review; slightly revised version at
>>
>> http://cr.openjdk.java.net/~darcy/8058202.3/
>>
>> Comments below.
>>
>>
>> On 10/9/2018 11:00 AM, Joel Borggrén-Franck wrote:
>> > Hi Joe,
>> >
>> > Good to see this happening. In general looks good to me, a few nits below.
>> >
>> > AnnotatedTypeBaseImpl contains comments indicating from which
>> > superclass or interface the overridden methods comes. I'd either add
>> > // Object at line 207 or delete line 145 and 177 for consistency.
>>
>> Okay; comments added to follow that local convention.
>>
>> >
>> > Even though this isn't performance critical by far the allocation at
>> > line 215 still bugs me a bit given that the common case will most
>> > certainly be no annotations.
>>
>> Sure; refactored to avoid the allocation.
>>
>> >
>> > Why the inverted logic line 250-253, IIRC you should be able to test
>> > if it is an AnnotatedBaseTypeImpl, or?
>>
>> I was aiming to avoid testing for the impl class directly to not
>> directly tie the classes to this particular implementation of the
>> AnnotatedType hierarchy. However, inter-operability based on the specs
>> would need the equals and hashCode behavior to be defined.
>>
>> >
>> > For equalsTypeAndAnnotations and baseHashCode lines 232-244 equals
>> > test for owner type equality while hashcode doesn't include owner
>> > type. I must confess I no longer remember the equals-hashCode contract
>> > but I assume this is intentional.
>>
>> Good catch; equals and hashCode checks made consistent.
>>
>> Some refactoring and hardening of the test included too.
>>
>> Delta-diffs below.
>>
>> Thanks,
>>
>> -Joe
>>
>> diff
>> webrev.2/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
>> webrev/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
>>
>> 2c2
>> < * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights
>> reserved.
>> ---
>> > * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights
>> reserved.
>> 207c207
>> < @Override
>> ---
>> > @Override // java.lang.Object
>> 215d214
>> < StringJoiner sj = new StringJoiner(" ");
>> 216a216
>> > StringJoiner sj = new StringJoiner(" ");
>> 228,229c228,231
>> < }
>> < return sj.toString();
>> ---
>> > return sj.toString();
>> > } else {
>> > return "";
>> > }
>> 244c246,247
>> < Objects.hash((Object[])getAnnotations());
>> ---
>> > Objects.hash((Object[])getAnnotations()) ^
>> > Objects.hash(getAnnotatedOwnerType());
>>
>> diff
>> webrev.2/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
>> webrev/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
>>
>> 142d141
>> <
>> 157,168c156,158
>> < AnnotatedType annotType1 = m.getAnnotatedReturnType();
>> < AnnotatedType annotType2 = m.getAnnotatedReturnType();
>> <
>> < boolean valid = annotType1.equals(annotType2);
>> <
>> < if (!valid) {
>> < errors++;
>> < System.err.println(annotType1);
>> < System.err.println(" is not equal to ");
>> < System.err.println(annotType2);
>> < System.err.println();
>> < }
>> ---
>> > checkTypesForEquality(m.getAnnotatedReturnType(),
>> > m.getAnnotatedReturnType(),
>> > true);
>> 171a162,185
>> > private static void checkTypesForEquality(AnnotatedType annotType1,
>> > AnnotatedType annotType2,
>> > boolean expected) {
>> > boolean comparison = annotType1.equals(annotType2);
>> >
>> > if (comparison) {
>> > int hash1 = annotType1.hashCode();
>> > int hash2 = annotType1.hashCode();
>> > if (hash1 != hash2) {
>> > errors++;
>> > System.err.format("Equal AnnotatedTypes with unequal hash
>> codes: %n%s%n%s%n",
>> > annotType1.toString(), annotType2.toString());
>> > }
>> > }
>> >
>> > if (comparison != expected) {
>> > errors++;
>> > System.err.println(annotType1);
>> >
Re: JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hi Werner,
On 10/10/2018 1:23 PM, Werner Dietl wrote:
Hi Joe, all,
the logic looks good to me.
In the tests I'm wondering whether to include an annotated wildcard
bound. There is:
307 public @AnnotType(11) Set<@AnnotType(13) ? extends Number>
fooNumberSet2() {return null;}
but nothing like
Set fooNumberSet2() {return null;}
I wouldn't expect problems for this, but a test would exercise some of
the code that gets added.
You were correct to probe at the bounds on the wildcard components; the
code that was reviewed and pushed does not print annotations on the bounds.
I'll work on an update to handle this and similar cases where there are
embedded types that could have annotations.
Thanks,
-Joe
Best,
cu, WMD.
On Wed, Oct 10, 2018 at 2:40 AM Joel Borggrén-Franck
wrote:
Hej Joe,
New version looks good!
Thanks for the explanations, makes sense to me.
Cheers
/Joel
On Wed, 10 Oct 2018 at 08:27, joe darcy wrote:
Hi Joel,
Thanks for the review; slightly revised version at
http://cr.openjdk.java.net/~darcy/8058202.3/
Comments below.
On 10/9/2018 11:00 AM, Joel Borggrén-Franck wrote:
Hi Joe,
Good to see this happening. In general looks good to me, a few nits below.
AnnotatedTypeBaseImpl contains comments indicating from which
superclass or interface the overridden methods comes. I'd either add
// Object at line 207 or delete line 145 and 177 for consistency.
Okay; comments added to follow that local convention.
Even though this isn't performance critical by far the allocation at
line 215 still bugs me a bit given that the common case will most
certainly be no annotations.
Sure; refactored to avoid the allocation.
Why the inverted logic line 250-253, IIRC you should be able to test
if it is an AnnotatedBaseTypeImpl, or?
I was aiming to avoid testing for the impl class directly to not
directly tie the classes to this particular implementation of the
AnnotatedType hierarchy. However, inter-operability based on the specs
would need the equals and hashCode behavior to be defined.
For equalsTypeAndAnnotations and baseHashCode lines 232-244 equals
test for owner type equality while hashcode doesn't include owner
type. I must confess I no longer remember the equals-hashCode contract
but I assume this is intentional.
Good catch; equals and hashCode checks made consistent.
Some refactoring and hardening of the test included too.
Delta-diffs below.
Thanks,
-Joe
diff
webrev.2/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
webrev/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
2c2
< * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights
reserved.
---
> * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights
reserved.
207c207
< @Override
---
> @Override // java.lang.Object
215d214
< StringJoiner sj = new StringJoiner(" ");
216a216
> StringJoiner sj = new StringJoiner(" ");
228,229c228,231
< }
< return sj.toString();
---
> return sj.toString();
> } else {
> return "";
> }
244c246,247
< Objects.hash((Object[])getAnnotations());
---
> Objects.hash((Object[])getAnnotations()) ^
> Objects.hash(getAnnotatedOwnerType());
diff
webrev.2/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
webrev/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
142d141
<
157,168c156,158
< AnnotatedType annotType1 = m.getAnnotatedReturnType();
< AnnotatedType annotType2 = m.getAnnotatedReturnType();
<
< boolean valid = annotType1.equals(annotType2);
<
< if (!valid) {
< errors++;
< System.err.println(annotType1);
< System.err.println(" is not equal to ");
< System.err.println(annotType2);
< System.err.println();
< }
---
> checkTypesForEquality(m.getAnnotatedReturnType(),
> m.getAnnotatedReturnType(),
> true);
171a162,185
> private static void checkTypesForEquality(AnnotatedType annotType1,
> AnnotatedType annotType2,
> boolean expected) {
> boolean comparison = annotType1.equals(annotType2);
>
> if (comparison) {
> int hash1 = annotType1.hashCode();
> int hash2 = annotType1.hashCode();
> if (hash1 != hash2) {
> errors++;
> System.err.format("Equal AnnotatedTypes with unequal hash
codes: %n%s%n%s%n",
> annotType1.toString(), annotType2.toString());
> }
> }
>
> if (comparison != expected) {
> errors++;
> System.err.println(annotType1);
> System.err.println(expected ? " is not
Re: JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hej Joe,
New version looks good!
Thanks for the explanations, makes sense to me.
Cheers
/Joel
On Wed, 10 Oct 2018 at 08:27, joe darcy wrote:
> Hi Joel,
>
> Thanks for the review; slightly revised version at
>
> http://cr.openjdk.java.net/~darcy/8058202.3/
>
> Comments below.
>
>
> On 10/9/2018 11:00 AM, Joel Borggrén-Franck wrote:
> > Hi Joe,
> >
> > Good to see this happening. In general looks good to me, a few nits
> below.
> >
> > AnnotatedTypeBaseImpl contains comments indicating from which
> > superclass or interface the overridden methods comes. I'd either add
> > // Object at line 207 or delete line 145 and 177 for consistency.
>
> Okay; comments added to follow that local convention.
>
> >
> > Even though this isn't performance critical by far the allocation at
> > line 215 still bugs me a bit given that the common case will most
> > certainly be no annotations.
>
> Sure; refactored to avoid the allocation.
>
> >
> > Why the inverted logic line 250-253, IIRC you should be able to test
> > if it is an AnnotatedBaseTypeImpl, or?
>
> I was aiming to avoid testing for the impl class directly to not
> directly tie the classes to this particular implementation of the
> AnnotatedType hierarchy. However, inter-operability based on the specs
> would need the equals and hashCode behavior to be defined.
>
> >
> > For equalsTypeAndAnnotations and baseHashCode lines 232-244 equals
> > test for owner type equality while hashcode doesn't include owner
> > type. I must confess I no longer remember the equals-hashCode contract
> > but I assume this is intentional.
>
> Good catch; equals and hashCode checks made consistent.
>
> Some refactoring and hardening of the test included too.
>
> Delta-diffs below.
>
> Thanks,
>
> -Joe
>
> diff
> webrev.2/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
>
> webrev/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
>
>
> 2c2
> < * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights
> reserved.
> ---
> > * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights
> reserved.
> 207c207
> < @Override
> ---
> > @Override // java.lang.Object
> 215d214
> < StringJoiner sj = new StringJoiner(" ");
> 216a216
> > StringJoiner sj = new StringJoiner(" ");
> 228,229c228,231
> < }
> < return sj.toString();
> ---
> > return sj.toString();
> > } else {
> > return "";
> > }
> 244c246,247
> < Objects.hash((Object[])getAnnotations());
> ---
> > Objects.hash((Object[])getAnnotations()) ^
> > Objects.hash(getAnnotatedOwnerType());
>
> diff
> webrev.2/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
>
> webrev/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
>
>
> 142d141
> <
> 157,168c156,158
> < AnnotatedType annotType1 = m.getAnnotatedReturnType();
> < AnnotatedType annotType2 = m.getAnnotatedReturnType();
> <
> < boolean valid = annotType1.equals(annotType2);
> <
> < if (!valid) {
> < errors++;
> < System.err.println(annotType1);
> < System.err.println(" is not equal to ");
> < System.err.println(annotType2);
> < System.err.println();
> < }
> ---
> > checkTypesForEquality(m.getAnnotatedReturnType(),
> > m.getAnnotatedReturnType(),
> > true);
> 171a162,185
> > private static void checkTypesForEquality(AnnotatedType annotType1,
> > AnnotatedType annotType2,
> > boolean expected) {
> > boolean comparison = annotType1.equals(annotType2);
> >
> > if (comparison) {
> > int hash1 = annotType1.hashCode();
> > int hash2 = annotType1.hashCode();
> > if (hash1 != hash2) {
> > errors++;
> > System.err.format("Equal AnnotatedTypes with unequal hash
> codes: %n%s%n%s%n",
> > annotType1.toString(), annotType2.toString());
> > }
> > }
> >
> > if (comparison != expected) {
> > errors++;
> > System.err.println(annotType1);
> > System.err.println(expected ? " is not equal to " : " is
> equal to ");
> > System.err.println(annotType2);
> > System.err.println();
> > }
> > }
> >
> 184,196c198,200
> < AnnotatedType annotType1 =
> methods[i].getAnnotatedReturnType();
> < AnnotatedType annotType2 =
> methods[j].getAnnotatedReturnType();
> <
> < boolean valid = !annotType1.equals(annotType2);
> <
> < if (!valid) {
> < errors++;
> < System.err.println(annotType1);
> <
Re: JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hi Joel,
Thanks for the review; slightly revised version at
http://cr.openjdk.java.net/~darcy/8058202.3/
Comments below.
On 10/9/2018 11:00 AM, Joel Borggrén-Franck wrote:
Hi Joe,
Good to see this happening. In general looks good to me, a few nits below.
AnnotatedTypeBaseImpl contains comments indicating from which
superclass or interface the overridden methods comes. I'd either add
// Object at line 207 or delete line 145 and 177 for consistency.
Okay; comments added to follow that local convention.
Even though this isn't performance critical by far the allocation at
line 215 still bugs me a bit given that the common case will most
certainly be no annotations.
Sure; refactored to avoid the allocation.
Why the inverted logic line 250-253, IIRC you should be able to test
if it is an AnnotatedBaseTypeImpl, or?
I was aiming to avoid testing for the impl class directly to not
directly tie the classes to this particular implementation of the
AnnotatedType hierarchy. However, inter-operability based on the specs
would need the equals and hashCode behavior to be defined.
For equalsTypeAndAnnotations and baseHashCode lines 232-244 equals
test for owner type equality while hashcode doesn't include owner
type. I must confess I no longer remember the equals-hashCode contract
but I assume this is intentional.
Good catch; equals and hashCode checks made consistent.
Some refactoring and hardening of the test included too.
Delta-diffs below.
Thanks,
-Joe
diff
webrev.2/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
webrev/raw_files/new/src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java
2c2
< * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights
reserved.
---
> * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights
reserved.
207c207
< @Override
---
> @Override // java.lang.Object
215d214
< StringJoiner sj = new StringJoiner(" ");
216a216
> StringJoiner sj = new StringJoiner(" ");
228,229c228,231
< }
< return sj.toString();
---
> return sj.toString();
> } else {
> return "";
> }
244c246,247
< Objects.hash((Object[])getAnnotations());
---
> Objects.hash((Object[])getAnnotations()) ^
> Objects.hash(getAnnotatedOwnerType());
diff
webrev.2/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
webrev/raw_files/new/test/jdk/java/lang/annotation/typeAnnotations/TestObjectMethods.java
142d141
<
157,168c156,158
< AnnotatedType annotType1 = m.getAnnotatedReturnType();
< AnnotatedType annotType2 = m.getAnnotatedReturnType();
<
< boolean valid = annotType1.equals(annotType2);
<
< if (!valid) {
< errors++;
< System.err.println(annotType1);
< System.err.println(" is not equal to ");
< System.err.println(annotType2);
< System.err.println();
< }
---
> checkTypesForEquality(m.getAnnotatedReturnType(),
> m.getAnnotatedReturnType(),
> true);
171a162,185
> private static void checkTypesForEquality(AnnotatedType annotType1,
> AnnotatedType annotType2,
> boolean expected) {
> boolean comparison = annotType1.equals(annotType2);
>
> if (comparison) {
> int hash1 = annotType1.hashCode();
> int hash2 = annotType1.hashCode();
> if (hash1 != hash2) {
> errors++;
> System.err.format("Equal AnnotatedTypes with unequal hash
codes: %n%s%n%s%n",
> annotType1.toString(), annotType2.toString());
> }
> }
>
> if (comparison != expected) {
> errors++;
> System.err.println(annotType1);
> System.err.println(expected ? " is not equal to " : " is
equal to ");
> System.err.println(annotType2);
> System.err.println();
> }
> }
>
184,196c198,200
< AnnotatedType annotType1 =
methods[i].getAnnotatedReturnType();
< AnnotatedType annotType2 =
methods[j].getAnnotatedReturnType();
<
< boolean valid = !annotType1.equals(annotType2);
<
< if (!valid) {
< errors++;
< System.err.println(annotType1);
< System.err.println(" is equal to ");
< System.err.println(annotType2);
< System.err.println();
< }
<
---
> checkTypesForEquality(methods[i].getAnnotatedReturnType(),
> methods[j].getAnnotatedReturnType(),
> false);
209,210c213
< Method[] methods1 = clazz1.getDeclaredMethods();
< Method[] methods2 =
Re: JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hi Joe,
Good to see this happening. In general looks good to me, a few nits below.
AnnotatedTypeBaseImpl contains comments indicating from which
superclass or interface the overridden methods comes. I'd either add
// Object at line 207 or delete line 145 and 177 for consistency.
Even though this isn't performance critical by far the allocation at
line 215 still bugs me a bit given that the common case will most
certainly be no annotations.
Why the inverted logic line 250-253, IIRC you should be able to test
if it is an AnnotatedBaseTypeImpl, or?
For equalsTypeAndAnnotations and baseHashCode lines 232-244 equals
test for owner type equality while hashcode doesn't include owner
type. I must confess I no longer remember the equals-hashCode contract
but I assume this is intentional.
Cheers
/Joel
On Mon, Oct 8, 2018 at 10:45 PM joe darcy wrote:
>
> Hello,
>
> Please review the changes to address:
>
> JDK-8058202 : AnnotatedType implementations don't override
> toString(), equals(), hashCode()
> http://cr.openjdk.java.net/~darcy/8058202.2/
>
> Some discussion and explanation of the changes:
>
> The java.lang.reflect.AnnotatedType interface hierarchy was added in JDK
> 8 to support type annotations
> (http://openjdk.java.net/projects/type-annotations/) in core reflection.
> It offers a parallel set of interfaces to the java.lang.reflect.Type
> family of interfaces added in JDK 5. The separate AnnotatedType
> interfaces are needed since type annotations are used on type *uses* and
> not type declarations. For example, it is necessary to be able to model
> both of the types of the fields foo and bar in a case like:
>
> @Nonnull String foo;
> @Nullable String bar;
>
> so clearly the same object cannot be used for both as the annotated type
> of the fields since the type annotations differ.
>
> The implementation classes for the AnnotatedType family used in core
> reflection are in a single file AnnotatedTypeFactory.java. There are
> instantiated instances both of the base class AnnotatedTypeBaseImpl and
> its subclasses which implement the specialized subinterfaces of
> AnnotatedType for arrays, type variables, etc.
>
> The implementation classes do *not* declare equals, hashCode, or
> toString methods, which the proposed change rectifies.
>
> For equals and hashCode, the interfaces do *not* define how the equals
> relation should be calculated, an omission shared by the Type
> interfaces. While the default identity-is-equality is correct in some
> sense, it is not very useful as the objects returned by successive "get
> annotated return type" calls on the same method object will not be equal.
>
> The proposed equals implementations check if the argument object
> implements the same AnnotatedType subinterface (and makes sure one of
> the subinterfaces is *not* implemented when determining equality for
> AnnotatedType implementation objects) and compares the results of
> corresponding methods. This is analogous to the equals implementations
> of the Type implementation classes found in the impl classes in the
> sun.reflect.generics.reflectiveObjects package.
>
> The order of annotations is considered significant for equality; this is
> arguably overly strict, but I didn't think it was worthwhile to form
> sets of annotations for the purposes of equality comparison in this case.
>
> For toString to replicate the source ordering for most kinds of types,
> the type annotations, if any, occur textually to the left of the text
> representing the type as in
>
> @Nonnull String
>
> or more interestingly
>
> @Nonnull List
>
> the latter meaning "a nonnull list where the list contains strings."
>
> However, this pattern is not followed for arrays. The annotated type
>
> @Nonnull String[]
>
> means "an array of strings where the strings are nonnull" rather than
> "an array that is nonnull where the array contains strings." The
> rationale for this is explained in JSR 308 related documents such as:
>
> https://checkerframework.org/jsr308/jsr308-faq.html#syntax
> https://checkerframework.org/jsr308/specification/java-annotation-design.html
>
> A multi-dimensional array is represented as an AnnotatedArrayType whose
> component type is an array type of lower dimension, and so on, bottoming
> out with a non-array component type. The existing toString output for
> the Type objects of an array uses VM style notation like
> "[[Ljava.lang.String" for a 2D string array. This is even less helpful
> for annotated type objects and therefore toString output in the style
> usable in source code ("String [] []") will be used instead. Starting
> from an AnnotatedArrayType object for a 2D array of strings, using the
> integer value of type annotations the encounter order for the
> annotations and types is:
>
> @TypeAnno(2) String @TypeAnno(0) [] @TypeAnno(1) []
>
> Therefore, for multi-dimensional arrays, the results for the nested
> dimensions are appended to the in-progress string
JDK 12 RFR of JDK-8058202 : AnnotatedType implementations don't override toString(), equals(), hashCode()
Hello,
Please review the changes to address:
JDK-8058202 : AnnotatedType implementations don't override
toString(), equals(), hashCode()
http://cr.openjdk.java.net/~darcy/8058202.2/
Some discussion and explanation of the changes:
The java.lang.reflect.AnnotatedType interface hierarchy was added in JDK
8 to support type annotations
(http://openjdk.java.net/projects/type-annotations/) in core reflection.
It offers a parallel set of interfaces to the java.lang.reflect.Type
family of interfaces added in JDK 5. The separate AnnotatedType
interfaces are needed since type annotations are used on type *uses* and
not type declarations. For example, it is necessary to be able to model
both of the types of the fields foo and bar in a case like:
@Nonnull String foo;
@Nullable String bar;
so clearly the same object cannot be used for both as the annotated type
of the fields since the type annotations differ.
The implementation classes for the AnnotatedType family used in core
reflection are in a single file AnnotatedTypeFactory.java. There are
instantiated instances both of the base class AnnotatedTypeBaseImpl and
its subclasses which implement the specialized subinterfaces of
AnnotatedType for arrays, type variables, etc.
The implementation classes do *not* declare equals, hashCode, or
toString methods, which the proposed change rectifies.
For equals and hashCode, the interfaces do *not* define how the equals
relation should be calculated, an omission shared by the Type
interfaces. While the default identity-is-equality is correct in some
sense, it is not very useful as the objects returned by successive "get
annotated return type" calls on the same method object will not be equal.
The proposed equals implementations check if the argument object
implements the same AnnotatedType subinterface (and makes sure one of
the subinterfaces is *not* implemented when determining equality for
AnnotatedType implementation objects) and compares the results of
corresponding methods. This is analogous to the equals implementations
of the Type implementation classes found in the impl classes in the
sun.reflect.generics.reflectiveObjects package.
The order of annotations is considered significant for equality; this is
arguably overly strict, but I didn't think it was worthwhile to form
sets of annotations for the purposes of equality comparison in this case.
For toString to replicate the source ordering for most kinds of types,
the type annotations, if any, occur textually to the left of the text
representing the type as in
@Nonnull String
or more interestingly
@Nonnull List
the latter meaning "a nonnull list where the list contains strings."
However, this pattern is not followed for arrays. The annotated type
@Nonnull String[]
means "an array of strings where the strings are nonnull" rather than
"an array that is nonnull where the array contains strings." The
rationale for this is explained in JSR 308 related documents such as:
https://checkerframework.org/jsr308/jsr308-faq.html#syntax
https://checkerframework.org/jsr308/specification/java-annotation-design.html
A multi-dimensional array is represented as an AnnotatedArrayType whose
component type is an array type of lower dimension, and so on, bottoming
out with a non-array component type. The existing toString output for
the Type objects of an array uses VM style notation like
"[[Ljava.lang.String" for a 2D string array. This is even less helpful
for annotated type objects and therefore toString output in the style
usable in source code ("String [] []") will be used instead. Starting
from an AnnotatedArrayType object for a 2D array of strings, using the
integer value of type annotations the encounter order for the
annotations and types is:
@TypeAnno(2) String @TypeAnno(0) [] @TypeAnno(1) []
Therefore, for multi-dimensional arrays, the results for the nested
dimensions are appended to the in-progress string builder while the
results for the non-array component are pre-pended.
Thanks,
-Joe
