Community
Participate
Working Groups
20020911 This isn't specifed, so my questions: Are source locations also copied to a cloned node (using ASTNode.copySubtrees) ? It seems to be for some but not all nodes. I would be happy if the source locations are copied. (The spec says that properties are not copied; what of course makes sense).
Could you please specify for which nodes the positions are preserved? I would consider positions as a client property and therefore there is no reason for them to be copied to the clone object. Jim - any comment?
I found only the copy of source locations in SimpleName and this is a bug. This is boggus code that has no reason to be there. It was an attempt to resolve a bug, but I forgot to remove this code. I will release the right code immediately.
The easiest way for your to copy the positions is probably to use a visitor and iterate the tree.
How whould you do that? It's not easy (or cheap) to find the corresponding element in the other tree. (Its easier to clear positions!) Are there good reasons to not copy source locations? I mean its a clone of a tre, so the cloned tree as well represents the Ast of a certain part of the code.
You can use a ast matcher between the clone and the original. It works because you will iterate the two trees in the same time. Something like: class CopyPositionsMatcher extends ASTMatcher { private void copyPositions(ASTNode source, ASTNode destination) { destination.setSourceRange(source.getStartPosition(), source.getLength()); } /* * @see ASTMatcher#match(ArrayType, Object) */ public boolean match(ArrayType node, Object other) { copyPositions(node, (ASTNode) other); return super.match(node, other); } /* * @see ASTMatcher#match(QualifiedName, Object) */ public boolean match(QualifiedName node, Object other) { copyPositions(node, (ASTNode) other); return super.match(node, other); } /* * @see ASTMatcher#match(SimpleName, Object) */ public boolean match(SimpleName node, Object other) { copyPositions(node, (ASTNode) other); return super.match(node, other); } /* * @see ASTMatcher#match(SimpleType, Object) */ public boolean match(SimpleType node, Object other) { copyPositions(node, (ASTNode) other); return super.match(node, other); } /* * @see ASTMatcher#match(PrimitiveType, Object) */ public boolean match(PrimitiveType node, Object other) { copyPositions(node, (ASTNode) other); return super.match(node, other); } ..... } I agree that this is not efficient, but it should work.
I forgot to put the usage of this class: Type clone = (Type) ASTNode.copySubtree(this.ast, type); type.subtreeMatch(new CopyPositionsMatcher(), clone);
The API spec for ASTNode.copySubtree etc. is silent on whether source positions are copied or not. Consequently, no client cannot rely on them being copied or not copied. I think it would be better for clients if this aspect was specified one way or the other. As Martin points out, it is easier for a client to clear unwanted source positions than to copy them over if the do want them. So I suggest changing the spec and implementation to always copy source range info.
This is what I did in the past and you said that clients might not need the source positions and therefore they should pay for it. It is slower to copy all positions. I have done that when I tried to get the subpart of an array type in the converter. So I change the current implementation to copy the source positions?
Ok, then change the specs and I will update the implementation as soon as the specs are changed.
Changed spec for ASTNode.copySubtree/s to say: * Source range information on the original nodes is automatically copied to the new * nodes. Client properties (<code>properties</code>) are not carried over.
Fixed and released in 2.1 stream.
Regression tests added. Verified.
Verified.
