Error in the cleanHardClippedCigar function ClippingOp class

[nlfortier]

I have found an error in the ClippingOp class used by the ReadClipper. The offending function is cleanHardClippedCigar. In this function a logic error results in the returned CigarShift object always having zero values for the shiftFromStart and shiftFromEnd members.

The offending loop is shown below:
`

for (int i = 1; i <= 2; i++) {
        final int shift = 0;
        int totalHardClip = 0;
        boolean readHasStarted = false;
        boolean addedHardClips = false;

        while (!cigarStack.empty()) {
            final CigarElement cigarElement = cigarStack.pop();

            if (!readHasStarted &&
                    cigarElement.getOperator() != CigarOperator.DELETION &&
                    cigarElement.getOperator() != CigarOperator.SKIPPED_REGION &&
                    cigarElement.getOperator() != CigarOperator.HARD_CLIP) {
                readHasStarted = true;
            } else if (!readHasStarted && cigarElement.getOperator() == CigarOperator.HARD_CLIP) {
                totalHardClip += cigarElement.getLength();
            } else if (!readHasStarted && cigarElement.getOperator() == CigarOperator.DELETION) {
                totalHardClip += cigarElement.getLength();
            } else if (!readHasStarted && cigarElement.getOperator() == CigarOperator.SKIPPED_REGION) {
                totalHardClip += cigarElement.getLength();
            }

            if (readHasStarted) {
                if (i == 1) {
                    if (!addedHardClips) {
                        if (totalHardClip > 0) {
                            inverseCigarStack.push(new CigarElement(totalHardClip, CigarOperator.HARD_CLIP));
                        }
                        addedHardClips = true;
                    }
                    inverseCigarStack.push(cigarElement);
                } else {
                    if (!addedHardClips) {
                        if (totalHardClip > 0) {
                            cleanCigar.add(new CigarElement(totalHardClip, CigarOperator.HARD_CLIP));
                        }
                        addedHardClips = true;
                    }
                    cleanCigar.add(cigarElement);
                }
            }
        }
        // first pass  (i=1) is from end to start of the cigar elements
        if (i == 1) {
            shiftFromEnd = shift;
            cigarStack = inverseCigarStack;
        }
        // second pass (i=2) is from start to end with the end already cleaned
        else {
            shiftFromStart = shift;
        }
    }
}

`

Notice that the variable shift is initialized, but never assigned to again for the duration of the loop. Thus shiftFromStart and shiftFromEnd are always set to zero upon completion of the loop. These values are used by the applyHardClipBases function, which is called in a number of places to hard clip bases from a read, but because of this error, they will always be zeroed out.

The function containing the error is in the file "src/main/java/org/broadinstitute/hellbender/utils/clipping/ClippingOp.java" line 523

[lbergelson]

@nlfortier Thank you for investigating and reporting this. These methods are a consistent source of weird edge cases.

[davidbenjamin]

Closed by #6403.