Bugzilla – Full Text Bug Listing
| Summary: | [api][breaking] Get rid of invalid implementation of SystemMessage#clone() | ||
|---|---|---|---|
| Product: | [Tools] Target Management | Reporter: | Martin Oberhuber <mober.at+eclipse> |
| Component: | RSE | Assignee: | Martin Oberhuber <mober.at+eclipse> |
| Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | dmcknigh |
| Version: | 2.0 | Keywords: | api |
| Target Milestone: | 3.0 M6 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
As a first step, I'm committing a fixed version of SystemMessage#clone() for educational purpose: [219975] Fix SystemMessage#clone() SystemMessage.java v1.11 SimpleSystemMessage.java v1.4 SystemUIMessage.java v1.3 In the second step, I'm getting rid of SystemMessage#clone(): [219975][api][breaking] Get rid of SystemMessage#clone() SystemMessage.java v1.12 SimpleSystemMessage.java v1.5 SystemUIMessage.java v1.4 Found some other places where clone() was not properly implemented or documented. There are also API changes involved: * RemoteFile - removed "Cloneable" from implements * SystemRemoteResourceInfo - removed "Cloneable" from implements These are not really an API changes, since Cloneable is just a markup interface so only runtime behavior is changed; and, those classes could never properly have been cloned because their state includes complex objects which have never been properly cloned before. So, if CloneNotSupportedException occurs at runtime, it will only be an indication that incorrect code has been exposed. [219975] Fix implementations of clone() SystemRemoteResourceInfo TarEntry HostProcessFilterImpl SystemFileTransferModeMapping RemoteFileFilterString RemoteFile RemoteCommandFilterString SystemCompileCommand |
The SystemMessage#clone() method is incorrectly implemented: * class SystemMessage does not implement the Cloneable interface * SystemMessage#clone() is only protected so only subclasses could call it * It's documentation does not state that substitutions are not cloned * Result of the clone is always a SystemMessage and not the runtime type of an extending class In the end, cloning a SystemMessage is risky because the Substitutions Object[] array may contain objects of complex type that cannot be cloned (and would thus throw a CloneNotSupportedException at runtime). Therefore, it is better to get rid of the method - it's unclear what the use of cloning a SystemMessage would be, anyways. This is a breaking API change because subclasses of SystemMessage could call clone(). But this is unlikely since SystemMessage was never meant to be extended.