Java "Suspicious assignment in copy constructor" for byte[] – What is suspicious?

Issue

I have a copy constructor for class, but Android Studio code inspection throws a warning I don’t understand:

Suspicious assignment in copy constructor of
‘java.util.Arrays.copyOf(other.value, other.value.length)’ to field
value

public class CpuVariable extends BaseIdentifier {
    private int memoryType;
    private byte[] value;

    public CpuVariable(@NonNull CpuVariable other) {
        super(other);
        this.memoryType = other.memoryType;
        if (other.value != null) {
            this.value = java.util.Arrays.copyOf(other.value, other.value.length);
        }
    }
}

Changing code to

            this.value = other.value

would remove the warning, but this is not an option since I need to create a deep copy or a clone for the field.

Am I coding something wrong or is it safe to ignore or suppress the warning?

Solution

It is clearly a false positive. There is nothing actually wrong with your constructor.

I think that the code that produced this warning is based on this code. Note that this is not the real Android Studio code, but there are clues to suggest that Android Studio may have "borrowed" it via some path.

If you look at the constructorAssignsAllFields method (line 63), the intent of the code seems to be to look for code bugs where a copy constructor is copying the wrong fields; e.g. something like this:

MyClass(MyClass other) {
   this.x = other.x;
   this.y = other.x; // Ooops
}

But the method is not correctly dealing with the case where the copy constructor is transforming one of the fields.

Looking at the code, you need to write this.value = in a way that makes the checker not realize that it is assigning to a field. For example, if you used a setter method something like this:

public CpuVariable(@NonNull CpuVariable other) {
    super(other);
    this.memoryType = other.memoryType;
    this.value = other.value;  // Dummy
    if (other.value != null) {
        this.setValue(java.util.Arrays.copyOf(other.value, other.value.length));
    }
}

Answered By – Stephen C

This Answer collected from stackoverflow, is licensed under cc by-sa 2.5 , cc by-sa 3.0 and cc by-sa 4.0

Leave a Reply

(*) Required, Your email will not be published