No clone method¶
ID: java/missing-clone-methodKind: problemSecurity severity: Severity: errorPrecision: mediumTags: - quality - reliability - correctnessQuery suites: - java-security-and-quality.qls
Click to see the query in the CodeQL repository
A class that implementsCloneable should overrideObject.clone. For non-trivial objects, theCloneable contract requires a deep copy of the object’s internal state. A class that does not have aclone method indicates that the class is breaking the contract and will have undesired behavior.
The Java API Specification states that, for an objectx, the general intent of theclone method is for it to satisfy the following three properties:
x.clone()!=x(the cloned object is a different object instance)x.clone().getClass()==x.getClass()(the cloned object is the same type as the source object)x.clone().equals(x)(the cloned object has the same ‘contents’ as the source object)For the cloned object to be of the same type as the source object, non-final classes must callsuper.cloneand that call must eventually reachObject.clone, which creates an instance of the right type. If it were to create a new object using a constructor, a subclass that does not implement theclonemethod returns an object of the wrong type. In addition, all of the class’s supertypes that also overrideclonemust callsuper.clone. Otherwise, it never reachesObject.cloneand creates an object of the incorrect type.
However, asObject.clone only does a shallow copy of the fields of an object, anyCloneable objects that have a “deep structure” (for example, objects that use an array orCollection) must take the clone that results from the call tosuper.clone and assign explicitly created copies of the structure to the clone’s fields. This means that the cloned instance does not share its internal state with the source object. If itdid share its internal state, any changes made in the cloned object would also affect the internal state of the source object, probably causing unintended behavior.
One added complication is thatclone cannot modify values in final fields, which would be already set by the call tosuper.clone. Some fields must be made non-final to correctly implement theclone method.
Recommendation¶
The necessity of creating a deep copy of an object’s internal state means that, for most objects,clone must be overridden to satisfy theCloneable contract. Implement aclone method that properly creates the internal state of the cloned object.
Notable exceptions to this recommendation are:
Classes that contain only primitive types (which will be properly cloned by
Object.cloneas long as itsCloneablesupertypes all callsuper.clone).Subclasses of
Cloneableclasses that do not introduce new state.
Example¶
In the following example,WrongStack does not implementclone. This means that whenws1clone is cloned fromws1, the defaultclone implementation is used. This results in operations on thews1clone stack affecting thews1 stack.
abstractclassAbstractStackimplementsCloneable{publicAbstractStackclone(){try{return(AbstractStack)super.clone();}catch(CloneNotSupportedExceptione){thrownewAssertionError("Should not happen");}}}classWrongStackextendsAbstractStack{privatestaticfinalintMAX_STACK=10;int[]elements=newint[MAX_STACK];inttop=-1;voidpush(intnewInt){elements[++top]=newInt;}intpop(){returnelements[top--];}// BAD: No 'clone' method to create a copy of the elements.// Therefore, the default 'clone' implementation (shallow copy) is used, which// is equivalent to://// public WrongStack clone() {// WrongStack cloned = (WrongStack) super.clone();// cloned.elements = elements; // Both 'this' and 'cloned' now use the same elements.// return cloned;// }}publicclassMissingMethodClone{publicstaticvoidmain(String[]args){WrongStackws1=newWrongStack();// ws1: {}ws1.push(1);// ws1: {1}ws1.push(2);// ws1: {1,2}WrongStackws1clone=(WrongStack)ws1.clone();// ws1clone: {1,2}ws1clone.pop();// ws1clone: {1}ws1clone.push(3);// ws1clone: {1,3}System.out.println(ws1.pop());// Because ws1 and ws1clone have the same// elements, this prints 3 instead of 2}}
In the following modified example,RightStackdoes implementclone. This means that whenrs1clone is cloned fromrs1, operations on thers1clone stack do not affect thers1 stack.
abstractclassAbstractStackimplementsCloneable{publicAbstractStackclone(){try{return(AbstractStack)super.clone();}catch(CloneNotSupportedExceptione){thrownewAssertionError("Should not happen");}}}classRightStackextendsAbstractStack{privatestaticfinalintMAX_STACK=10;int[]elements=newint[MAX_STACK];inttop=-1;voidpush(intnewInt){elements[++top]=newInt;}intpop(){returnelements[top--];}// GOOD: 'clone' method to create a copy of the elements.publicRightStackclone(){RightStackcloned=(RightStack)super.clone();cloned.elements=elements.clone();// 'cloned' has its own elements.returncloned;}}publicclassMissingMethodClone{publicstaticvoidmain(String[]args){RightStackrs1=newRightStack();// rs1: {}rs1.push(1);// rs1: {1}rs1.push(2);// rs1: {1,2}RightStackrs1clone=rs1.clone();// rs1clone: {1,2}rs1clone.pop();// rs1clone: {1}rs1clone.push(3);// rs1clone: {1,3}System.out.println(rs1.pop());// Correctly prints 2}}
References¶
J. Bloch,Effective Java (second edition), Item 11. Addison-Wesley, 2008.
Java API Specification:Object.clone().