Community
Participate
Working Groups
In this example, the reference to #foo in the Derived class fails to resolve public class Example { void foo() { // } /** * {@link #foo}. * @see #foo */ void goo() { // } } class Derived extends Example { /** * {@link #foo} * @see #foo */ void hoo() { // } } But the same thing is valid and works properly in the Example base class. This case comes up in EMF generated code where we'd have to generated * {@link org.eclipse.emf.edit.command.MoveCommand} in {@link #createCommand(Object, org.eclipse.emf.edit.domain.EditingDomain, Class, org.eclipse.emf.edit.command.CommandParameter)}. instead of * {@link org.eclipse.emf.edit.command.MoveCommand} in {@link #createCommand}. One might argue that it would be better to change the generator. I suppose it would be nice for this to be fixed/changed in JDT though...
I was finally getting around to using the great Java doc warnings to clean my EMF javadoc and I realized that this bug is still outstanding. There are so many cases of this bug in our generated code, that it forces me to turn off the option to check the Javadoc, which is really unfortunate because it's such a great option to have on. Any chance this could still be fixed for 3.4?
Does the Javadoc tool generate hyperlinks in these cases?
Created attachment 98791 [details] Proposed changes that make it work better.
Assigning to Frederic who will review the patch.
Created attachment 98794 [details] An example showing the changes in action I tried generating the Javadoc for this project (it's included) using Java 6.0. I tried to write the patch so that things resolve the same was as the do in the generated Javadoc... I'm not sure what cases of ambiguity you'd like to report either...
Created attachment 98802 [details] Need to properly iterate over all the interfaces
Ed, I do not think that modifying the compiler for a Javadoc issue is the right approach in this peculiar case. This would add time penalties for all compiled units and this is something we ever try to avoid. But before to try to find a fix in Javadoc AST nodes as JavadocMessageSend (may be in internalResolveType(Scope) method), I want to clarify one point. I didn't get any error if I just add parenthesis to the method references: class Derived extends Example { /** * {@link #foo()} * @see #foo() */ void hoo() { // } } In this case, the problem comes from the fact that #foo looks first as a field reference instead of a method reference. Is there any reason why the generator does not add the parenthesis to make a true method reference and avoid this issue?
We end up generating quite a few {@link #createCommand} references where the signature ends up being quite large. {@link #createCommand(Object, EditingDomain, Class, org.eclipse.emf.edit.command.CommandParameter)} Things like CommandParameter aren't typically imported into the generated class. In the past, if we did generate the import, we'd get an unused import error, though that's much better now. But the full signature still ends up being quite large. Since Javadoc itself happily references the method it's of course ideal if the Javadoc warnings in JDT don't complain about a missing reference when there is a valid resolution. I'll rethink the approach for fixing this given that my naive change-the-compiler is obviously bad. Any hints and help are greatly appreciated!
Created attachment 98964 [details] New proposed patch Currently the problem is in JavadocFieldReference.internalResolveType(Scope) method. When no binding is found for a field reference the compiler tries to see if there's a method with the field token. But the ReferenceBinding.getMethods(char[]) used to get the method binding does not walk the super classes and super interfaces hierarchy and the test case returns no binding. So, as the code in your patch walk through the hierarchy, I just move it to JavadocFieldReference to use it in the internalResolveType method and... it works perfectly :-) Please verify that it's OK for you and I'll ask a review to put this fix in RC1...
Frederic, Totally awesome. It works very well! The code generated by EMF now has no Malformed Javadoc references as verified by JDT and F3 navigates correctly. Merci beaucoup!
Created attachment 99014 [details] Same fix than previous patch with additional test cases
Philippe, could you please review?
I canceled the review as Philippe found a test case not handled by the proposed fix: The fix is not complete as it fails to solve following test case: public class X { void foo() {} class Y { /** * {@link #foo} * @see #foo */ void hoo() {} } } In this case, there's still warnings on #foo references although Javadoc tool accepts this syntax...
Also is it supposed to resolve correctly if multiple matches ? (like for static imports). e.g. class X { void foo() {} void foo(int i) {} } class Y extends Y { /** @see #foo */ void bar(){} }
I also wondered it two separate interfaces contributed different overloads if there should still be a warning? This implies that all resolutions from bases classes and interfaces should perhaps be composed into a single result, eliminating overloads. If there is only one possible resolution, that's good, but if there are multiple, a warning seems like a useful thing. interface X { void foo(); } interface Y { void foo(int i); } abstract class Z implements X, Y { /** * @see #foo */ void bar() {} } As far as I could tell, Javadoc prefers the method from the first interface (the resolution changes depending X being before Y) over a method from the base class, so in terms of F3 navigation, it would be best to resolve the same way as does the actual generated Javadoc. I imagine that might not be such a well defined thing though...
Created attachment 99047 [details] New patch also addressing comment 13 test case I also added several new tests in JavadocBugsTest and also in SelectionJavadocModelTests to verify that the bound method is always the expected one...
Comment on attachment 99047 [details] New patch also addressing comment 13 test case Sorry Ed, I did miss your last comment before posting the patch...
The fact here is that Javadoc tool never complains as soon as it can find a method with the same selector. So, I guess we need to mimic this behavior and so do not report any warning either. However, I agree that bind this reference to the first found method (as Javadoc tool does) is not the right solution (see bug 51911 comment 7). Then I propose that if the method with no parameter exists, then bind the reference to it instead whatever is position in the methods list or in the hierarchy. Otherwise, bind to the first method found in the list as Javadoc tool does... With this rule, test case of bug 51911 comment 7 will appear in Javadoc as: /** * @see <link to a()> */
Created attachment 99072 [details] Final patch As described in previous comment, this patch does no longer report any warning when several methods have the same selector than the one used for the reference. Of course I had to fix tests for bug 51911 which now are all conform although some did report warnings before. But note that Dani's argument to have the warning is no longer valid as the compiler binds to the correct method with this patch... Note that this patch also fixes remaining warning for bug 65180: the limitation does no longer exist...
Philippe, can you please review?
+1 for 3.4RC1
Released for 3.4RC1 in HEAD stream.
Verified for 3.4RC1 using build I20080510-2000.
