Community
Participate
Working Groups
2.1 The Javadoc of CompilationUnit.findDeclaringNode says * The following table indicates the expected node type for the various * different kinds of bindings: ... * <code>ClassInstanceCreation</code> (for anonymous classes) </li> The implementation is different: When called with a anonyomous type binding, a 'AnonymousClassDeclaration' is returned.
Since ClassInstanceCreation and AnonymousClassDeclaration are in one-to-one correspondence and bi-directionally linked, it could be spec'd either way. I recommend bringing the implementations of both CompilationUnit.findDeclaringNode methods into line with the existing specs.
It looks awkward to me that the expected node is a class instance creation in case of an anonymous class. The type binding used to get this node is retrieved using resolveTypeBinding() on the AnonymousTypeDeclaration node. So it would make sense to me that the reverse operation (starting from the binding to the declaring node) returns the AnonymousTypeDeclaration. Then getting the parent is good enough to get the corresponding class instance creation node. Any comment?
I agree with Olivier. The node that contains the .resolveTypeBinding should be returned. Changing the spec instead of the implementation also has the advantage of not breaking existing code.
I could go either way, but not for a bogus argument. The only client code it doesn't break is existing code replying on the buggy behavior rather than what's spec'd. We need to be more careful at detecting and fixing these mistakes before we ship them. Flame off ;-) If we do change the spec, we would need to document it in the compatibility section of the release notes because it is a breaking API change. On the other hand, if we fix the bug in the implementation, and JDT UI fixes their bug, and we're done.
I opt to change the spec since I would expect to get the type declaration node not the instance creation node when call findDeclaration(...). This makes it consistent with the rest of the return values of findDeclaringNode.
I agree that we should have found this one prior to the release. The point is that the spec is inconsistent, because in one case the node is not the one that was used to resolve the binding. Changing the spec makes more sense to me even if we need to document it. For me this is the same problem than bug 25330. The spec could be improved to clean up the user code.
Agreed. It is more consistent if cu.findDeclaringNode(binding) returns a node with the property that node.resolveBinding() == binding. Changing the spec. For the API compatibility section of the release notes: The API specs for both CompilationUnit.findDeclaringNode methods were changed to state the AnonymousClassDeclaration node is returned for anonymous class bindings, rather than the parent ClassInstanceCreation node. This makes the spec consistent in that cu.findDeclaringNode(binding) == node always implies node.resolveBinding() == binding. This brings the spec into line with what was implemented all along, so there is low risk of negatively impacting existing clients.
Fixed and released in HEAD. Regression tests added.
Change milestone.
Verified.
