Skip to content
This repository has been archived by the owner on Feb 6, 2019. It is now read-only.

Wrong behavior of Type.isAssignableBy(...) for TypeVariables #247

Closed
mlangkabel opened this issue Jun 16, 2017 · 3 comments
Closed

Wrong behavior of Type.isAssignableBy(...) for TypeVariables #247

mlangkabel opened this issue Jun 16, 2017 · 3 comments
Assignees

Comments

@mlangkabel
Copy link
Collaborator

It looks like the behavior of Type.isAssignableBy(...) for TypeVariables is not implemented correctly yet. Consider the following code snippet:

ReflectionTypeSolver typeSolver = new ReflectionTypeSolver();
TypeParameterDeclaration tpDeclA = TypeParameterDeclaration.onType("A", "foo.Bar", Collections.emptyList());
TypeVariable tpA = new TypeVariable(tpDeclA);
ReferenceTypeImpl tpString = new ReferenceTypeImpl(new ReflectionClassDeclaration(String.class, typeSolver), typeSolver);
boolean assignable1 = tpString.isAssignableBy(tpA);
boolean assignable2 = tpA.isAssignableBy(tpString);

When executing this code both assignable1 and assignable2 evaluate to true where they actually should be false (assignable1 would only be true in case tpA would have an extends bound for String).

@mlangkabel mlangkabel self-assigned this Jun 16, 2017
@mlangkabel
Copy link
Collaborator Author

I managed to fix the first of the two cases mentioned above (boolean assignable1 = tpString.isAssignableBy(tpA);) and I also wrote a test for that. However, the second case may not behave wrongly at all. I thought that it had to be working incorrectly because the following code snippet would not compile in java.

<T> void foo() {
	T t = new String();
}

But it turnes out that the isAssignable method is used when trying to figure out if a method declaration is applicable when solving a method call (regarding the method's name and the passed argument types). In this case a method declaration that specifies a parameter type T can be applicable when called with a String argument :)

@ftomassetti
Copy link
Member

It is tricky, isn't it? :D
I agree with your thinking process, and I think the only way out of this is building one example at the time. Probably I should also document better isAssignableBy. However your PR for this one looks good to me, so we are getting closer! And we also remove one "TODO" from the code

ftomassetti added a commit that referenced this issue Jun 18, 2017
@mlangkabel
Copy link
Collaborator Author

Oh yes, it's tricky. I only had to change a few lines of code but it felt like a whole day of debugging ;) Thanks for the review, I'll close this one now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants