Community
Participate
Working Groups
20070503 When there is a binary super type that has unresolved return types or parameters, the binding AST does not return any fields or methods. From discussions in 184613, we realized it would be good to have at least the members that can be resolved. At the moment these members can exist as a reference binding in the AST, but they not in the children list of their declared type. -> fieldBinding.getDeclaringClass().getDeclaredFields() does not contain fieldBinding
Created attachment 65740 [details] Proposed patch More invocations of ReferenceBinding#methods()/fields() should be addressed (ping'ed David for codeassist/codeselect). Owners of search/binding key/DOM AST should review the proposed changes
Kent - for 3.3, I think we should ensure more are using #availableFields/availableMethods(), but looking into 3.4 I am wondering if we shouldn't rather do sthg different: Simply make BinaryTypeBinding#methods() and #fields() more resilient (i.e. swallow the behavior of available[Methods/Fields] into them. Rationale: would be less error prone for existing users, and make binary side be consistent with source side (which is already resilient). Now, this is fairly late to do in 3.4, and we would need to test/bullet-proof our story a little before going there (thinking of incremental builder consequence for instance). David/Jerome/Frederic/Olivier - if comfortable with suggested patch, please say so, and provide regression tests demonstrating the net effect (on resilience).
Kent - if you agree with comment 2, pls file a separate bug for evolving it into 3.4.
Created attachment 65799 [details] Patch fixing failing search tests Some potential matches become exact ones with this fix. Fortunately, there were 3 tests on invalid class path, 2 pass with Philippe's patch, 1 is still failing => sounds like we still cover all cases... So, my patch is simply to update the output assertion for the failing tests...
Maybe I don't understand exactly what u want to do... BinaryTypes need to cause classpath errors to happen during the normal compile loop. I don't see why we would want to avoid this happening. Other users such as code assist can skip fields/methods with unresolvable types, but a normal compile loop MUST fail if the classpath is incomplete.
The javadoc would need to be updated for getDeclaredMethods() and getDeclaredFields() to reflect the new behavior.
(In reply to comment #5) > BinaryTypes need to cause classpath errors to happen during the normal compile > loop. I don't see why we would want to avoid this happening. > Other users such as code assist can skip fields/methods with unresolvable > types, but a normal compile loop MUST fail if the classpath is incomplete. Kent, this is exactly what the patch is doing. It affects only DOM and search. So now in the DOM you can retrieve all the methods/fields that can actually be resolved instead of an empty list as soon as one fails to resolve. I am comfortable with the patch and I'll prepare a regression test in DOM to show that more fields or methods are now returned. I still need to be able to clarify the javadoc of getDeclaredFields() and getDeclaredMethods(). Martin, any suggestions?
sounds good to me!
I meant any suggestions on how to rephrase the javadoc of these two methods :-).
I do have a regression test for the DOM side, but since it contains some binary resources I cannot directly attach it here. I'll release it as a disabled test. See org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#_test0677
/** * Returns a list of bindings representing all the fields declared * as members of this class, interface, or enum type. These include public, * protected, default (package-private) access, and private fields declared * by the class, but excludes inherited fields. Synthetic fields may or * may not be included. * Fields from binary types that reference unresolvable types may not be included. * ...
Forgot to request explicit review.
Reviewed Philippe's patch. I'll adjust the javadoc and release.
Released for 3.3RC1. Enabled org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0677
Created attachment 66592 [details] Proposed fix New patch with doc updates.
Please undo this change (at least for search). As shown in the patch to Java search tests, we now miss valid potential matches.
(In reply to comment #16) > Please undo this change (at least for search). As shown in the patch to Java > search tests, we now miss valid potential matches. > My fault, I missed this point :-( We can either revert the change on ClassFileMatchLocator or complete it by not returning when there's not the same number of fields/methods and availableFields/Methods. I'm working on it...
Talking with Philippe and Jerome about this, we agreed that it will be safer to revert the change on ClassFileMatchLocator and open a new bug against Search component to improve it in this peculiar case...
I opened bug 186333 to fix the Search Engine issue and target it to 3.4...
(In reply to comment #16) > Please undo this change (at least for search). As shown in the patch to Java > search tests, we now miss valid potential matches. Done. Both search and search tests have been reverted.
Verified for 3.3 RC1 using I20070515-0010
