Skip to content

Commit

Permalink
Add baseline support for records (#377)
Browse files Browse the repository at this point in the history
Fixes #374

Avoid throwing exceptions when the source code contains records.  Since
records are a new kind of class, refactor the switch statements that
make assertions about which elements and trees represent classes.  Use
general purpose `isClassElement` and `isClassTree` methods that are
forward-compatible with records instead of enumerating `ElementKind` or
`Tree.Kind` constants.

Refactor `checkFieldInitialization` to ignore final fields.  This check
was wrongly flagging records because it couldn't analyze record
constructors properly.  Without the change to ignore final fields, this
check runs into two kinds of trouble with records:

  1. It doesn't inspect record constructors at all because they are
     generated.  `NullAway.isConstructor(MethodTree)` returns false and
     so those trees are skipped.

  2. The trees for the generated constructors don't contain explicit
     field assignments.  That's just not how those were implemented.
     Something else is responsible for assigning the fields at runtime,
     apparently.

Rather than trying to teach NullAway about record constructors
specifically, count on javac to verify that all final fields are
initialized.  Presumably, if NullAway were to complain that a constructor
did not initialize a final field, that complaint would be redundant (with
the error that javac would produce) at best and incorrect otherwise.

Co-authored-by: Manu Sridharan <msridhar@gmail.com>
  • Loading branch information
michaelhixson and msridhar committed Apr 2, 2021
1 parent 0c27e89 commit e552ba7
Showing 1 changed file with 14 additions and 25 deletions.
39 changes: 14 additions & 25 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Expand Up @@ -109,6 +109,8 @@
import javax.lang.model.element.NestingKind;
import javax.lang.model.type.TypeKind;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.javacutil.ElementUtils;
import org.checkerframework.javacutil.TreeUtils;

/**
* Checker for nullability errors. It assumes that any field, method parameter, or return type that
Expand Down Expand Up @@ -1422,6 +1424,12 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) {
constructorInitInfo = checkConstructorInitialization(entities, state);
notInitializedInConstructors = ImmutableSet.copyOf(constructorInitInfo.values());
}
// Filter out final fields, since javac will already check initialization
notInitializedInConstructors =
ImmutableSet.copyOf(
Sets.filter(
notInitializedInConstructors,
symbol -> !symbol.getModifiers().contains(Modifier.FINAL)));
class2ConstructorUninit.putAll(classSymbol, notInitializedInConstructors);
Set<Symbol> notInitializedAtAll =
notAssignedInAnyInitializer(entities, notInitializedInConstructors, state);
Expand Down Expand Up @@ -1734,6 +1742,10 @@ private FieldInitEntities collectEntities(ClassTree tree, VisitorState state) {

// we assume getMembers() returns members in the same order as the declarations
for (Tree memberTree : tree.getMembers()) {
if (TreeUtils.isClassTree(memberTree)) {
// do nothing
continue;
}
switch (memberTree.getKind()) {
case METHOD:
// check if it is a constructor or an @Initializer method
Expand Down Expand Up @@ -1776,12 +1788,6 @@ private FieldInitEntities collectEntities(ClassTree tree, VisitorState state) {
instanceInitializerBlocks.add(blockTree);
}
break;
case ENUM:
case CLASS:
case INTERFACE:
case ANNOTATION_TYPE:
// do nothing
break;
default:
throw new RuntimeException(
memberTree.getKind().toString() + " " + state.getSourceForNode(memberTree));
Expand Down Expand Up @@ -1981,30 +1987,13 @@ private boolean mayBeNullFieldAccess(VisitorState state, ExpressionTree expr, Sy
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}

/**
* @param kind an element kind.
* @return <code>true</code> if a deference of the kind might dereference null, <code>false</code>
* otherwise
*/
private boolean kindMayDeferenceNull(ElementKind kind) {
switch (kind) {
case CLASS:
case PACKAGE:
case ENUM:
case INTERFACE:
case ANNOTATION_TYPE:
return false;
default:
return true;
}
}

private Description matchDereference(
ExpressionTree baseExpression, ExpressionTree derefExpression, VisitorState state) {
Symbol dereferenced = ASTHelpers.getSymbol(baseExpression);
if (dereferenced == null
|| dereferenced.type.isPrimitive()
|| !kindMayDeferenceNull(dereferenced.getKind())) {
|| dereferenced.getKind() == ElementKind.PACKAGE
|| ElementUtils.isClassElement(dereferenced)) {
// we know we don't have a null dereference here
return Description.NO_MATCH;
}
Expand Down

0 comments on commit e552ba7

Please sign in to comment.