Is there an easy way to write a FindBugs detector for comparisons?

Question

I currently would like to write some static analysis tool that analyzes a java code base, and helps detect any situation where you do a comparison of a java Optional and null.

So possible code like:

    Optional result = method();
    if (result != null) {
         //do something
    } else {
         //never reached because method should never return null
    }

When I looked at possibly extending FindBugs, the major comparison detector I found was here: https://github.com/findbugsproject/findbugs/blob/d1e60f8dbeda0a454f2d497ef8dcb878fa8e3852/findbugs/src/java/edu/umd/cs/findbugs/detect/FindRefComparison.java

That's alot of code to glean through to figure out how to write something that might be able to do this comparison detection, and doesn't seem that easy to extend.

While this sort of static analysis tool would be useful to us, as it would help prevent situations where someone decided to change the method signature of a function from an object that may or may not be null to one that always returns a java Optional, but didn't update all of the locations it was used, but it's not so valuable I can spend time puttering away trying to implement it in FindBugs.

1. Am I parsing the FindBugs code base enough that adding this sort of check would take some effort to extend?
2. Is there any easier way to get what I want?

Answer 1

private static final ImmutableSet<String> OPTIONAL_CLASSES =
      ImmutableSet.of(com.google.common.base.Optional.class.getName(), "java.util.Optional");

private static final Set<Kind> COMPARISON_OPERATORS =
          EnumSet.of(Kind.EQUAL_TO, Kind.NOT_EQUAL_TO);

private static boolean isNull(ExpressionTree tree) {
    return tree.getKind() == Kind.NULL_LITERAL;
}

private static boolean isOptional(ExpressionTree tree, VisitorState state) {
        Type type = ASTHelpers.getType(tree);
    for (String className : OPTIONAL_CLASSES) {
            if (ASTHelpers.isSameType(type, state.getTypeFromString(className), state)) {
                    return true;
            }
    }
    return false;
}

private boolean isSuppressed(Tree tree, String suppression) {
    SuppressWarnings annotation = ASTHelpers.getAnnotation(ASTHelpers.getSymbol(tree), SuppressWarnings.class);
    return annotation != null && Arrays.stream(annotation.value()).anyMatch(suppression::equals);
}

@Override
public Description matchBinary(BinaryTree tree, VisitorState state) {
    if (!COMPARISON_OPERATORS.contains(tree.getKind())) {
        return Description.NO_MATCH;
    }

    ExpressionTree leftOperand = tree.getLeftOperand();
    ExpressionTree rightOperand = tree.getRightOperand();
    if (isNull(leftOperand) && isOptional(rightOperand, state) ||
            isNull(rightOperand) && isOptional(leftOperand, state)) {
            return describeMatch(tree);
    }

    return Description.NO_MATCH;
}

I think I got this working in ErrorProne. If I get some tests working for it, I'll contribute it back to the community.