[Fixes #3099] Added ability to solve type with a list of expected type arguments #3213
Conversation
|
This PR is not ready to be merged. This needs more tests to make sure the intended behavior is covered. |
Codecov Report
@@ Coverage Diff @@
## master #3213 +/- ##
===============================================
+ Coverage 57.313% 57.326% +0.012%
- Complexity 2671 2680 +9
===============================================
Files 635 635
Lines 33566 33576 +10
Branches 5790 5791 +1
===============================================
+ Hits 19238 19248 +10
+ Misses 12275 12274 -1
- Partials 2053 2054 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Ready to be reviewed! :) |
|
Can you reorganize your commits? |
…g, List<ResolvedType>)
4everTheOne
force-pushed
the
fix/illegal-argument-exception
branch
from
1388ad1
to
e844b57
Compare
|
Updated commit history to simplify reading |
| } | ||
| } | ||
| } | ||
|
|
||
| // Check if the node implements other types |
There was a problem hiding this comment.
Probably "Check if the node extends other types"
|
|
||
| assertTrue(alphaContext.solveType("Foo").isSolved()); | ||
| assertTrue(alphaContext.solveType("Foo", Collections.emptyList()).isSolved()); | ||
| assertFalse(alphaContext.solveType("Foo", Collections.singletonList(ResolvedPrimitiveType.INT)).isSolved()); |
There was a problem hiding this comment.
Why is the type "Foo" resolved in this context?
There was a problem hiding this comment.
In this assertions, there are three things being confirmed:
assertTrue(alphaContext.solveType("Foo").isSolved());
We are making sure, that we are retro-compatible, so if someones is using the API without type parameters, the type is solved using the previous rules.
assertTrue(alphaContext.solveType("Foo", Collections.emptyList()).isSolved());
Looking at the class definition, the class Foo does not have type parameters. In this test, we make surethat trying to resolve it with a empty list it's resolved. Note that a empty list in type arguments is the same as saying it does not have type parameter.
assertFalse(alphaContext.solveType("Foo", Collections.singletonList(ResolvedPrimitiveType.INT)).isSolved());
Test to make sure, if we try to resolve Foo with a type parameter, it's not resolved since the declaration says it does not have one.
| ClassOrInterfaceDeclarationContext betaContext = new ClassOrInterfaceDeclarationContext(beta, typeSolver); | ||
|
|
||
| assertTrue(betaContext.solveType("Foo").isSolved()); | ||
| assertFalse(betaContext.solveType("Foo", Collections.emptyList()).isSolved()); |
There was a problem hiding this comment.
I'm not sure if this is the expected result?
There was a problem hiding this comment.
Considering now that Foo has a type argument (T), this test should Fail.
Since this assertion, is trying to solve a type "Foo" that does not have any type parameters.
This would be equivalent of doing this in IDE, which is not possible:
class Foo<T> {}
Foo<> fooObject;
|
|
||
| assertTrue(betaContext.solveType("Foo").isSolved()); | ||
| assertFalse(betaContext.solveType("Foo", Collections.emptyList()).isSolved()); | ||
| assertTrue(betaContext.solveType("Foo", Collections.singletonList(ResolvedPrimitiveType.INT)).isSolved()); |
There was a problem hiding this comment.
I'm not sure if this is the expected result?
There was a problem hiding this comment.
Considering now that Foo has a type argument (T), this test should be valid, since this assertion, is trying to solve a type "Foo" that does have a type parameters of integer type.
This would be equivalent of doing this in IDE, which is possible:
class Foo<T> {}
Foo<Integer> fooObject;
|
Thank you for this contribution. |
jlerbsc
added
PR: Added
Currently, when JP SS is trying to solve a type, it doesn't take into consideration the number of type arguments it expects to find. This have been an issue that was reported in #3099 where we can have two different classes/interfaces with the same name. In this scenario JP SS will return the first one it finds, and not the one that fits the expected type.
This PRs adds a new method in context to solve a type with the expected type arguments. If the type arguments is provided it will only find a type where the type arguments matchs the expected.
Examples
Lets imagine we have two declarations of
Foo. We have comAlpha.FooandBeta.Foo<T>whereFoois a inner class ofAlphaandFoo<T>is a inner class ofBeta. If we want to solveAlpha.Foowithout arguments we can solve it like:if we want to resolve it as
Beta.Foo<T>we execute:If we just want it to be resolved and don't care how many arguments it has we can use:
Implementation notes
ClassOrInterfaceDeclarationContextwas moved toJavaParserTypeDeclarationAdapter, becauseAnnotationDeclarationContextshould also follow the same rules;JavaParserTypeDeclarationAdapterwas reordered to match the expected behavior (the same order that was previously inClassOrInterfaceDeclarationContext).Related Issues