Support go to declaration for Mapper#qualifiedByName#119
Support go to declaration for Mapper#qualifiedByName#119filiphr merged 4 commits intomapstruct:mainfrom
Mapper#qualifiedByName#119Conversation
thunderhook
force-pushed
the
go-to-qualified-by-name
branch
2 times, most recently
from
69c303c to
0446f0b
Compare
provides code completion and resolving of named methods from local and external (defined with `uses = ...`) mappers
thunderhook
force-pushed
the
go-to-qualified-by-name
branch
from
0446f0b to
ac592b0
Compare
|
@thunderhook Looks awesome thanks! Can you tell when approximately this should get merged? |
|
Since I am not a part of the plugin maintainers, I can't really tell. It depends on the free time of the plugin maintainers (e.g. @filiphr) and if the pull request looks good to them. If they get time, it usually will be released quickly. 🙂 |
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
This is really great work @thunderhook. The functionality is great. I've added some comments to clean it up a bit.
Please let us know if you don't have the time to finish it.
Btw. sorry for the late reply, but I've been pretty busy lately and haven't had enough time to look into this.
| * @return the classes / interfaces that are defined with the {@code uses} attribute, | ||
| * or {@code null} if there isn't anything defined | ||
| */ | ||
| public static List<PsiClass> resolveUsesConfigClasses(PsiAnnotation mapperAnnotation) { |
There was a problem hiding this comment.
Can we also look into the Mapper#config class for this? We do something with it in TargetUtils#isBuilderEnabled (as a reference example). Would also be good to add a test case for it as well
| .filter( MapstructUtil::isNamedMethod ) | ||
| .filter( this::methodHasReturnType ) |
There was a problem hiding this comment.
Let's move these checks in the findAllMethodsFromThisAndReferencedMappers. That one is already partially doing this for the named methods in used mappers.
| .filter( MapstructUtil::isNamedMethod ) | ||
| .filter( this::methodHasReturnType ) |
There was a problem hiding this comment.
Let's move these checks in the findAllMethodsFromThisAndReferencedMappers. That one is already partially doing this for the named methods in used mappers.
| } | ||
|
|
||
| @NotNull | ||
| private List<PsiMethod> findAllMethodsFromThisAndReferencedMappers(@NotNull PsiMethod mappingMethod) { |
There was a problem hiding this comment.
I think that it is better if we return a Stream<PsiMethod> here
| method, | ||
| lookupString, | ||
| lookupString, | ||
| String.format( |
There was a problem hiding this comment.
I think that for this it would be good to show the method parameters here as well. e.g. StringMapper#trim(String).
|
I added the named method resolving of mappers that are referenced by If you have any other ideas for this plugin (with that kind of complexity), please let me know, and I'm willing to help out again. 👍 |
|
That's a lot for your changes @thunderhook. I pushed one more commit with some small polishing.
Thanks for offering your help. I think that you can have a look at the open issues to see what is being requested. If you are interested in something let us know. Also if you have some other ideas that you'd like to see let us know, we are more than happy to accept your contributions. |
3 participants
This pull request closes #107.
What it does:
@Mapping#qualifiedByName@Mapper(uses = AnotherMapper.class))@BeforeMapping/@AfterMapping: I just provide named methods containing a return type different thanPsiType.VOID- maybe this needs to be more precise?What it does not:
@BeanMapping#qualifiedByName: I tried to implement it, but discarded it due to the complexity of nested qualifiers as seen here