-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enforce rawType to be a Class in ParameterizedTypeImpl #2759
Enforce rawType to be a Class in ParameterizedTypeImpl #2759
Conversation
I think we should probably just change the type of the Ordinarily we wouldn't change parameter types in a public API, but in this case the API is doubly marked as internal: it has |
06ab36e
to
0cffb73
Compare
I agree with you. I did what you suggested, in this case in I also directly removed a small piece of code in Also, am I correct that we can ignore the error |
0cffb73
to
ce0fb34
Compare
*/ | ||
public static TypeToken<?> getParameterized(Type rawType, Type... typeArguments) { | ||
public static TypeToken<?> getParameterized(Class<?> rawType, Type... typeArguments) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, we can't change the signature of this method. It is part of the public API and it is not internal. So I think you want to undo the changes in this method, and just change rawType
to rawClass
in the return
statement.
@@ -186,16 +185,7 @@ public void testParameterizedFactory_Invalid() { | |||
NullPointerException.class, | |||
() -> TypeToken.getParameterized(List.class, new Type[] {null})); | |||
|
|||
GenericArrayType arrayType = (GenericArrayType) TypeToken.getArray(String.class).getType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would undo these changes too.
c83c6fd
to
e1b2d49
Compare
Enforce rawType to be a Class in ParameterizedTypeImpl Enforce rawType to be a Class in ParameterizedTypeImpl
e1b2d49
to
e44c37e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is great!
There is a JDK-6255169 issue that states that
getRawType()
inParameterizedType
should returnClass
and notType
. However, this issue is closed, it was explained by the fact that there is no way to fix it due to backward compatibility. Our TODO specifies a related issue JDK-8250659 which calls to comment out thatgetRawType()
always returnsClass
asType
Accordingly, looking at all this, we implement
ParameterizedType
and assume thatrawType
will always beClass
. However, in our implementation class in the constructor we can throw underrawType
more than justClass
, which is not the right thing to doHere I tried to solve this problem by throwing the
IllegalArgumentException(“rawType must be a Class type”)
exception