9

The Apache Thrift code generator generates classes that look something like this. The numCpus field is nullable, but since it's held as a value type there's another isSet field to determine if it's really set:

public class TaskConfig extends TBase<TaskConfig, TaskConfig._Fields> {
  private boolean isSetNumCpus; // Actual implemntation is a bitfield.
  private double numCpus;

  public boolean isSetNumCpus() {
    return isSetNumCpus;
  }

  public double getNumCpus() {
    return numCpus;
  }

  public void setNumCpus(double numCpus) {
    this.numCpus = numCpus;
    this.isSetNumCpus = true;
  }

  // hashCode, equals, copy constructor, field enum etc. omitted
}

Our style guide prefers wrapping nullable values as Optional so that we can't forget a null check. So it's common to see this bit of code:

TaskConfig task = getTaskConfigFromWire();
Optional<Double> numCpus = Optional.ofNullable(task.getNumCpus());

But this is wrong - thanks to autoboxing this argument can never be null, and the correct call should look like:

TaskConfig task = getTaskConfigFromWire();
Optional<Double> numCpus = task.isSetNumCpus()
    ? Optional.of(task.getNumCpus())
    : Optional.<Double>empty();

Is there a way to write a PMD rule that catches this call (Optional.ofNullable called with a value type that will be autoboxed)?

1 Answer 1

1

I would download the PMD binary and use the designer, you will be able to see how the expression has a PrimaryExpression and PrimaryPrefix of 'Name: Optional.ofNullable', then you can inspect the PrimarySuffix Arguments to find the method call or variable. You'll then have to look back up the class file to find the types and use that to determine whether you have incorrectly Optionalised an autoboxed value.

I personally would implement this as a Java rule not an XPath one.

See http://pmd.sourceforge.net/pmd-4.3.0/howtowritearule.html for more details

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.