Skip to content
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

Provide the fully qualified class name in Link elements #20

Open
joffrey-bion opened this issue May 16, 2018 · 10 comments
Open

Provide the fully qualified class name in Link elements #20

joffrey-bion opened this issue May 16, 2018 · 10 comments
Assignees

Comments

@joffrey-bion
Copy link
Contributor

For now, the class name provided in Link elements is taken directly from the Javadoc's text. This is barely usable to actually find the corresponding class/method.
It would be nice to find a way to actually read the corresponding fully qualified class name from the imports, so that we can provide the actual Class<?>/Method object at runtime.

@bbottema
Copy link
Contributor

Actually, I've had success finding the class and even Methods from @link references by using my other open source library, Java Reflection. It's become quite advanced, here's how I use it in my Simple Java Mail spike regarding your library:

@Nonnull
private static Method findMethodForLink(Link link) {
	// there are several variations of locateClass()
	Class<?> aClass = ClassUtils.locateClass(link.getReferencedClassName(), "org.simplejavamail", null);
	assumeTrue(aClass != null, "Class not found for @link: " + link);
	Set<Method> matchingMethods = MethodUtils.findMatchingMethods(aClass, link.getReferencedMemberName(), link.getParams());
	assumeTrue(!matchingMethods.isEmpty(), format("Method not found on %s for @link: %s", aClass, link));
	assumeTrue(matchingMethods.size() == 1, format("Multiple methods on %s match given @link's signature: %s", aClass, link));
	return matchingMethods.iterator().next();
}

@joffrey-bion
Copy link
Contributor Author

joffrey-bion commented Sep 28, 2018

This looks like it requires you to know the package/root package of the class. In a generic tool generating a documentation, you can't expect every reference to have the same root package name.

How do you solve this problem if runtime-javadoc doesn't provide the information?

@bbottema
Copy link
Contributor

bbottema commented Sep 28, 2018

This looks like it requires you to know the package/root package of the class.

As the comment in the code says, there are variations, including one that does a full scan on all packages (but this can result in the wrong Class that happens to have the same name).

In a generic tool generating a documentation, you can't expect every reference to have the same root package name.

True, for me it only works because I already know the package it should be in. If it was included in Therapi, that would be impossible.

How do you solve this problem if rubtime-javadoc doesn't provide the information?

I used to use runtime-retention annotations, which is a lot of manual work in my case. That's why I got excited seeing therapi-runtime-javadoc!

@bbottema
Copy link
Contributor

bbottema commented Oct 4, 2018

Then in conclusion: unless you can get access to the actual source, there's no way to infer the packages.

@joffrey-bion
Copy link
Contributor Author

@bbottema Indeed, that's why I believe therapi-runtime-javadoc should add this piece of info during annotation processing, since it has access to the source at that moment

@dnault
Copy link
Owner

dnault commented Aug 15, 2020

I took another look at this recently, and it seems like there's no standard way to access the full source code of the class being compiled... so we can't just grab the imports. One strategy would be to walk the element tree as described in https://stackoverflow.com/questions/14734445/java-annotation-processing-api-accessing-import-statements to get the set of all referenced classes. This has at least one limitation though -- if the unqualified class name is referenced only by the javadoc, then you can't infer its package this way.

@kirkch
Copy link
Contributor

kirkch commented Sep 14, 2020

What is the latest thinking here? I see within the code base that we have ImportUtils.getImports which falls back to the compilers AST and strikes me as a very pragmatic way of moving forward given the lack of support within the annotation processor itself.

I quickly spiked wiring that up to scribe and added its results to the output Json. It appears to work pretty well and looks like a good approach. It would not take much to create a runtime resolver that could read those imports from the json and give a best effort resolution, falling back to whats in the JavaDoc when no better resolution was found.

If an approach is agreed, I am happy to help out with the leg work on this one.

@dnault
Copy link
Owner

dnault commented Sep 15, 2020

@kirkch Sounds good, you have a green light.

@dnault dnault added this to the 0.12.0 milestone Sep 15, 2020
kirkch added a commit to kirkch/therapi-runtime-javadoc that referenced this issue Sep 19, 2020
kirkch added a commit to kirkch/therapi-runtime-javadoc that referenced this issue Sep 19, 2020
kirkch added a commit to kirkch/therapi-runtime-javadoc that referenced this issue Sep 19, 2020
kirkch added a commit to kirkch/therapi-runtime-javadoc that referenced this issue Sep 19, 2020
@kirkch
Copy link
Contributor

kirkch commented Oct 3, 2020

@dnault I appreciate that you are busy and things may have slipped your mind; these PRs are a couple of weeks old now. Any chance of getting some feedback on them?

@dnault
Copy link
Owner

dnault commented Oct 3, 2020

@kirkch You're right, sorry it took me so long, and thanks for the reminder. I'll add some feedback.

@dnault dnault removed this from the 0.12.0 milestone Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants