Community
Participate
Working Groups
The NodeFinder class is part of the package org.eclipse.jdt.internal.corext.dom and provides very usefull logic. Therefore I would like to use some similar functions in my plugin. Sadly the NodeFinder is part of an internal package and should only be used by plugins strongly related to jdt. (My isn't). Would be nice if the node finder (or a similar one) is becomes part of the AST API. Thanks, Martin (Kersten)
Also the GenericVisitor would be a nice to have feature. Maybe the ASTNodeVisitor may replicate the behaviour. But may be you need it for other purpose. Don't know. But I think the AST API still lacks ways to find a particular node. Would be nice to have the NodeFinder and GenericVisitor in a non-internal package.
Deferred
I would like to repoen this bug for comments: It would be nice to make NodeFinder or at least GenericVisitor API. WTP EJB code generation makes use of them (for a simple task to find ASTNodes of TypeDecleration) I think they are useful outside the boundaries of JST
Adding david to CC
Olivier, please look at this for M3
Need to synchronize with JDT/UI since they could then use the core version of the class. Right now this class is inside the JDT/UI component. Markus, can we see this together post M2 ?
Created attachment 112408 [details] add boolean ASTVisitor#preVisit2(ASTNode) > Markus, can we see this together post M2 ? Sure, but unfortunately I'll be away till 2008-10-06, so I can only give feedback in 3 weeks. NodeFinder needs some Javadoc cleanup, and I would - make it final or @noextend, and - use a private class to implement the actual ASTVisitor I think the GenericVisitor is only there because of a deficiency in the ASTVisitor API. The problem is that the generic preVisit(ASTNode) should return a boolean like all the type-specific visit(*) methods. See the attached patch for how to make the GenericVistor obsolete.
Created attachment 115071 [details] Proposed fix This patch overrides the previous one by defining a new NodeFinder class inside the org.eclipse.jdt.core.dom package.
It might evolve over time to meet more specific requirements.
(In reply to comment #7) > The problem is that the generic preVisit(ASTNode) should return > a boolean like all the type-specific visit(*) methods. Hmm, 'boolean preVisit2(ASTNode)' is enough for NodeFinder, but it's not a full replacement for the GenericVisitor. We have cases where subclasses of GenericVisitor override visitNode(ASTNode) as well as some visit(...) methods. We have to see case-by case whether preVisit2(ASTNode) is actually sufficient to implement all these cases, but I guess it's not. I don't have time to do this analysis for M3, so I opened bug 250944. We will probably still need GenericVisitor, either as a separate class, or integrated into the ASTVisitor.
So Markus, should I release the actual patch and let you adapt to it to see if you need more changes ?
> So Markus, should I release the actual patch and let you adapt to it to see if > you need more changes ? Yes, and if you want to be nice to other clients, you could add a warning that the API is still in flux to NodeFinder and ASTVisitor#preVisit2(ASTNode).
So I could add something like: This API is still subject to modifications before the 3.5 release.
> This API is still subject to modifications before the 3.5 release. Yeah, or: This API is still under discussion, see https://bugs.eclipse.org/53024 .
Created attachment 115150 [details] Proposed fix New patch with the warning in NodeFinder and ASTVisitor.preVisit2.
Released for 3.5M3. These is nothing to verify as is. Once JDT/UI will adopt this new API, all existing refactoring tests will extensively use it.
Verified for 3.5M3 using JDT/Core plug-in source code of I20081026-2000.
(In reply to comment #15) > Created an attachment (id=115150) [details] > Proposed fix > > New patch with the warning in NodeFinder and ASTVisitor.preVisit2. The warning is still in the sources. Since the bug is closed, should the comments be removed now?
>Since the bug is closed, should the comments be removed now? Yes. Olivier please commit my patch.
Created attachment 148752 [details] Fix that removes outdated comments.
(In reply to comment #19) > Yes. Olivier please commit my patch. Done.
*** Bug 220185 has been marked as a duplicate of this bug. ***
