search

GumTreeDiff / gumtree

An awesome code differencing tool
https://github.com/GumTreeDiff/gumtree/wiki
GNU Lesser General Public License v3.0
933 stars 174 forks source link

[Question] Why does deepCopy not copy over metadata? #361

Closed kjy5 closed 3 months ago

kjy5 commented 4 months ago

The JavaDoc for deepCopy says

Make a deep copy of the tree. Deep copy of node however shares Metadata

But it's unclear what "Deep copy of node" means. The default behavior of deepCopy does not copy metadata which seems unintuitive since the purpose of the function is to make a faithful copy of the tree.

This can be fixed by adding

this.getMetadata().forEachRemaining(entry -> copy.setMetadata(entry.getKey(), entry.getValue()));

to the implementations, so that makes me think not including this was on purpose. Why?

I came across this issue when I tried using deepCopy with the expectation that metadata was copied over but I was surprised when it wasn't. I'm using the above code to fix this in my usage.

jrfaller commented 3 months ago

Hi, and thanks a lot for the report.

Maybe it's a bug, maybe it's a feature :-) but it's clearly inconsistent with the documentation.

With the code you propose, it would not be a deep copy anymore since the metadata objects would be shared. However, deep copying arbitrary objects in Java is tough, so maybe it's best to leave it this way (not copying anything) and document it? I am not sure what's best.

Cheers.

kjy5 commented 3 months ago

Ah, that's a good point. I wasn't putting objects in the metadata so this would've worked for my purposes. Yes, I think a documentation update would probably make more sense here.