S2975: For a final class, it should be allowed to leave out super.clone()

I am using Eclipse SonarLint 4.3.12432.

I get squid:S2975, when I use the private copy constructor of my final class in clone().

public final class FinalClass implements Cloneable {
    public FinalClass() {}
    public FinalClass(FinalClass base) {}
    @Override public FinalClass clone() {
        return new FinalClass(this);  // NOSONAR false-positive squid:S2975
    }
}

Hi,

  • I don’t see you are using private copy constructor (I see only 2 public constructors)
  • If having copy constructor why having also clone?

This rule might not necessarily report the problem in your code, but some location which might lead to the problem in the future.

If you believe you are safe and there is no sense in refactoring your code, it’s ok.
Still I don’t see a consistent pattern this rule should avoid.

Hi,

Sorry, it’s a mistake, the second constructor is private:

private FinalClass(FinalClass base) {}

But my problem is independent of this. I think, the super.clone() requirement shouldn’t be applied to non-final classes. Or, is there a sane reason?

So, is the final class modifier a consistent pattern when this rule should avoid?

Hello @davidsusu,

Before digging into the problem, I want to say that the information you gave us is quite sparse, I had to extrapolate a lot. The problem discussed is interesting, and you should motivate a bit more your suggestions.

The confusion comes from the fact that you state rule S2975, but it seems to me that you are in fact discussing rule S1182.

Without this confusion, the problem is clearer: one of the concern of this rule is that if you extend a class defining clone() without a call to super.clone(), you are likely to violate the invariant #2: x.clone().getClass() == x.getClass(). If the class is final though, you are not subject to this problem.

What worries me at this point is that super.clone() is not here only for invariant #2, have you though about how it could affect them?

In addition, could you explain me why you have the need for both a clone and a copy constructor? From my point of vue, a copy constructor is here exactly to avoid all the problems of clone.

You are right, now I see that this is S1182. Hm, probably, there was something wrong with the problem view in Eclipse, sorry.

There are several reasons for using clone() instead of a constructor or static copy method. The three most important are:

  • using in a generic context, e. g. T item = item.clone() (where <T extends Cloneable>)
  • method chaining
  • the name ‘clone’ is semantic/explicit

Of course, all of these can be replaced by other patterns, but, at least for me, clone() seems to be the simplest solution.

Out of curiosity, why do you want to implement Cloneable?

  • Generic context doesn’t work because Cloneable interface doesn’t have clone method.
  • Even if generic context did work, the clone contract requires that x.clone() != x which means that immutable objects don’t implement Cloneable (as doing so would encourage wasteful copying).
  • You don’t need to implement Cloneable if you want a clone method in your object. Implementing Cloneable interface is only necessary if you want to use super.clone().
  • Having code where method chaining with clone() method makes the code cleaner compared to clone constructor likely violates Law of Demeter.
  • About semanticness/explictiness of clone method, I would argue that having a clone constructor (a constructor that accepts an object of its own class) is more common in Java code.

I would argue there is no reason to implement clone unless you need to because you are extending a class that implements Cloneable.

Right. However, someone could have a subinterface that extends pure Cloneable.

I don’t want to inherit immutable objects from Cloneable.

This is your best argument against using Cloneable without using super.clone(). I would allow it anyway.

Right. However, I am not an unconditional fan of LoD because of its reverse coupling and vertical chaining traps.

It’s probably true. But I am trying to minimize the number of constructor calls outside of factories.

All in all, this rule is based on Oracle’s Javadoc convention, by definition, you are free to not follow it if you are confident your solution will work as expected and without confusion for future readers.

If it’s the case, resolving the issue as “won’t fix” seems a good option.

Best,
Quentin